From 1b8236f5f88cee8e3cffa47a41803651ccb42b2f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 11 May 2016 15:05:40 +0100 Subject: [PATCH 1/4] Make template usage on dashboard easier to scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dashboard looked a bit table-y. This commit makes four main changes: - show a bar chart (drawn in CSS) for template usage (only shown if you’ve used more than one template recently) - only break down template usage by template name, not template type (because that’s happening with the big numbers) - change the style of the ‘show more’ links under each section so that they are all consistent, and a little less busy (one less keyline) - remove the ‘recent templates‘ title so that the first two sections of the page group under ‘in the last 7 days’ --- .../stylesheets/components/big-number.scss | 39 +++++++++--------- .../stylesheets/components/show-more.scss | 34 +++++++++++++++ app/assets/stylesheets/main.scss | 1 + app/assets/stylesheets/views/dashboard.scss | 41 +++++++++++++++---- app/main/views/dashboard.py | 17 ++++++-- app/templates/components/big-number.html | 16 ++++---- app/templates/components/show-more.html | 3 ++ .../dashboard/all-template-statistics.html | 21 ++++++---- app/templates/views/dashboard/dashboard.html | 6 ++- .../views/dashboard/template-statistics.html | 41 ++++++++++--------- app/templates/views/dashboard/today.html | 31 ++++++++------ tests/app/main/views/test_dashboard.py | 34 ++++++--------- 12 files changed, 182 insertions(+), 102 deletions(-) create mode 100644 app/assets/stylesheets/components/show-more.scss create mode 100644 app/templates/components/show-more.html diff --git a/app/assets/stylesheets/components/big-number.scss b/app/assets/stylesheets/components/big-number.scss index 3e046c12a..f9d0508be 100644 --- a/app/assets/stylesheets/components/big-number.scss +++ b/app/assets/stylesheets/components/big-number.scss @@ -1,19 +1,29 @@ %big-number, .big-number { - @include bold-48; + display: block; + + &-number { + @include bold-48; + display: block; + } &-label { @include core-19; - display: block; - padding-bottom: $gutter-half; + display: inline-block; + padding-bottom: 10px; } } .big-number-smaller { + @extend %big-number; - @include bold-36($tabular-numbers: true); + + .big-number-number { + @include bold-36($tabular-numbers: true); + } + } .big-number-with-status { @@ -22,25 +32,12 @@ background: $text-colour; color: $white; position: relative; - - &-show-more-link { - @include core-16; - display: block; - padding: 0.75em 0 0.5625em 0; - margin-bottom: $gutter-half; - text-align: center; - border-bottom: 1px solid $border-colour; - - &:focus { - outline: none; - color: $text-colour; - border-bottom: 1px solid $brown; - } - } + margin-bottom: $gutter-half; .big-number { padding: 15px; position: relative; + } .big-number-overlay-link { @@ -66,6 +63,8 @@ &:link, &:visited { color: $white; + text-decoration: none; + border-bottom: 1px solid $white; } } @@ -85,6 +84,8 @@ &:active, &:hover { color: $white; + text-decoration: none; + border-bottom: 1px solid $white; } } diff --git a/app/assets/stylesheets/components/show-more.scss b/app/assets/stylesheets/components/show-more.scss new file mode 100644 index 000000000..a02c4b53d --- /dev/null +++ b/app/assets/stylesheets/components/show-more.scss @@ -0,0 +1,34 @@ +.show-more { + + @include core-16; + display: block; + padding: 0 0; + margin: $gutter-half 0 $gutter 0; + text-align: center; + border-top: 1px solid $border-colour; + + &:focus { + + outline: none; + color: $text-colour; + box-shadow: 0 -10px 0 0 $yellow; + border-color: $yellow; + + span { + background: $yellow; + outline: none; + border-color: $text-colour; + } + + } + + span { + position: relative; + top: -11px; + outline: 10px solid white; + background: $white; + display: inline-block; + border-bottom: 1px solid $light-blue; + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 356110614..eae80027d 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -50,6 +50,7 @@ $path: '/static/images/'; @import 'components/vendor/previous-next-navigation'; @import 'components/pill'; @import 'components/secondary-button'; +@import 'components/show-more'; @import 'views/job'; @import 'views/edit-template'; diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index d4dfb1159..30f26f57d 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -5,16 +5,43 @@ } .keyline-block { - border-top: 1px solid $border-colour; padding-top: $gutter-half; +} - &-show-more-link { - @include core-16; - border-top: 1px solid $border-colour; - border-bottom: 1px solid $border-colour; - padding: 0.75em 0 0.5625em 0; - text-align: center; +.spark-bar { + + box-sizing: border-box; + display: block; + width: 100%; + text-align: right; + margin-bottom: $gutter-half; + height: 30px; + color: $govuk-blue; + line-height: 30px; + padding-right: 5px; + font-weight: bold; + color: $govuk-blue; + + span { + display: block; + float: left; + background: $govuk-blue; + color: $white; + height: 30px; + padding-left: 5px; + padding-right: 5px; + } + + &-label { + display: block; + line-height: 30px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + color: $link-colour; + max-width: 100%; + background: $white; } } diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index b63cc81c8..efbaa1045 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -71,10 +71,14 @@ def service_dashboard_updates(service_id): @login_required @user_has_permissions('view_activity', admin_override=True) def template_history(service_id): + template_statistics = aggregate_usage( + template_statistics_client.get_template_statistics_for_service(service_id) + ) return render_template( 'views/dashboard/all-template-statistics.html', - template_statistics=aggregate_usage( - template_statistics_client.get_template_statistics_for_service(service_id) + template_statistics=template_statistics, + most_used_template_count=max( + [row['usage_count'] for row in template_statistics] or [0] ) ) @@ -197,12 +201,17 @@ def get_dashboard_statistics_for_service(service_id): sms_sent = usage['data'].get('sms_count', 0) emails_sent = usage['data'].get('email_count', 0) + template_statistics = aggregate_usage( + template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) + ) + return { 'statistics': add_rates_to(sum_of_statistics( statistics_api_client.get_statistics_for_service(service_id, limit_days=7)['data'] )), - 'template_statistics': aggregate_usage( - template_statistics_client.get_template_statistics_for_service(service_id, limit_days=7) + 'template_statistics': template_statistics, + 'most_used_template_count': max( + [row['usage_count'] for row in template_statistics] or [0] ), 'emails_sent': emails_sent, 'sms_free_allowance': sms_free_allowance, diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index 654946f1f..b52f98c6f 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -1,14 +1,16 @@ {% macro big_number(number, label, label_link=None, currency='', smaller=False) %}
- {% if number is number %} - {% if currency %} - {{ "{}{:,.2f}".format(currency, number) }} +
+ {% if number is number %} + {% if currency %} + {{ "{}{:,.2f}".format(currency, number) }} + {% else %} + {{ "{}{:,}".format(currency, number) }} + {% endif %} {% else %} - {{ "{}{:,}".format(currency, number) }} + {{ number }} {% endif %} - {% else %} - {{ number }} - {% endif %} +
{% if label_link %} {{ label }} diff --git a/app/templates/components/show-more.html b/app/templates/components/show-more.html new file mode 100644 index 000000000..24b945f81 --- /dev/null +++ b/app/templates/components/show-more.html @@ -0,0 +1,3 @@ +{% macro show_more(url, label) %} + {{ label }} +{% endmacro %} diff --git a/app/templates/views/dashboard/all-template-statistics.html b/app/templates/views/dashboard/all-template-statistics.html index 7ec88eeb6..bb3a7d0dc 100644 --- a/app/templates/views/dashboard/all-template-statistics.html +++ b/app/templates/views/dashboard/all-template-statistics.html @@ -5,13 +5,16 @@ {% endblock %} {% block maincolumn_content %} -

- Templates sent this year -

-

- 1 April 2016 to date -

- {% with period = "" %} - {% include 'views/dashboard/template-statistics.html' %} - {% endwith %} + +
+
+

Templates sent

+
+
+ 1 April 2016 to date +
+
+ + {% include 'views/dashboard/template-statistics.html' %} + {% endblock %} diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index 192700f25..50b00e0a6 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -8,6 +8,8 @@
+

Dashboard

+ {% if not templates and current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], any_=True) %} {% include 'views/dashboard/get-started.html' %} {% elif current_user.has_permissions([ @@ -20,9 +22,9 @@ {% include 'views/dashboard/no-permissions-banner.html' %} {% endif %} -

+

In the last 7 days -

+ {% include 'views/dashboard/today.html' %}
diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index ee69efdd4..89ae52ae3 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -1,23 +1,24 @@ {% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading %} -{% call(item, row_number) list_table( - template_statistics, - caption="Templates", - caption_visible=True, - empty_message='You haven’t used any templates {}'.format(period), - field_headings=['Template', hidden_field_heading('Type'), hidden_field_heading('Messages sent')], - field_headings_visible=False -) %} - {% call field() %} - - {{ item.template.name }} - - {% endcall %} - {% call field(align='right') %} - {{ item.usage_count }} - {% endcall %} - {% call field() %} - {{'text messages sent' if 'sms' == item.template.template_type else 'emails sent'}} - {% endcall %} -{% endcall %} +
+ {% for item in template_statistics %} +
+ +
+ + + {{ item.template.name }} + + +
+
+ + + {{ item.usage_count }} messages sent + +
+ +
+ {% endfor %} +
\ No newline at end of file diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index aa2128c63..859409ba6 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -1,4 +1,5 @@ {% from "components/big-number.html" import big_number, big_number_with_status %} +{% from "components/show-more.html" import show_more %}
{{ big_number_with_status( statistics.emails_requested, - 'email' if statistics.emails_requested == 1 else 'emails', + 'email sent' if statistics.emails_requested == 1 else 'emails sent', statistics.emails_failed, statistics.get('emails_failure_rate', 0.0), statistics.get('emails_failure_rate', 0)|float > 3, @@ -22,7 +23,7 @@
{{ big_number_with_status( statistics.sms_requested, - 'text message' if statistics.sms_requested == 1 else 'text messages', + 'text message sent' if statistics.sms_requested == 1 else 'text messages sent', statistics.sms_failed, statistics.get('sms_failure_rate', 0.0), statistics.get('sms_failure_rate', 0)|float > 3, @@ -31,18 +32,23 @@ ) }}
- Compare to previous weeks + {{ show_more( + url_for('.weekly', service_id=current_service.id), + 'Compare to previous weeks' + ) }}
- {% with period = "in the last 7 days" %} + + {% if template_statistics|length > 1 %} {% include 'views/dashboard/template-statistics.html' %} - {% endwith %} -

- See all templates sent this year -

+ {{ show_more( + url_for('.template_history', service_id=current_service.id), + 'See all templates sent this year' + ) }} + {% endif %} {% if current_user.has_permissions(['manage_settings'], admin_override=True) %} -

Usage

+

This year

@@ -65,9 +71,10 @@
-
- See breakdown -
+ {{ show_more( + url_for(".usage", service_id=current_service['id']), + 'See usage breakdown' + ) }} {% endif %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index dd32cabab..d878c4243 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -86,21 +86,16 @@ def test_should_show_recent_templates_on_dashboard(app_, headers = [header.text.strip() for header in page.find_all('h2') + page.find_all('h1')] assert 'Test Service' in headers assert 'In the last 7 days' in headers - template_usage_headers = [th.text.strip() for th in page.thead.find_all('th')] - for th in ['Template', 'Type', 'Messages sent']: - assert th in template_usage_headers - table_rows = page.tbody.find_all('tr') + + table_rows = page.find_all('dt') assert len(table_rows) == 2 - first_row = page.tbody.find_all('tr')[0] - table_data = first_row.find_all('td') - assert len(table_data) == 3 - assert table_data[1].text.strip() == '206' - second_row = page.tbody.find_all('tr')[1] - table_data = second_row.find_all('td') - assert len(table_data) == 3 - assert table_data[1].text.strip() == '13' + assert page.find_all('dt')[0].text.strip() == 'Pickle feet' + assert page.find_all('dd')[0].text.strip() == '206 messages sent' + + assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' + assert page.find_all('dd')[1].text.strip() == '13 messages sent' def test_should_show_all_templates_on_template_statistics_page( @@ -130,20 +125,15 @@ def test_should_show_all_templates_on_template_statistics_page( mock_template_stats.assert_called_once_with(SERVICE_ONE_ID) page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - headers = [header.text.strip() for header in page.find_all('h2')] - table_rows = page.tbody.find_all('tr') + table_rows = page.find_all('dt') assert len(table_rows) == 2 - first_row = page.tbody.find_all('tr')[0] - table_data = first_row.find_all('td') - assert len(table_data) == 3 - assert table_data[1].text.strip() == '206' + assert page.find_all('dt')[0].text.strip() == 'Pickle feet' + assert page.find_all('dd')[0].text.strip() == '206 messages sent' - second_row = page.tbody.find_all('tr')[1] - table_data = second_row.find_all('td') - assert len(table_data) == 3 - assert table_data[1].text.strip() == '13' + assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' + assert page.find_all('dd')[1].text.strip() == '13 messages sent' def _test_dashboard_menu(mocker, app_, usr, service, permissions): From 3dc349c8a7f98f9447a518b5f312f3f1e73d8f49 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 12 May 2016 21:31:47 +0100 Subject: [PATCH 2/4] Include template type in bar graph label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s possible that users will have email and SMS templates with similar names, and will send them depending on their users’ contact preferences. So it’s useful to be able to compare how many emails vs SMS you’re sending, even if the template names are similar. --- app/assets/stylesheets/views/dashboard.scss | 18 ++++++++++-------- .../views/dashboard/template-statistics.html | 9 ++++----- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 30f26f57d..a2697f7b3 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -11,37 +11,39 @@ .spark-bar { + @include core-16; box-sizing: border-box; display: block; width: 100%; text-align: right; margin-bottom: $gutter-half; - height: 30px; + height: $gutter-half; color: $govuk-blue; - line-height: 30px; - padding-right: 5px; - font-weight: bold; + text-align: left; color: $govuk-blue; span { + box-sizing: border-box; display: block; - float: left; background: $govuk-blue; color: $white; - height: 30px; + height: $gutter-half; padding-left: 5px; padding-right: 5px; + margin: 3px 0 5px 0; + transition: width 0.6s ease-in-out; } &-label { + @include bold-19; display: block; - line-height: 30px; white-space: nowrap; overflow: hidden; text-overflow: ellipsis; - color: $link-colour; + color: $govuk-blue; max-width: 100%; background: $white; + margin-bottom: $gutter; } } diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 89ae52ae3..0af34f4ba 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -6,16 +6,15 @@
- - {{ item.template.name }} - + {{ item.template.name }}
- - {{ item.usage_count }} messages sent + + {{ item.usage_count }} {{ 'text messages' if item.template.template_type == 'sms' else 'emails' }} sent +
From 8a4b0ba88cd128f214bfebf3cdd762a327db4946 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 13 May 2016 09:58:07 +0100 Subject: [PATCH 3/4] Add a message count component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logic to say ‘1 email sent’ vs ‘12 text messages sent’ is repeated all over the place. So this commit adds a component to put it in one place. --- app/templates/components/message-count-label.html | 15 +++++++++++++++ .../views/dashboard/template-statistics.html | 5 ++--- app/templates/views/dashboard/today.html | 5 +++-- tests/app/main/views/test_dashboard.py | 8 ++++---- 4 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 app/templates/components/message-count-label.html diff --git a/app/templates/components/message-count-label.html b/app/templates/components/message-count-label.html new file mode 100644 index 000000000..532a711d5 --- /dev/null +++ b/app/templates/components/message-count-label.html @@ -0,0 +1,15 @@ +{% macro message_count_label(count, template_type) -%} + {%- if template_type == 'sms' -%} + {%- if count == 1 -%} + text message + {%- else -%} + text messages + {%- endif -%} + {%- elif template_type == 'email' -%} + {%- if count == 1 -%} + email + {%- else -%} + emails + {%- endif -%} + {%- endif %} sent +{%- endmacro %} \ No newline at end of file diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 0af34f4ba..1d3f8e2ac 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -1,4 +1,4 @@ -{% from "components/table.html" import list_table, field, hidden_field_heading, right_aligned_field_heading %} +{% from "components/message-count-label.html" import message_count_label %}
{% for item in template_statistics %} @@ -13,8 +13,7 @@
- {{ item.usage_count }} {{ 'text messages' if item.template.template_type == 'sms' else 'emails' }} sent - + {{ item.usage_count }} {{ message_count_label(item.usage_count, item.template.template_type) }}
diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index 859409ba6..be28c2b10 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -1,5 +1,6 @@ {% from "components/big-number.html" import big_number, big_number_with_status %} {% from "components/show-more.html" import show_more %} +{% from "components/message-count-label.html" import message_count_label %}
{{ big_number_with_status( statistics.emails_requested, - 'email sent' if statistics.emails_requested == 1 else 'emails sent', + message_count_label(statistics.emails_requested, 'email'), statistics.emails_failed, statistics.get('emails_failure_rate', 0.0), statistics.get('emails_failure_rate', 0)|float > 3, @@ -23,7 +24,7 @@
{{ big_number_with_status( statistics.sms_requested, - 'text message sent' if statistics.sms_requested == 1 else 'text messages sent', + message_count_label(statistics.sms_requested, 'sms'), statistics.sms_failed, statistics.get('sms_failure_rate', 0.0), statistics.get('sms_failure_rate', 0)|float > 3, diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index d878c4243..2454b63d3 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -92,10 +92,10 @@ def test_should_show_recent_templates_on_dashboard(app_, assert len(table_rows) == 2 assert page.find_all('dt')[0].text.strip() == 'Pickle feet' - assert page.find_all('dd')[0].text.strip() == '206 messages sent' + assert page.find_all('dd')[0].text.strip() == '206 text messages sent' assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' - assert page.find_all('dd')[1].text.strip() == '13 messages sent' + assert page.find_all('dd')[1].text.strip() == '13 text messages sent' def test_should_show_all_templates_on_template_statistics_page( @@ -130,10 +130,10 @@ def test_should_show_all_templates_on_template_statistics_page( assert len(table_rows) == 2 assert page.find_all('dt')[0].text.strip() == 'Pickle feet' - assert page.find_all('dd')[0].text.strip() == '206 messages sent' + assert page.find_all('dd')[0].text.strip() == '206 text messages sent' assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' - assert page.find_all('dd')[1].text.strip() == '13 messages sent' + assert page.find_all('dd')[1].text.strip() == '13 text messages sent' def _test_dashboard_menu(mocker, app_, usr, service, permissions): From c3d78f565251e85e4f4245bd32c69557ed216165 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 13 May 2016 10:11:56 +0100 Subject: [PATCH 4/4] =?UTF-8?q?Show=20template=20stats=20even=20if=20there?= =?UTF-8?q?=E2=80=99s=20only=201=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you’re only ever sending one template it’s really useful to be able to jump straight to that template from the dashboard. So this commit: - shows the template stats even if there’s only one row - hides the bar chart if there’s only one row (because it will always be 100%, and won’t be obvious what it is without its siblings) --- app/templates/views/dashboard/template-statistics.html | 6 +++++- app/templates/views/dashboard/today.html | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 1d3f8e2ac..80e12fdd7 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -11,10 +11,14 @@
+ {% if template_statistics|length > 1 %} - + {{ item.usage_count }} {{ message_count_label(item.usage_count, item.template.template_type) }} + {% else %} + {{ item.usage_count }} {{ message_count_label(item.usage_count, item.template.template_type) }} + {% endif %}
diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index be28c2b10..b3058951e 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -40,7 +40,7 @@
- {% if template_statistics|length > 1 %} + {% if template_statistics|length %} {% include 'views/dashboard/template-statistics.html' %} {{ show_more( url_for('.template_history', service_id=current_service.id),