From 3bbd5381c6faa0dc08a381aac54a63e6db8d52ac Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 18 Feb 2020 16:16:51 +0000 Subject: [PATCH] Revert "Revert "Restyle template statistics and received text messages"" --- app/assets/stylesheets/components/banner.scss | 55 +++++++++++++++++ .../stylesheets/components/show-more.scss | 7 ++- app/assets/stylesheets/components/table.scss | 60 +++++++++++++++++++ app/assets/stylesheets/views/dashboard.scss | 35 +++-------- app/main/views/dashboard.py | 19 ------ app/templates/components/show-more.html | 11 ++-- app/templates/components/table.html | 7 +-- app/templates/views/dashboard/_inbox.html | 27 ++++----- app/templates/views/dashboard/_totals.html | 12 ++-- app/templates/views/dashboard/_upcoming.html | 1 - app/templates/views/dashboard/_usage.html | 6 +- .../dashboard/all-template-statistics.html | 6 +- .../views/dashboard/template-statistics.html | 20 ++++--- tests/app/main/views/test_dashboard.py | 32 +++++----- tests/conftest.py | 2 +- 15 files changed, 186 insertions(+), 114 deletions(-) diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 5a6266e4a..dac7613d4 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -137,3 +137,58 @@ } } + +.banner-dashboard { + + $baseline-shift: 5px; + + display: block; // for browsers that don't support flexbox + display: flex; + align-items: baseline; + flex-wrap: wrap; + padding: ($gutter-half - 1px) 0 ($gutter-half + 1px) 0; + border-top: 1px solid $border-colour; + border-bottom: 1px solid $border-colour; + margin-bottom: $gutter; + text-decoration: none; + + &:focus { + border-top: 1px solid transparent; + border-bottom: 1px solid transparent; + } + + &-count, + &-meta { + float: left; // for browsers that don't support flexbox + } + + &-count { + @include govuk-font(36, $weight: bold); + padding-right: 8px; + position: relative; + // remove the top of the extra line-height this introduces + top: $baseline-shift; + margin-top: -$baseline-shift; + flex: 0 1 0.85ch; + } + + &-count-label { + @include govuk-font(24, $weight: bold); + text-decoration: underline; + padding-right: govuk-spacing(6); + margin: 10px 0px 5px; // 10px includes 5px extra to counter the -5px margin-top on the count item + flex: 2 1 auto; + } + + &-meta { + @include govuk-font(19); + float: right; + text-align: right; + flex: initial; + } + + & + .banner-dashboard { + margin-top: -$gutter; + border-top: none; + } +} diff --git a/app/assets/stylesheets/components/show-more.scss b/app/assets/stylesheets/components/show-more.scss index ed7742f0a..cf912a311 100644 --- a/app/assets/stylesheets/components/show-more.scss +++ b/app/assets/stylesheets/components/show-more.scss @@ -4,7 +4,7 @@ @include core-16; display: block; padding: 0 0; - margin: $gutter-half 0 $gutter 0; + margin: $gutter-half 0 $gutter-half 0; text-align: center; border-top: 1px solid $border-colour; @@ -34,7 +34,8 @@ } -.show-more-empty { +.show-more-no-border { @extend %show-more; - margin-top: -10px; + border-top: 1px solid transparent; + margin-top: -5px; } diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index d8572e776..61dba8ed5 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -51,6 +51,66 @@ } +.template-statistics-table { + + .table { + table-layout: fixed; + } + + .table-heading { + @include core-19; + margin: 0 0 10px 0; + } + + .table-field-heading-first { + width: 52.5%; + } + + .table-row { + th { + display: table-cell; + width: 52.5%; + font-weight: normal; + + .hint, + p { + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + } + } + + &-template-name { + + @include bold-24; + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + padding: 10px 0 32px 0; + margin: -10px 0 -32px 0; + + &:focus { + + color: $text-colour; + + & + .template-statistics-table-hint { + color: $text-colour; + } + + } + + } + + &-hint { + @include core-19; + color: $secondary-text-colour; + pointer-events: none; + } + +} + .settings-table { table { diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 76a5e834c..b300d5be2 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -1,15 +1,7 @@ .dashboard { - table { - th { - @include core-19; - border-bottom: 0; - } - - td { - @include core-19; - border: 0; - } + th { + font-weight: normal; } > .heading-medium:first-of-type { @@ -34,15 +26,17 @@ color: $text-colour; text-align: left; - span { + &-bar { + @include bold-27; box-sizing: border-box; display: inline-block; overflow: visible; background: $panel-colour; color: $black; - padding: 5px 5px 2px 5px; - text-indent: -2px; - margin: 3px 0 5px 0; + padding: 10px 6px 8px 0; + text-indent: 12px; + text-align: right; + margin: 2px 0 1px 0; transition: width 0.6s ease-in-out; } @@ -84,19 +78,6 @@ color: $error-colour; } -.template-usage-table { - - border-top: 1px solid $border-colour; - border-bottom: 1px solid $border-colour; - margin-top: 10px; - margin-bottom: $gutter * 1.3333; - - .table { - margin-bottom: 5px; - } - -} - .align-with-message-body { display: block; margin-top: $gutter * 5 / 6; diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 3a02451ea..2ae4394b0 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -280,15 +280,8 @@ def get_dashboard_partials(service_id): all_statistics = template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) template_statistics = aggregate_template_usage(all_statistics) stats = aggregate_notifications_stats(all_statistics) - column_width, max_notifiction_count = get_column_properties(3) dashboard_totals = get_dashboard_totals(stats), - highest_notification_count = max( - sum( - value[key] for key in {'requested', 'failed', 'delivered'} - ) - for key, value in dashboard_totals[0].items() - ) free_sms_allowance = billing_api_client.get_free_sms_fragment_limit_for_year( current_service.id, get_current_financial_year(), @@ -312,10 +305,6 @@ def get_dashboard_partials(service_id): 'views/dashboard/_totals.html', service_id=service_id, statistics=dashboard_totals[0], - column_width=column_width, - smaller_font_size=( - highest_notification_count > max_notifiction_count - ), ), 'template-statistics': render_template( 'views/dashboard/template-statistics.html', @@ -330,7 +319,6 @@ def get_dashboard_partials(service_id): ), 'usage': render_template( 'views/dashboard/_usage.html', - column_width=column_width, **calculate_usage(yearly_usage, free_sms_allowance), ), } @@ -505,10 +493,3 @@ def get_tuples_of_financial_years( ) for year in range(start, end + 1) ) - - -def get_column_properties(number_of_columns): - return { - 2: ('column-half', 999999999), - 3: ('column-third', 99999), - }.get(number_of_columns) diff --git a/app/templates/components/show-more.html b/app/templates/components/show-more.html index 77803eced..e72f6ef80 100644 --- a/app/templates/components/show-more.html +++ b/app/templates/components/show-more.html @@ -1,7 +1,6 @@ -{% macro show_more(url=None, label=None) %} - {% if url and label %} - {{ label }} - {% else %} - - {% endif %} +{% macro show_more(url, label, with_border=True) %} + {{ label }} {% endmacro %} diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 8cbce0241..48c63ebdc 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -188,11 +188,8 @@ ) %} {% call field(align='right') %} - - {{ big_number( - count, - smallest=True - ) }} + + {{ '{:,.0f}'.format(count) }} {% endcall %} diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html index 0e3f386b4..9d3586903 100644 --- a/app/templates/views/dashboard/_inbox.html +++ b/app/templates/views/dashboard/_inbox.html @@ -1,21 +1,20 @@ {% from "components/big-number.html" import big_number, big_number_with_status %} +{% from "components/message-count-label.html" import message_count_label %}
{% if inbound_sms_summary != None %} -
- {{ - big_number_with_status( - inbound_sms_summary.count, - 'text messages received', - link=url_for('.inbox', service_id=current_service.id), - show_failures=False - ) - }} -
- {% if inbound_sms_summary.most_recent %} +
{% endif %}
diff --git a/app/templates/views/dashboard/_totals.html b/app/templates/views/dashboard/_totals.html index 778862a4c..b979d9b18 100644 --- a/app/templates/views/dashboard/_totals.html +++ b/app/templates/views/dashboard/_totals.html @@ -3,7 +3,7 @@
-
+
{{ big_number_with_status( statistics['email']['requested'], message_count_label(statistics['email']['requested'], 'email', suffix='sent'), @@ -12,10 +12,10 @@ statistics['email']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='email', status='failed'), link=url_for(".view_notifications", service_id=service_id, message_type='email', status='sending,delivered,failed'), - smaller=smaller_font_size + smaller=True, ) }}
-
+
{{ big_number_with_status( statistics['sms']['requested'], message_count_label(statistics['sms']['requested'], 'sms', suffix='sent'), @@ -24,10 +24,10 @@ statistics['sms']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='failed'), link=url_for(".view_notifications", service_id=service_id, message_type='sms', status='sending,delivered,failed'), - smaller=smaller_font_size + smaller=True, ) }}
-
+
{{ big_number_with_status( statistics['letter']['requested'], message_count_label(statistics['letter']['requested'], 'letter', suffix='sent'), @@ -36,7 +36,7 @@ statistics['letter']['show_warning'], failure_link=url_for(".view_notifications", service_id=service_id, message_type='letter', status='failed'), link=url_for(".view_notifications", service_id=service_id, message_type='letter', status=''), - smaller=smaller_font_size + smaller=True, ) }}
diff --git a/app/templates/views/dashboard/_upcoming.html b/app/templates/views/dashboard/_upcoming.html index 95b0572f7..24894ec78 100644 --- a/app/templates/views/dashboard/_upcoming.html +++ b/app/templates/views/dashboard/_upcoming.html @@ -36,7 +36,6 @@ ) }} {% endcall %} {% endcall %} - {{ show_more() }}
{% endif %}
diff --git a/app/templates/views/dashboard/_usage.html b/app/templates/views/dashboard/_usage.html index 6dc53b630..89a9b33d6 100644 --- a/app/templates/views/dashboard/_usage.html +++ b/app/templates/views/dashboard/_usage.html @@ -1,12 +1,12 @@ {% from "components/big-number.html" import big_number %}
-
+
{{ big_number("Unlimited", 'free email allowance', smaller=True) }}
-
+
{% if sms_chargeable %} {{ big_number( @@ -20,7 +20,7 @@ {% endif %}
-
+
{{ big_number( letter_cost, diff --git a/app/templates/views/dashboard/all-template-statistics.html b/app/templates/views/dashboard/all-template-statistics.html index 202e3bc9c..922aaebb3 100644 --- a/app/templates/views/dashboard/all-template-statistics.html +++ b/app/templates/views/dashboard/all-template-statistics.html @@ -28,7 +28,7 @@ No messages sent

{% else %} -
+
{% call(item, row_number) list_table( month.templates_used, caption=month.name, @@ -41,8 +41,8 @@ field_headings_visible=False ) %} {% call row_heading() %} - {{ item.name }} - + {{ item.name }} + {{ message_count_label(1, item.type, suffix='template')|capitalize }} {% endcall %} diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index cb3554afa..019a2a6d0 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -6,39 +6,41 @@
{% if template_statistics|length > 1 %} -
+
{% call(item, row_number) list_table( template_statistics, - caption="Templates used", - caption_visible=False, + caption="By template", + caption_visible=True, empty_message='', field_headings=[ 'Template', 'Messages sent' ], - field_headings_visible=True + field_headings_visible=False ) %} {% call row_heading() %} {% if item.is_precompiled_letter %} - + Provided as PDF - + Letter {% else %} - {{ item.template_name }} - + {{ item.template_name }} + {{ message_count_label(1, item.template_type, suffix='template')|capitalize }} {% endif %} {% endcall %} + {{ spark_bar_field(item.count, most_used_template_count, id=item.template_id) }} {% endcall %} {{ show_more( url_for('.template_usage', service_id=current_service.id), - 'See templates used by month' + 'See templates used by month', + with_border=False ) }}
{% endif %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 440042f8e..fd95ee7b9 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -222,11 +222,11 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_messages( 'main.service_dashboard', service_id=SERVICE_ONE_ID, ) - + banner = page.select_one('a.banner-dashboard') assert normalize_spaces( - page.select('.big-number-meta-wrapper')[0].text - ) == '99 text messages received latest message just now' - assert page.select('.big-number-meta-wrapper a')[0]['href'] == url_for( + banner.text + ) == '9,999 text messages received latest message just now' + assert banner['href'] == url_for( 'main.inbox', service_id=SERVICE_ONE_ID ) @@ -248,9 +248,9 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_no_messages( 'main.service_dashboard', service_id=SERVICE_ONE_ID, ) - - assert normalize_spaces(page.select('.big-number-meta-wrapper')[0].text) == '0 text messages received' - assert page.select('.big-number-meta-wrapper a')[0]['href'] == url_for( + banner = page.select_one('a.banner-dashboard') + assert normalize_spaces(banner.text) == '0 text messages received' + assert banner['href'] == url_for( 'main.inbox', service_id=SERVICE_ONE_ID ) @@ -553,7 +553,7 @@ def test_should_not_show_recent_templates_on_dashboard_if_only_one_template_used expected_count = stats[0]['count'] assert expected_count == 50 assert normalize_spaces( - page.select_one('#total-sms .big-number').text + page.select_one('#total-sms .big-number-smaller').text ) == ( '{} text messages sent'.format(expected_count) ) @@ -713,14 +713,13 @@ def test_should_show_upcoming_jobs_on_dashboard( ['email', 'sms'], ['email', 'sms', 'letter'], )) -@pytest.mark.parametrize('totals, big_number_class', [ +@pytest.mark.parametrize('totals', [ ( { 'email': {'requested': 0, 'delivered': 0, 'failed': 0}, 'sms': {'requested': 99999, 'delivered': 0, 'failed': 0}, 'letter': {'requested': 99999, 'delivered': 0, 'failed': 0} }, - '.big-number', ), ( { @@ -728,7 +727,6 @@ def test_should_show_upcoming_jobs_on_dashboard( 'sms': {'requested': 0, 'delivered': 0, 'failed': 0}, 'letter': {'requested': 100000, 'delivered': 0, 'failed': 0}, }, - '.big-number-smaller', ), ]) def test_correct_font_size_for_big_numbers( @@ -743,7 +741,6 @@ def test_correct_font_size_for_big_numbers( service_one, permissions, totals, - big_number_class, ): service_one['permissions'] = permissions @@ -758,11 +755,12 @@ def test_correct_font_size_for_big_numbers( service_id=service_one['id'], ) - assert len(page.select_one('[data-key=totals]').select('.column-third')) == 3 - assert len(page.select_one('[data-key=usage]').select('.column-third')) == 3 - - assert len( - page.select('.big-number-with-status {}'.format(big_number_class)) + assert ( + len(page.select_one('[data-key=totals]').select('.column-third')) + ) == ( + len(page.select_one('[data-key=usage]').select('.column-third')) + ) == ( + len(page.select('.big-number-with-status .big-number-smaller')) ) == 3 diff --git a/tests/conftest.py b/tests/conftest.py index 6335f0c17..dfa844724 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1960,7 +1960,7 @@ def mock_get_inbound_sms_summary(mocker): service_id, ): return { - 'count': 99, + 'count': 9999, 'most_recent': datetime.utcnow().isoformat() }