From e5d25148468b491e35f2b235077763891bd36288 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Jun 2016 16:35:03 +0100 Subject: [PATCH] Make template graphs look consistent with page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The graphs of template usage feel a bit weird to me now. 1. They are counts of messages, but the numbers are very small not big like we do everywhere else (eg the counts on a job) 2. There’s a lot of blue, especially for something that you can’t click This commit makes the numbers bigger and the bar chart grey. --- app/assets/stylesheets/views/dashboard.scss | 16 +++++------ app/main/views/jobs.py | 1 + .../views/dashboard/template-statistics.html | 28 ++++++++++++++----- app/templates/views/notifications.html | 2 +- tests/app/main/views/test_dashboard.py | 24 ++++++++++------ tests/app/main/views/test_jobs.py | 16 +++++++---- 6 files changed, 58 insertions(+), 29 deletions(-) diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 6069a8f20..59fa6baca 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -26,16 +26,16 @@ width: 100%; margin-bottom: $gutter-half; height: $gutter-half; - color: $govuk-blue; + color: $text-colour; span { box-sizing: border-box; - display: block; - background: $govuk-blue; - color: $white; - height: $gutter-half; - padding-left: 5px; - padding-right: 5px; + display: inline-block; + overflow: visible; + background: $panel-colour; + color: $black; + padding: 5px 5px 2px 5px; + text-indent: -2px; margin: 3px 0 5px 0; transition: width 0.6s ease-in-out; } @@ -49,7 +49,7 @@ color: $govuk-blue; max-width: 100%; background: $white; - margin-bottom: $gutter; + margin-bottom: $gutter-two-thirds; } } diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 194cb0563..9158004e8 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -26,6 +26,7 @@ from app.utils import ( generate_previous_next_dict, user_has_permissions, generate_notifications_csv) +from app.statistics_utils import sum_of_statistics def _parse_filter_args(filter_dict): diff --git a/app/templates/views/dashboard/template-statistics.html b/app/templates/views/dashboard/template-statistics.html index 80e12fdd7..c0d42104d 100644 --- a/app/templates/views/dashboard/template-statistics.html +++ b/app/templates/views/dashboard/template-statistics.html @@ -1,26 +1,40 @@ {% from "components/message-count-label.html" import message_count_label %} +{% from "components/big-number.html" import big_number %}
{% for item in template_statistics %}
- +
{{ item.template.name }} + + {{ message_count_label(1, item.template.template_type, suffix='template')|capitalize }} +
{% if template_statistics|length > 1 %} - - - {{ item.usage_count }} {{ message_count_label(item.usage_count, item.template.template_type) }} - + + + {{ big_number( + item.usage_count, + smallest=True + ) }} + + {{ message_count_label(item.usage_count, item.template.template_type) }} + + + {% else %} - {{ item.usage_count }} {{ message_count_label(item.usage_count, item.template.template_type) }} + + {{ item.usage_count }} + {{ message_count_label(item.usage_count, item.template.template_type) }} + {% endif %}
{% endfor %} -
\ No newline at end of file + diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 4e9c14622..ce682a7f6 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -3,7 +3,7 @@ {% from "components/previous-next-navigation.html" import previous_next_navigation %} {% from "components/page-footer.html" import page_footer %} {% from "components/pill.html" import pill %} -{% from "components/message-count-label.html" import message_count_label %} +{% from "components/message-count-label.html" import message_count_label, recipient_count_label %} {% block page_title %} {{ message_count_label(99, message_type, suffix='') | capitalize }} – GOV.UK Notify diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index d143166d2..ef5ec8942 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -91,11 +91,15 @@ 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 text messages sent' + assert 'Pickle feet' in page.find_all('dt')[0].text + assert 'Text message template' in page.find_all('dt')[0].text + assert '206' in page.find_all('dd')[0].text + assert 'text messages sent' in page.find_all('dd')[0].text - assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' - assert page.find_all('dd')[1].text.strip() == '13 text messages sent' + assert 'Brine Shrimp' in page.find_all('dt')[1].text + assert 'Text message template' in page.find_all('dt')[1].text + assert '13' in page.find_all('dd')[1].text + assert 'text messages sent' in page.find_all('dd')[1].text def test_should_show_all_templates_on_template_statistics_page( @@ -129,11 +133,15 @@ 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 text messages sent' + assert 'Pickle feet' in page.find_all('dt')[0].text + assert 'Text message template' in page.find_all('dt')[0].text + assert '206' in page.find_all('dd')[0].text + assert 'text messages sent' in page.find_all('dd')[0].text - assert page.find_all('dt')[1].text.strip() == 'Brine Shrimp' - assert page.find_all('dd')[1].text.strip() == '13 text messages sent' + assert 'Brine Shrimp' in page.find_all('dt')[1].text + assert 'Text message template' in page.find_all('dt')[1].text + assert '13' in page.find_all('dd')[1].text + assert 'text messages sent' in page.find_all('dd')[1].text def _test_dashboard_menu(mocker, app_, usr, service, permissions): diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 791183dd7..8d666a2ab 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -32,6 +32,7 @@ def test_should_show_page_for_one_job( service_one, active_user_with_permissions, mock_get_service_template, + mock_get_service_statistics, mock_get_job, mocker, mock_get_notifications, @@ -57,6 +58,7 @@ def test_should_show_updates_for_one_job_as_json( service_one, active_user_with_permissions, mock_get_notifications, + mock_get_service_statistics, mock_get_job, mocker, fake_uuid @@ -112,6 +114,7 @@ def test_can_show_notifications( service_one, active_user_with_permissions, mock_get_notifications, + mock_get_service_statistics, mocker, message_type, page_title, @@ -166,11 +169,14 @@ def test_can_show_notifications( assert 'text/csv' in csv_response.headers['Content-Type'] -def test_should_show_notifications_for_a_service_with_next_previous(app_, - service_one, - active_user_with_permissions, - mock_get_notifications_with_previous_next, - mocker): +def test_should_show_notifications_for_a_service_with_next_previous( + app_, + service_one, + active_user_with_permissions, + mock_get_notifications_with_previous_next, + mock_get_service_statistics, + mocker +): with app_.test_request_context(): with app_.test_client() as client: client.login(active_user_with_permissions, mocker, service_one)