From 5bac0214935ad6560e43cda90307764ed780c00e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 6 Dec 2017 21:59:34 +0000 Subject: [PATCH 01/13] Remove unused variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This variable isn’t used by our Jinja templates. Presumably it was from the time before instances of `Template` were responsible for showing who the email/text message was being sent to. --- app/main/views/send.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 6e8b81f44..d53053ed9 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -21,7 +21,6 @@ from flask import ( from flask_login import login_required, current_user from notifications_python_client.errors import HTTPError -from notifications_utils.columns import Columns from notifications_utils.recipients import ( RecipientCSV, first_column_headings, @@ -523,18 +522,12 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): choose_time_form = ChooseTimeForm() with suppress(StopIteration): - first_recipient = None template.values = next(recipients.rows) - first_recipient = template.values.get( - Columns.make_key(recipients.recipient_column_headers[0]), - '' - ) session['upload_data']['notification_count'] = len(list(recipients.rows)) session['upload_data']['valid'] = not recipients.has_errors return dict( recipients=recipients, - first_recipient=first_recipient, template=template, errors=recipients.has_errors, row_errors=get_errors_for_csv(recipients, template.template_type), From 71368301f758c9335f950c2a27ce0e31fddf3a88 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Dec 2017 14:32:05 +0000 Subject: [PATCH 02/13] Make test use new fixture The `client_request` fixture means the tests take fewer lines of code and also checks some extra things (eg page titles matching

s). Changing it in this commit, independently of functional changes to this test. --- tests/app/main/views/test_send.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index f3618f384..c148d03cb 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -441,7 +441,7 @@ def test_upload_csv_invalid_extension( def test_upload_valid_csv_shows_file_contents( - logged_in_client, + client_request, mocker, mock_get_service_template_with_placeholders, mock_s3_upload, @@ -455,14 +455,13 @@ def test_upload_valid_csv_shows_file_contents( 07700900986, Jo, foo, foo, foo """) - response = logged_in_client.post( - url_for('main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid), - data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, - follow_redirects=True, + page = client_request.post( + 'main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, + _follow_redirects=True, + _expected_status=200, ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.text.strip() == 'Preview of Two week reminder' for index, cell in enumerate([ ' 2 ', From 3e5deeffdb97f9baf92dc4e35dcd65e0c878e4a6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Dec 2017 15:42:58 +0000 Subject: [PATCH 03/13] Split successful upload test into two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Easier to test the individual aspects of what’s going on here if they’re two separate tests, one hitting the initial `POST`, and one hitting the page that they’re subsequently redirected to. --- tests/app/main/views/test_send.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index c148d03cb..6cba9b778 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -440,7 +440,21 @@ def test_upload_csv_invalid_extension( assert "invalid.txt isn’t a spreadsheet that Notify can read" in resp.get_data(as_text=True) -def test_upload_valid_csv_shows_file_contents( +def test_upload_valid_csv_redirects_to_check_page( + client_request, + mock_get_service_template_with_placeholders, + mock_s3_upload, + fake_uuid, +): + client_request.post( + 'main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid, + _data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, + _expected_status=302, + expected_redirect='foo' + ) + + +def test_upload_valid_csv_shows_preview_and_table( client_request, mocker, mock_get_service_template_with_placeholders, @@ -450,16 +464,19 @@ def test_upload_valid_csv_shows_file_contents( fake_uuid, ): + with client_request.session_transaction() as session: + session['upload_data'] = {'template_id': fake_uuid} + mocker.patch('app.main.views.send.s3download', return_value=""" phone number,name,thing,thing,thing 07700900986, Jo, foo, foo, foo """) - page = client_request.post( - 'main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid, - _data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')}, - _follow_redirects=True, - _expected_status=200, + page = client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_type='sms', + upload_id=fake_uuid, ) assert page.h1.text.strip() == 'Preview of Two week reminder' From 587e18d2efa7f01a1a54ccca171e7a9add9d5d43 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 6 Dec 2017 22:00:45 +0000 Subject: [PATCH 04/13] Lookup index, rather than iterating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since instances of `Recipients` are now iterable we can just look up the first row by index, rather than having to hit the rows property as a generator. There is a chance that there won’t be a first row, if the file has 1 or fewer columns. So we still need to handle a possible exception (albeit it a different one to before). --- app/main/views/send.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index d53053ed9..305f5a107 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -521,8 +521,8 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) choose_time_form = ChooseTimeForm() - with suppress(StopIteration): - template.values = next(recipients.rows) + with suppress(IndexError): + template.values = recipients[0] session['upload_data']['notification_count'] = len(list(recipients.rows)) session['upload_data']['valid'] = not recipients.has_errors From c3e2bce98b977a2a3bc1bcb508c06525da7d9747 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 6 Dec 2017 22:04:58 +0000 Subject: [PATCH 05/13] Add URLs for previewing all rows of a spreadsheet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’ve heard from some users, especially those sending letters, that they’d like to check that a spreadsheet they’ve uploaded has populated the template correctly. My reckon is that seeing just one row of the spreadsheet populate the template isn’t enough to give people confidence that everything’s working properly. So this commit is the first step towards being able to preview an arbitrary row of a template, by extending the URL structure to optionally accept a row number for pages or files (ie PNG) that preview successfully uploaded spreadsheets. What this commit doesn’t do is link to these pages; that will come as part of a subsequent commit. --- app/main/views/send.py | 24 +++++--- tests/app/main/views/test_send.py | 94 +++++++++++++++++++++++++++++-- 2 files changed, 103 insertions(+), 15 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 305f5a107..d7b920d98 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -2,7 +2,6 @@ import itertools from string import ascii_uppercase from orderedset import OrderedSet -from contextlib import suppress from zipfile import BadZipFile from xlrd.biffh import XLRDError from werkzeug.routing import RequestRedirect @@ -460,7 +459,7 @@ def send_test_preview(service_id, template_id, filetype): return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page')) -def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): +def _check_messages(service_id, template_type, upload_id, preview_row, letters_as_pdf=False): if not session.get('upload_data'): # if we just return a `redirect` (302) object here, we'll get errors when we try and unpack in the @@ -496,6 +495,7 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): template_type=template_type, upload_id=upload_id, filetype='png', + row_index=preview_row, ) if not letters_as_pdf else None, email_reply_to=email_reply_to, sms_sender=sms_sender @@ -521,8 +521,11 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) choose_time_form = ChooseTimeForm() - with suppress(IndexError): - template.values = recipients[0] + try: + template.values = recipients[preview_row] + except IndexError: + if preview_row > 0: + abort(404) session['upload_data']['notification_count'] = len(list(recipients.rows)) session['upload_data']['valid'] = not recipients.has_errors @@ -554,11 +557,12 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False): @main.route("/services///check/", methods=['GET']) +@main.route("/services///check//row-", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages(service_id, template_type, upload_id): +def check_messages(service_id, template_type, upload_id, row_index=0): - data = _check_messages(service_id, template_type, upload_id) + data = _check_messages(service_id, template_type, upload_id, row_index) if ( data['recipients'].too_many_rows or @@ -581,22 +585,24 @@ def check_messages(service_id, template_type, upload_id): @main.route("/services///check/.", methods=['GET']) +@main.route("/services///check//row-.", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def check_messages_preview(service_id, template_type, upload_id, filetype): +def check_messages_preview(service_id, template_type, upload_id, filetype, row_index=0): if filetype not in ('pdf', 'png'): abort(404) template = _check_messages( - service_id, template_type, upload_id, letters_as_pdf=True + service_id, template_type, upload_id, row_index, letters_as_pdf=True )['template'] return TemplatePreview.from_utils_template(template, filetype) @main.route("/services///check/", methods=['POST']) +@main.route("/services///check//row-", methods=['POST']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def recheck_messages(service_id, template_type, upload_id): +def recheck_messages(service_id, template_type, upload_id, row_index=0): if not session.get('upload_data'): return redirect(url_for('main.choose_template', service_id=service_id)) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 6cba9b778..d8b5b40ee 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -454,14 +454,34 @@ def test_upload_valid_csv_redirects_to_check_page( ) +@pytest.mark.parametrize('extra_args, expected_recipient, expected_message', [ + ( + {}, + 'To: 07700900001', + 'Test Service: A, Template content with & entity', + ), + ( + {'row_index': 0}, + 'To: 07700900001', + 'Test Service: A, Template content with & entity', + ), + ( + {'row_index': 2}, + 'To: 07700900003', + 'Test Service: C, Template content with & entity', + ), +]) def test_upload_valid_csv_shows_preview_and_table( client_request, mocker, + mock_get_live_service, mock_get_service_template_with_placeholders, - mock_s3_upload, mock_get_users_by_service, mock_get_detailed_service_for_today, fake_uuid, + extra_args, + expected_recipient, + expected_message, ): with client_request.session_transaction() as session: @@ -469,7 +489,9 @@ def test_upload_valid_csv_shows_preview_and_table( mocker.patch('app.main.views.send.s3download', return_value=""" phone number,name,thing,thing,thing - 07700900986, Jo, foo, foo, foo + 07700900001, A, foo, foo, foo + 07700900002, B, foo, foo, foo + 07700900003, C, foo, foo, foo """) page = client_request.get( @@ -477,13 +499,17 @@ def test_upload_valid_csv_shows_preview_and_table( service_id=SERVICE_ONE_ID, template_type='sms', upload_id=fake_uuid, + **extra_args ) assert page.h1.text.strip() == 'Preview of Two week reminder' + assert page.select_one('.sms-message-recipient').text.strip() == expected_recipient + assert page.select_one('.sms-message-wrapper').text.strip() == expected_message + for index, cell in enumerate([ ' 2 ', - '
07700900986
', - '
Jo
', + '
07700900001
', + '
A
', ( ' ' '
' @@ -497,6 +523,43 @@ def test_upload_valid_csv_shows_preview_and_table( assert normalize_spaces(str(page.select('table tbody td')[index])) == cell +@pytest.mark.parametrize('row_index, expected_status', [ + (0, 200), + (2, 200), + (3, 404), +]) +def test_404_for_previewing_a_row_out_of_range( + client_request, + mocker, + mock_get_live_service, + mock_get_service_template_with_placeholders, + mock_get_users_by_service, + mock_get_detailed_service_for_today, + fake_uuid, + row_index, + expected_status, +): + + with client_request.session_transaction() as session: + session['upload_data'] = {'template_id': fake_uuid} + + mocker.patch('app.main.views.send.s3download', return_value=""" + phone number,name,thing,thing,thing + 07700900001, A, foo, foo, foo + 07700900002, B, foo, foo, foo + 07700900003, C, foo, foo, foo + """) + + client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_type='sms', + upload_id=fake_uuid, + row_index=row_index, + _expected_status=expected_status, + ) + + def test_send_test_doesnt_show_file_contents( logged_in_client, mocker, @@ -1389,6 +1452,20 @@ def test_can_start_letters_job( @pytest.mark.parametrize('filetype', ['pdf', 'png']) +@pytest.mark.parametrize('extra_args, expected_values', [ + ( + {}, + {'postcode': 'abc123', 'addressline1': '123 street'}, + ), + ( + {'row_index': 0}, + {'postcode': 'abc123', 'addressline1': '123 street'}, + ), + ( + {'row_index': 1}, + {'postcode': 'cba321', 'addressline1': '321 avenue'}, + ), +]) def test_should_show_preview_letter_message( filetype, logged_in_platform_admin_client, @@ -1398,6 +1475,8 @@ def test_should_show_preview_letter_message( service_one, fake_uuid, mocker, + extra_args, + expected_values, ): service_one['permissions'] = ['letter'] mocker.patch('app.service_api_client.get_service', return_value={"data": service_one}) @@ -1407,7 +1486,8 @@ def test_should_show_preview_letter_message( 'app.main.views.send.s3download', return_value='\n'.join( ['address line 1, postcode'] + - ['123 street, abc123'] + ['123 street, abc123'] + + ['321 avenue, cba321'] ) ) mocked_preview = mocker.patch( @@ -1430,7 +1510,8 @@ def test_should_show_preview_letter_message( service_id=service_id, template_type='letter', upload_id=fake_uuid, - filetype=filetype + filetype=filetype, + **extra_args ) ) @@ -1441,6 +1522,7 @@ def test_should_show_preview_letter_message( assert mocked_preview.call_args[0][0].id == template_id assert type(mocked_preview.call_args[0][0]) == LetterPreviewTemplate assert mocked_preview.call_args[0][1] == filetype + assert mocked_preview.call_args[0][0].values == expected_values def test_dont_show_preview_letter_templates_for_bad_filetype( From 5f5dd3ac410dbc52ad8ee7a6cbc7b2bc9822c4c5 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Dec 2017 11:48:30 +0000 Subject: [PATCH 06/13] Rewrite check for row existence as conditional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because exceptions can be expensive performance wise (see: https://docs.python.org/3/faq/design.html#how-fast-are-exceptions). Since we’re counting the number of rows anyway this doesn’t introduce any performance overhead there. And I think it’s equally readable/same number of lines of code. --- app/main/views/send.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index d7b920d98..f375c1645 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -521,20 +521,21 @@ def _check_messages(service_id, template_type, upload_id, preview_row, letters_a back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) choose_time_form = ChooseTimeForm() - try: - template.values = recipients[preview_row] - except IndexError: - if preview_row > 0: - abort(404) + count_of_recipients = len(list(recipients.rows)) - session['upload_data']['notification_count'] = len(list(recipients.rows)) + if preview_row < count_of_recipients: + template.values = recipients[preview_row] + elif preview_row > 0: + abort(404) + + session['upload_data']['notification_count'] = count_of_recipients session['upload_data']['valid'] = not recipients.has_errors return dict( recipients=recipients, template=template, errors=recipients.has_errors, row_errors=get_errors_for_csv(recipients, template.template_type), - count_of_recipients=session['upload_data']['notification_count'], + count_of_recipients=count_of_recipients, count_of_displayed_recipients=( len(list(recipients.initial_annotated_rows_with_errors)) if any(recipients.rows_with_errors) and not recipients.missing_column_headers else From eb395db2d6fe8392089be40488b9d98ed0bf697b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Dec 2017 11:59:51 +0000 Subject: [PATCH 07/13] Fix error when file has header rows but no data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the error was saying ‘It needs at least one row of data, and columns called None’. --- app/templates/views/check/column-errors.html | 18 ++++++++++++------ tests/app/main/views/test_send.py | 10 ++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 553449600..6f3b626d1 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -38,12 +38,18 @@

Your file is missing some rows

-

- It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list( - prefix='a column called', - prefix_plural='columns called' - ) }}. -

+ {% if recipients.missing_column_headers %} +

+ It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list( + prefix='a column called', + prefix_plural='columns called' + ) }}. +

+ {% else %} +

+ It needs at least one row of data. +

+ {% endif %} {% elif not recipients.has_recipient_columns %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index d8b5b40ee..bc7423dea 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -352,6 +352,16 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors( 'Skip to file contents' ) ), + ( + """ + phone number, name + """, + ( + 'Your file is missing some rows ' + 'It needs at least one row of data. ' + 'Skip to file contents' + ) + ), ( "+447700900986", ( From 8bfb67c702abba52fd360718762e8f78d58758a2 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sun, 29 Oct 2017 22:18:46 +0000 Subject: [PATCH 08/13] Go fullscreen for row-level errors in spreadsheets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bits of context: 1. As we start dealing with letters, which have more columns, it’s more likely that people’s spreadsheets won’t fit in our current layout. 2. We already removed the view of the template from the page that shows row-level errors (eg bad phone number or missing personalisation) in spreadsheets because you don’t need to know about the content of the message in order to fix the errors. This commit goes further by removing anything that isn’t to do with the errors, including the normal GOV.UK header and the service’s navigation. This means the content can go the width of the page, which means it can be allowed to scroll horizontally without being a usability car crash. Which means that the layout doesn’t break with a spreadsheet that has lots of columns. --- app/assets/javascripts/fullscreenTable.js | 43 ++++++++++ .../components/fullscreen-table.scss | 85 +++++++++++++++++++ app/assets/stylesheets/main.scss | 1 + app/templates/fullscreen_template.html | 23 +++++ app/templates/views/check/row-errors.html | 16 ++-- gulpfile.babel.js | 1 + 6 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 app/assets/javascripts/fullscreenTable.js create mode 100644 app/assets/stylesheets/components/fullscreen-table.scss create mode 100644 app/templates/fullscreen_template.html diff --git a/app/assets/javascripts/fullscreenTable.js b/app/assets/javascripts/fullscreenTable.js new file mode 100644 index 000000000..69d49fa1c --- /dev/null +++ b/app/assets/javascripts/fullscreenTable.js @@ -0,0 +1,43 @@ +(function(Modules) { + "use strict"; + + Modules.FullscreenTable = function() { + + this.start = function(component) { + + this.$component = $(component); + this.nativeHeight = this.$component.innerHeight(); + this.topOffset = this.$component.offset().top; + + this.insertShim(); + this.maintainHeight(); + + $(window).on('scroll resize', this.maintainHeight); + + if ( + window.GOVUK.stopScrollingAtFooter && + window.GOVUK.stopScrollingAtFooter.updateFooterTop + ) { + window.GOVUK.stopScrollingAtFooter.updateFooterTop(); + } + + }; + + this.insertShim = () => this.$component.after( + $("
").css({ + 'height': this.nativeHeight - this.topOffset, + 'top': this.topOffset + }) + ); + + this.maintainHeight = () => this.$component.css({ + 'max-height': Math.min( + $(window).height() - this.topOffset + $('html, body').scrollTop(), + this.nativeHeight + ), + 'min-height': $(window).height() - this.topOffset + }); + + }; + +})(window.GOVUK.Modules); diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss new file mode 100644 index 000000000..b9e406caf --- /dev/null +++ b/app/assets/stylesheets/components/fullscreen-table.scss @@ -0,0 +1,85 @@ +body.with-fullscreen { + + #global-header, + #global-header-bar { + display: none; + } + + #footer { + height: 0; + overflow: hidden; + border-color: $white; + } + + .shim { + margin-bottom: 5px; + } + +} + + +.fullscreen { + + &-header { + padding: $gutter-half $gutter-half 0 $gutter-half; + margin-top: -$gutter-half; + } + + &-content { + + width: 100%; + background: $white; + z-index: 10; + overflow-x: scroll; + overflow-y: hidden; + box-sizing: border-box; + position: absolute; + padding-left: $gutter-half; + + .banner-dangerous { + margin: $gutter-half $gutter-half 0 $gutter-half; + position: sticky; + left: 0; + } + + .table { + border-right: $gutter-half solid $white; // border used as padding + } + + th, + .table-field-error-label { + white-space: nowrap; + } + + .table-show-more-link { + border: none; + text-align: left; + } + + } + + &-shim { + width: 100%; + position: relative; + z-index: 9; + background: $white; + } + + &-sticky-bar { + + z-index: 20; + padding-right: 0; + + .page-footer-back-link { + position: absolute; + right: $gutter-half; + top: 20px; + } + + .file-upload-button { + margin: 0 0 0 $gutter-half; + } + + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index ec57bf390..d4a2315df 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -59,6 +59,7 @@ $path: '/static/images/'; @import 'components/letter'; @import 'components/live-search'; @import 'components/stick-at-top-when-scrolling'; +@import 'components/fullscreen-table'; @import 'components/vendor/breadcrumbs'; @import 'components/vendor/responsive-embed'; diff --git a/app/templates/fullscreen_template.html b/app/templates/fullscreen_template.html new file mode 100644 index 000000000..11be0fa67 --- /dev/null +++ b/app/templates/fullscreen_template.html @@ -0,0 +1,23 @@ +{% extends "admin_template.html" %} + +{% block inside_header %}{% endblock %} +{% block proposition_header %}{% endblock %} + +{% block body_classes %} with-fullscreen {% endblock %} + +{% block footer_top %}{% endblock %} +{% block footer_support_links %}{% endblock %} + +{% block content %} +
+
+ {% block fullscreen_pre_title %}{% endblock %} +
+
+ {% block fullscreen_title %}{% endblock %} +
+
+ {% block fullscreen_content %}{% endblock %} +
+
+{% endblock %} diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html index 537710516..1ea7f870d 100644 --- a/app/templates/views/check/row-errors.html +++ b/app/templates/views/check/row-errors.html @@ -1,4 +1,4 @@ -{% extends "withnav_template.html" %} +{% extends "fullscreen_template.html" %} {% from "components/banner.html" import banner_wrapper %} {% from "components/radios.html" import radio_select %} {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %} @@ -13,13 +13,13 @@

{% endmacro %} -{% block service_page_title %} +{% block per_page_title %} Error {% endblock %} -{% block maincolumn_content %} +{% block fullscreen_pre_title %} -
+
{% call banner_wrapper(type='dangerous') %} {% if row_errors|length == 1 %}

@@ -45,10 +45,16 @@ {% endcall %}

-
+ {% endblock %} + {% block fullscreen_title %} +
{{ file_upload(form.file, button_text='Re-upload your file') }} + Go back
+ {% endblock %} + {% block fullscreen_content %} + {% call(item, row_number) list_table( recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, caption=original_file_name, diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 872630dfb..2f724efd9 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -68,6 +68,7 @@ gulp.task('javascripts', () => gulp paths.src + 'javascripts/liveSearch.js', paths.src + 'javascripts/errorTracking.js', paths.src + 'javascripts/preventDuplicateFormSubmissions.js', + paths.src + 'javascripts/fullscreenTable.js', paths.src + 'javascripts/main.js' ]) .pipe(plugins.prettyerror()) From a58cb75b88c13a14db72ed3226509df4e68084e4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 11 Dec 2017 14:10:45 +0000 Subject: [PATCH 09/13] Force scrollbar always visible on OS X --- .../components/fullscreen-table.scss | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss index b9e406caf..004fa7258 100644 --- a/app/assets/stylesheets/components/fullscreen-table.scss +++ b/app/assets/stylesheets/components/fullscreen-table.scss @@ -15,6 +15,25 @@ body.with-fullscreen { margin-bottom: 5px; } + &::-webkit-scrollbar { + -webkit-appearance: none; + } + + &::-webkit-scrollbar:vertical { + width: 11px; + } + + &::-webkit-scrollbar-thumb { + border-radius: 8px; + border: 2px solid $white; + background-color: rgba(0, 0, 0, .5); + } + + &::-webkit-scrollbar-track { + background-color: $white; + border-radius: 8px; + } + } @@ -36,6 +55,25 @@ body.with-fullscreen { position: absolute; padding-left: $gutter-half; + &::-webkit-scrollbar { + -webkit-appearance: none; + } + + &::-webkit-scrollbar:horizontal { + height: 11px; + } + + &::-webkit-scrollbar-thumb { + border-radius: 8px; + border: 2px solid $white; + background-color: rgba(0, 0, 0, .5); + } + + &::-webkit-scrollbar-track { + background-color: $white; + border-radius: 8px; + } + .banner-dangerous { margin: $gutter-half $gutter-half 0 $gutter-half; position: sticky; From e3be2522f408cecacbc27ce03f8160b4129d939f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 11 Dec 2017 15:33:40 +0000 Subject: [PATCH 10/13] Fix jumpy cancel button --- app/assets/stylesheets/components/fullscreen-table.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss index 004fa7258..74593a9ed 100644 --- a/app/assets/stylesheets/components/fullscreen-table.scss +++ b/app/assets/stylesheets/components/fullscreen-table.scss @@ -114,7 +114,8 @@ body.with-fullscreen { top: 20px; } - .file-upload-button { + .file-upload-button, + .file-upload-button-cancel { margin: 0 0 0 $gutter-half; } From c6f54966bfa01356079f52ad9be3b0a561314c93 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 14 Dec 2017 11:58:40 +0000 Subject: [PATCH 11/13] Change tables to scroll in-page, not full screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were three problems with showing tables fullscreen: - it was over-optimised for very big spreadsheets, whereas most users will only have a few columns in their files - it was jarring to go from full screen and back to the normal layout - it was a bit change for existing users, where we prefer incremental changes that make things better without disrupting people’s work (where possible) So this commit changes the big table to scroll horizontally in the page, not take up the full width of the page. From the fullscreen table it keeps: - the shimming method to keep the horizontal scrollbar at the bottom of the screen at all times It introduces some more refinements to make it nicer to use: - fixing the first column, so you always know what row you’re on - adding shadows indicate where there is content that’s scrolled outside the edges of the container --- app/assets/javascripts/fullscreenTable.js | 94 ++++++++-- .../components/fullscreen-table.scss | 166 ++++++++++-------- .../stick-at-top-when-scrolling.scss | 21 +++ app/assets/stylesheets/components/table.scss | 3 +- app/templates/fullscreen_template.html | 23 --- app/templates/views/check/column-errors.html | 86 ++++----- app/templates/views/check/ok.html | 52 +++--- app/templates/views/check/row-errors.html | 86 +++++---- 8 files changed, 303 insertions(+), 228 deletions(-) delete mode 100644 app/templates/fullscreen_template.html diff --git a/app/assets/javascripts/fullscreenTable.js b/app/assets/javascripts/fullscreenTable.js index 69d49fa1c..5d4080b2f 100644 --- a/app/assets/javascripts/fullscreenTable.js +++ b/app/assets/javascripts/fullscreenTable.js @@ -6,13 +6,22 @@ this.start = function(component) { this.$component = $(component); - this.nativeHeight = this.$component.innerHeight(); + this.$table = this.$component.find('table'); + this.nativeHeight = this.$component.innerHeight() + 20; // 20px to allow room for scrollbar this.topOffset = this.$component.offset().top; - this.insertShim(); + this.insertShims(); + this.maintainWidth(); this.maintainHeight(); + this.toggleShadows(); - $(window).on('scroll resize', this.maintainHeight); + $(window) + .on('scroll resize', this.maintainHeight) + .on('resize', this.maintainWidth); + + this.$scrollableTable + .on('scroll', this.toggleShadows) + .on('scroll', this.maintainHeight); if ( window.GOVUK.stopScrollingAtFooter && @@ -23,20 +32,75 @@ }; - this.insertShim = () => this.$component.after( - $("
").css({ - 'height': this.nativeHeight - this.topOffset, - 'top': this.topOffset - }) - ); + this.insertShims = () => { - this.maintainHeight = () => this.$component.css({ - 'max-height': Math.min( - $(window).height() - this.topOffset + $('html, body').scrollTop(), + this.$table.wrap('
'); + + this.$component + .append( + this.$component.find('.fullscreen-scrollable-table') + .clone() + .addClass('fullscreen-fixed-table') + .removeClass('fullscreen-scrollable-table') + .attr('role', 'presentation') + ) + .append( + '
' + ) + .after( + $("
").css({ + 'height': this.nativeHeight, + 'top': this.topOffset + }) + ); + + this.$scrollableTable = this.$component.find('.fullscreen-scrollable-table'); + this.$fixedTable = this.$component.find('.fullscreen-fixed-table'); + + }; + + this.maintainHeight = () => { + + let height = Math.min( + $(window).height() - this.topOffset + $('html, body').scrollTop() + 5, this.nativeHeight - ), - 'min-height': $(window).height() - this.topOffset - }); + ); + + this.$scrollableTable.outerHeight(height); + this.$fixedTable.outerHeight(height); + + }; + + this.maintainWidth = () => { + + let indexColumnWidth = this.$fixedTable.find('.table-field-index').outerWidth(); + + this.$scrollableTable + .css({ + 'width': this.$component.parent('main').width() - indexColumnWidth, + 'margin-left': indexColumnWidth + }); + + this.$fixedTable + .width(indexColumnWidth + 4); + + }; + + this.toggleShadows = () => { + + this.$fixedTable + .toggleClass( + 'fullscreen-scrolled-table', + this.$scrollableTable.scrollLeft() > 0 + ); + + this.$component.find('.fullscreen-right-shadow') + .toggleClass( + 'visible', + this.$scrollableTable.scrollLeft() < (this.$table.width() - this.$scrollableTable.width()) + ); + + }; }; diff --git a/app/assets/stylesheets/components/fullscreen-table.scss b/app/assets/stylesheets/components/fullscreen-table.scss index 74593a9ed..b552fcbd8 100644 --- a/app/assets/stylesheets/components/fullscreen-table.scss +++ b/app/assets/stylesheets/components/fullscreen-table.scss @@ -1,59 +1,68 @@ -body.with-fullscreen { - - #global-header, - #global-header-bar { - display: none; - } - - #footer { - height: 0; - overflow: hidden; - border-color: $white; - } - - .shim { - margin-bottom: 5px; - } - - &::-webkit-scrollbar { - -webkit-appearance: none; - } - - &::-webkit-scrollbar:vertical { - width: 11px; - } - - &::-webkit-scrollbar-thumb { - border-radius: 8px; - border: 2px solid $white; - background-color: rgba(0, 0, 0, .5); - } - - &::-webkit-scrollbar-track { - background-color: $white; - border-radius: 8px; - } - -} - - .fullscreen { - &-header { - padding: $gutter-half $gutter-half 0 $gutter-half; - margin-top: -$gutter-half; - } - &-content { - width: 100%; background: $white; z-index: 10; - overflow-x: scroll; overflow-y: hidden; box-sizing: border-box; position: absolute; - padding-left: $gutter-half; + margin: 5px 0 $gutter 0; + padding: 0 0 0 0; + overflow: hidden; + border-bottom: 1px solid $border-colour; + + .table { + + margin-bottom: 0; + + tr:last-child { + td { + border-bottom: 1px solid $white; + } + } + + } + + th, + .table-field-error-label, + .table-field-center-aligned { + white-space: nowrap; + } + + } + + &-right-shadow { + + position: absolute; + top: 0; + right: 0; + width: 4px; + height: 100%; + z-index: 200; + + &.visible { + transition: box-shadow 0.3s ease-in-out; + box-shadow: inset -1px 0 0 0 $border-colour, inset -3px 0 0 0 rgba($border-colour, 0.2); + } + + } + + &-scrollable-table { + + overflow-x: auto; + overflow-y: hidden; + + .table-field-heading-first, + .table-field-index { + display: none; + } + + .table-field-center-aligned { + position: relative; + z-index: 150; + background: $white; + } &::-webkit-scrollbar { -webkit-appearance: none; @@ -61,6 +70,7 @@ body.with-fullscreen { &::-webkit-scrollbar:horizontal { height: 11px; + background-color: $white; } &::-webkit-scrollbar-thumb { @@ -74,24 +84,43 @@ body.with-fullscreen { border-radius: 8px; } - .banner-dangerous { - margin: $gutter-half $gutter-half 0 $gutter-half; - position: sticky; - left: 0; + } + + &-fixed-table { + + position: absolute; + top: 0; + overflow: hidden; + + .table-field-heading { + visibility: hidden; } - .table { - border-right: $gutter-half solid $white; // border used as padding + .table-field-center-aligned { + width: 0; + position: relative; + z-index: 100; + visibility: hidden; } - th, - .table-field-error-label { - white-space: nowrap; + .table-field-heading-first, + .table-field-index { + transition: none; + position: relative; + z-index: 200; + background: $white; } - .table-show-more-link { - border: none; - text-align: left; + } + + &-scrolled-table { + + padding-bottom: 20px; + + .table-field-heading-first, + .table-field-index { + transition: box-shadow 0.3s ease-in-out; + box-shadow: 1px 0 0 0 $border-colour, 3px 0 0 0 rgba($border-colour, 0.2); } } @@ -100,25 +129,6 @@ body.with-fullscreen { width: 100%; position: relative; z-index: 9; - background: $white; - } - - &-sticky-bar { - - z-index: 20; - padding-right: 0; - - .page-footer-back-link { - position: absolute; - right: $gutter-half; - top: 20px; - } - - .file-upload-button, - .file-upload-button-cancel { - margin: 0 0 0 $gutter-half; - } - } } diff --git a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss index cd87884f1..cee9ffd16 100644 --- a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss +++ b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss @@ -16,9 +16,24 @@ margin-bottom: 20px; } + .back-to-top-link { + + position: absolute; + top: $gutter; + right: $gutter-half; + opacity: 0; + transition: opacity 0.1s ease-in-out; + + @include ie-lte(8) { + display: none; + } + + } + } .content-fixed { + position: fixed; top: 0; background: $white; @@ -28,6 +43,12 @@ border-bottom: 1px solid $border-colour; box-shadow: 0 2px 0 0 rgba($border-colour, 0.2); transition: background 0.6s ease-in-out, margin-top 0.4s ease-out; + + .back-to-top-link { + opacity: 1; + transition: opacity 0.6s ease-in-out; + } + } .shim { diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index c86d5cc12..5bd4c68c0 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -209,10 +209,9 @@ .table-show-more-link { @include core-16; color: $secondary-text-colour; - margin-top: -30px; margin-bottom: $gutter * 1.3333; border-bottom: 1px solid $border-colour; - padding: 0.75em 0 0.5625em 0; + padding: 10px 0 10px 0; text-align: center; } diff --git a/app/templates/fullscreen_template.html b/app/templates/fullscreen_template.html deleted file mode 100644 index 11be0fa67..000000000 --- a/app/templates/fullscreen_template.html +++ /dev/null @@ -1,23 +0,0 @@ -{% extends "admin_template.html" %} - -{% block inside_header %}{% endblock %} -{% block proposition_header %}{% endblock %} - -{% block body_classes %} with-fullscreen {% endblock %} - -{% block footer_top %}{% endblock %} -{% block footer_support_links %}{% endblock %} - -{% block content %} -
-
- {% block fullscreen_pre_title %}{% endblock %} -
-
- {% block fullscreen_title %}{% endblock %} -
-
- {% block fullscreen_content %}{% endblock %} -
-
-{% endblock %} diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 6f3b626d1..4fec14c21 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -114,54 +114,58 @@ {% endcall %}
-
- {% if request.args.from_test %} - Back - {% else %} - {{file_upload(form.file, button_text='Re-upload your file')}} - {% endif %} + +
+
+ {% if request.args.from_test %} + Back + {% else %} + {{file_upload(form.file, button_text='Re-upload your file')}} + {% endif %} +
+ Back to top
{% if not request.args.from_test %}

{{ original_file_name }}

- {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - ''|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].error and not recipients.missing_column_headers %} - {% call field() %} - - {{ item['columns'][column].error }} - {{ item['columns'][column].data if item['columns'][column].data != None }} - - {% endcall %} - {% elif item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + ''|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].error and not recipients.missing_column_headers %} + {% call field() %} + + {{ item['columns'][column].error }} + {{ item['columns'][column].data if item['columns'][column].data != None }} + + {% endcall %} + {% elif item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} - - {% endif %} - + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} + {% endif %} +
{% if recipients.too_many_rows %}

diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index 4cd5d20f2..792f016d1 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -49,32 +49,34 @@

{{ original_file_name }}

- {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - ''|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + ''|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} +
{% endif %} diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html index 1ea7f870d..e92647886 100644 --- a/app/templates/views/check/row-errors.html +++ b/app/templates/views/check/row-errors.html @@ -1,4 +1,4 @@ -{% extends "fullscreen_template.html" %} +{% extends "withnav_template.html" %} {% from "components/banner.html" import banner_wrapper %} {% from "components/radios.html" import radio_select %} {% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %} @@ -13,11 +13,11 @@

{% endmacro %} -{% block per_page_title %} +{% block service_page_title %} Error {% endblock %} -{% block fullscreen_pre_title %} +{% block maincolumn_content %}
{% call banner_wrapper(type='dangerous') %} @@ -45,50 +45,48 @@ {% endcall %}
- {% endblock %} - {% block fullscreen_title %} -
- {{ file_upload(form.file, button_text='Re-upload your file') }} - Go back +
+
+ {{ file_upload(form.file, button_text='Re-upload your file') }} +
+ Back to top
- {% endblock %} - {% block fullscreen_content %} - - {% call(item, row_number) list_table( - recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, - caption=original_file_name, - caption_visible=False, - field_headings=[ - ''|safe - ] + recipients.column_headers - ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item['columns'][column].error and not recipients.missing_column_headers %} - {% call field() %} - - {{ item['columns'][column].error }} - {{ item['columns'][column].data if item['columns'][column].data != None }} - - {% endcall %} - {% elif item['columns'][column].ignore %} - {{ text_field(item['columns'][column].data or '', status='default') }} - {% else %} - {{ text_field(item['columns'][column].data or '') }} - {% endif %} - {% endfor %} - {% if item['columns'].get(None) %} - {% for column in item['columns'][None].data %} - {{ text_field(column, status='default') }} +
+ {% call(item, row_number) list_table( + recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, + caption=original_file_name, + caption_visible=False, + field_headings=[ + ''|safe + ] + recipients.column_headers + ) %} + {% call index_field() %} + + {{ item.index + 2 }} + + {% endcall %} + {% for column in recipients.column_headers %} + {% if item['columns'][column].error and not recipients.missing_column_headers %} + {% call field() %} + + {{ item['columns'][column].error }} + {{ item['columns'][column].data if item['columns'][column].data != None }} + + {% endcall %} + {% elif item['columns'][column].ignore %} + {{ text_field(item['columns'][column].data or '', status='default') }} + {% else %} + {{ text_field(item['columns'][column].data or '') }} + {% endif %} {% endfor %} - {% endif %} - {% endcall %} - + {% if item['columns'].get(None) %} + {% for column in item['columns'][None].data %} + {{ text_field(column, status='default') }} + {% endfor %} + {% endif %} + {% endcall %} +
{% if count_of_displayed_recipients < count_of_recipients %}

{% if row_errors and not recipients.missing_column_headers %} From d6ba0546f93414ad779b87334e4a65f29ae0d3c4 Mon Sep 17 00:00:00 2001 From: pyup-bot Date: Mon, 18 Dec 2017 23:05:21 +0000 Subject: [PATCH 12/13] Update pyexcel-xlsx from 0.5.4 to 0.5.5 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index bcc35be19..0b98d7a14 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ blinker==1.4 pyexcel==0.5.6 pyexcel-io==0.5.4 pyexcel-xls==0.5.5 -pyexcel-xlsx==0.5.4 +pyexcel-xlsx==0.5.5 pyexcel-ods3==0.5.2 pytz==2017.3 gunicorn==19.7.1 From aa22568c64d65824307d6d5ab0cbfe71df3af38a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 20 Dec 2017 15:37:34 +0000 Subject: [PATCH 13/13] Fix bug horizontal scrollbar being pushed off page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We weren’t calculating the height quite right; we were trying to compensate for something that should have been compensated for in the `stick-at-top-when-scrolling` code. Add the 5px to the shim there is required because we’re adding it to the element that the shim in replacing. --- app/assets/javascripts/fullscreenTable.js | 2 +- .../stylesheets/components/stick-at-top-when-scrolling.scss | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/fullscreenTable.js b/app/assets/javascripts/fullscreenTable.js index 5d4080b2f..27ff52636 100644 --- a/app/assets/javascripts/fullscreenTable.js +++ b/app/assets/javascripts/fullscreenTable.js @@ -62,7 +62,7 @@ this.maintainHeight = () => { let height = Math.min( - $(window).height() - this.topOffset + $('html, body').scrollTop() + 5, + $(window).height() - this.topOffset + $('html, body').scrollTop(), this.nativeHeight ); diff --git a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss index cee9ffd16..77d076ff1 100644 --- a/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss +++ b/app/assets/stylesheets/components/stick-at-top-when-scrolling.scss @@ -53,4 +53,5 @@ .shim { display: block; + margin-bottom: 5px; }