From 2d7ed4d34fbfcee6e2686e6f802b23b8207843ce Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 9 Sep 2016 12:25:39 +0100 Subject: [PATCH 1/5] Fix overlapping text on tables of notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit changes the tables of notifications from 3 columns to two columns. This is so the text has more room, so it doesn’t start overlapping. It also makes sure that if the recipient gets really long that it will be cut off with an ellipsis, rather than overlapping… I hypothesize that if a notification fails you probably don’t care when it failed, just that it failed. --- app/__init__.py | 9 +++++++++ app/assets/stylesheets/components/table.scss | 3 ++- app/templates/partials/jobs/notifications.html | 11 +++++------ app/templates/views/notifications.html | 18 ++++++++++-------- tests/app/main/views/test_jobs.py | 2 +- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 4eacdff6b..62ef89f19 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -116,6 +116,7 @@ def create_app(): application.add_template_filter(format_date_normal) application.add_template_filter(format_date_short) application.add_template_filter(format_notification_status) + application.add_template_filter(format_notification_status_with_time) application.add_template_filter(format_notification_status_as_field_status) application.add_template_filter(format_notification_status_as_url) @@ -281,6 +282,14 @@ def format_notification_status(status, template_type): }.get(template_type).get(status, status) +def format_notification_status_with_time(status, template_type, when): + return { + 'delivered': 'Delivered {}'.format(when), + 'sending': 'Sending since {}'.format(when), + 'created': 'Sending since {}'.format(when) + }.get(status, format_notification_status(status, template_type)) + + def format_notification_status_as_field_status(status): return { 'failed': 'error', diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 1d6c3bbeb..baa5551b5 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -40,7 +40,8 @@ width: 52.5%; font-weight: normal; - .hint { + .hint, + p { white-space: nowrap; overflow: hidden; text-overflow: ellipsis; diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 266ad75ff..4cc539c87 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -43,17 +43,13 @@ empty_message="No messages to show", field_headings=[ 'Recipient', - 'Time', 'Status' ], field_headings_visible=False ) %} {% call row_heading() %} - {{ item.to }} +

{{ item.to }}

{% endcall %} - {{ date_field( - (item.updated_at or item.created_at)|format_datetime_short - ) }} {% call field( align='right', status=item.status|format_notification_status_as_field_status @@ -61,7 +57,10 @@ {% if item.status|format_notification_status_as_url %} {% endif %} - {{ item.status|format_notification_status(item.template.template_type) }} + {{ item.status|format_notification_status_with_time( + item.template.template_type, + (item.updated_at or item.created_at)|format_datetime_short + ) }} {% if item.status|format_notification_status_as_url %} {% endif %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 50445772e..082cea952 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -1,5 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, date_field %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, notification_status_field %} {% from "components/previous-next-navigation.html" import previous_next_navigation %} {% from "components/page-footer.html" import page_footer %} {% from "components/pill.html" import pill %} @@ -50,7 +50,7 @@ caption="Recent activity", caption_visible=False, empty_message='No messages found', - field_headings=['Recipient', 'Started', 'Status'], + field_headings=['Recipient', 'Status'], field_headings_visible=False ) %} @@ -68,15 +68,17 @@

{% endcall %} - {{ date_field( - (item.updated_at or item.created_at)|format_datetime_short - ) }} - - {% call field(status=item.status|format_notification_status_as_field_status, align='right') %} + {% call field( + align='right', + status=item.status|format_notification_status_as_field_status + ) %} {% if item.status|format_notification_status_as_url %} {% endif %} - {{ item.status|format_notification_status(item.template.template_type) }} + {{ item.status|format_notification_status_with_time( + item.template.template_type, + (item.updated_at or item.created_at)|format_datetime_short + ) }} {% if item.status|format_notification_status_as_url %} {% endif %} diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 4d9f7de52..c17c3f3fc 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -78,7 +78,7 @@ def test_should_show_page_for_one_job( '{}: Your vehicle tax is about to expire'.format(service_one['name']) ) assert ' '.join(page.find('tbody').find('tr').text.split()) == ( - '07123456789 1 January at 11:10am Delivered' + '07123456789 Delivered 1 January at 11:10am' ) assert page.find('div', {'data-key': 'notifications'})['data-resource'] == url_for( 'main.view_job_updates', From 17d5af1a6e8c6a0827d546a04ebbf4ff6211b72d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 9 Sep 2016 15:36:07 +0100 Subject: [PATCH 2/5] Show delivery time even for failed notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We reckon it’s probably still useful. --- app/__init__.py | 11 +++++------ app/assets/stylesheets/components/table.scss | 7 +++++++ app/templates/partials/jobs/notifications.html | 10 +++++++--- app/templates/views/notifications.html | 11 ++++++++--- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 62ef89f19..08821bb56 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -116,7 +116,7 @@ def create_app(): application.add_template_filter(format_date_normal) application.add_template_filter(format_date_short) application.add_template_filter(format_notification_status) - application.add_template_filter(format_notification_status_with_time) + application.add_template_filter(format_notification_status_as_time) application.add_template_filter(format_notification_status_as_field_status) application.add_template_filter(format_notification_status_as_url) @@ -282,12 +282,11 @@ def format_notification_status(status, template_type): }.get(template_type).get(status, status) -def format_notification_status_with_time(status, template_type, when): +def format_notification_status_as_time(status, when): return { - 'delivered': 'Delivered {}'.format(when), - 'sending': 'Sending since {}'.format(when), - 'created': 'Sending since {}'.format(when) - }.get(status, format_notification_status(status, template_type)) + 'sending': ' since {}'.format(when), + 'created': ' since {}'.format(when) + }.get(status, when) def format_notification_status_as_field_status(status): diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index baa5551b5..2fc79ebe0 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -94,6 +94,13 @@ } + .status-hint { + display: block; + font-weight: normal; + color: $red; + margin-top: 5px; + } + } &-yes, diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 4cc539c87..2a8f20220 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -57,13 +57,17 @@ {% if item.status|format_notification_status_as_url %} {% endif %} - {{ item.status|format_notification_status_with_time( - item.template.template_type, - (item.updated_at or item.created_at)|format_datetime_short + {{ item.status|format_notification_status( + item.template.template_type ) }} {% if item.status|format_notification_status_as_url %} {% endif %} + + {{ item.status|format_notification_status_as_time( + (item.updated_at or item.created_at)|format_datetime_short + ) }} + {% endcall %} {% endcall %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 082cea952..6d8760a62 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -75,13 +75,18 @@ {% if item.status|format_notification_status_as_url %} {% endif %} - {{ item.status|format_notification_status_with_time( - item.template.template_type, - (item.updated_at or item.created_at)|format_datetime_short + {{ item.status|format_notification_status( + item.template.template_type ) }} {% if item.status|format_notification_status_as_url %} +
{% endif %} + + {{ item.status|format_notification_status_as_time( + (item.updated_at or item.created_at)|format_datetime_short + ) }} + {% endcall %} {% endcall %} From 95506e7c3b2b23489d2b664977e5d5cc352c9a9e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 9 Sep 2016 15:40:27 +0100 Subject: [PATCH 3/5] Refactor notification status into its own field type --- app/templates/components/table.html | 20 ++++++++++++++++ .../partials/jobs/notifications.html | 22 ++---------------- app/templates/views/notifications.html | 23 ++----------------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 21a73f185..8f0453012 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -104,3 +104,23 @@ {% macro hidden_field_heading(text) %} {{ text }} {%- endmacro %} + + +{% macro notification_status_field(notification) %} + {% call field(status=notification.status|format_notification_status_as_field_status, align='right') %} + {% if notification.status|format_notification_status_as_url %} + + {% endif %} + {{ notification.status|format_notification_status( + notification.template.template_type + ) }} + {% if notification.status|format_notification_status_as_url %} + + {% endif %} + + {{ notification.status|format_notification_status_as_time( + (notification.updated_at or notification.created_at)|format_datetime_short + ) }} + + {% endcall %} +{% endmacro %} diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 2a8f20220..d75b8310d 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -1,4 +1,4 @@ -{% from "components/table.html" import list_table, field, right_aligned_field_heading, date_field, row_heading %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading, date_field, row_heading, notification_status_field %} {% from "components/page-footer.html" import page_footer %}
@@ -50,25 +50,7 @@ {% call row_heading() %}

{{ item.to }}

{% endcall %} - {% call field( - align='right', - status=item.status|format_notification_status_as_field_status - ) %} - {% if item.status|format_notification_status_as_url %} - - {% endif %} - {{ item.status|format_notification_status( - item.template.template_type - ) }} - {% if item.status|format_notification_status_as_url %} - - {% endif %} - - {{ item.status|format_notification_status_as_time( - (item.updated_at or item.created_at)|format_datetime_short - ) }} - - {% endcall %} + {{ notification_status_field(item) }} {% endcall %} {% if more_than_one_page %} diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index 6d8760a62..c2170c334 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -1,5 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, notification_status_field %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, date_field, notification_status_field %} {% from "components/previous-next-navigation.html" import previous_next_navigation %} {% from "components/page-footer.html" import page_footer %} {% from "components/pill.html" import pill %} @@ -68,26 +68,7 @@

{% endcall %} - {% call field( - align='right', - status=item.status|format_notification_status_as_field_status - ) %} - {% if item.status|format_notification_status_as_url %} - - {% endif %} - {{ item.status|format_notification_status( - item.template.template_type - ) }} - {% if item.status|format_notification_status_as_url %} - -
- {% endif %} - - {{ item.status|format_notification_status_as_time( - (item.updated_at or item.created_at)|format_datetime_short - ) }} - - {% endcall %} + {{ notification_status_field(item) }} {% endcall %} {% if notifications %} From 6c961cc7927f12c5dcffe127f0663c501a273b4f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 9 Sep 2016 15:41:32 +0100 Subject: [PATCH 4/5] Remove date table field type No longer used anywhere. --- app/assets/stylesheets/components/table.scss | 4 ---- app/templates/components/table.html | 6 ------ app/templates/partials/jobs/notifications.html | 2 +- app/templates/views/notifications.html | 2 +- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 2fc79ebe0..0f4bb7292 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -122,10 +122,6 @@ width: 15px; } - &-date { - white-space: nowrap; - } - p { margin: 0 0 5px 0; } diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 8f0453012..a97783247 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -67,12 +67,6 @@ {%- endmacro %} -{% macro date_field(text) -%} - - {{ text }} - -{% endmacro %} - {% macro text_field(text, status='') -%} {% call field(status=status) %} {{ text }} diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index d75b8310d..408073a39 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -1,4 +1,4 @@ -{% from "components/table.html" import list_table, field, right_aligned_field_heading, date_field, row_heading, notification_status_field %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading, row_heading, notification_status_field %} {% from "components/page-footer.html" import page_footer %}
diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index c2170c334..c8b677127 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -1,5 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, date_field, notification_status_field %} +{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, notification_status_field %} {% 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 e9d230b9a48a236b33b91d1707a334799468d672 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 9 Sep 2016 15:57:05 +0100 Subject: [PATCH 5/5] Always show created time for sending state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a notification is in `created` or `sending` the thing you care about is how long it’s been sitting there. --- app/__init__.py | 8 ++++---- app/templates/components/table.html | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 08821bb56..a73b20125 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -282,11 +282,11 @@ def format_notification_status(status, template_type): }.get(template_type).get(status, status) -def format_notification_status_as_time(status, when): +def format_notification_status_as_time(status, created, updated): return { - 'sending': ' since {}'.format(when), - 'created': ' since {}'.format(when) - }.get(status, when) + 'sending': ' since {}'.format(created), + 'created': ' since {}'.format(created) + }.get(status, updated) def format_notification_status_as_field_status(status): diff --git a/app/templates/components/table.html b/app/templates/components/table.html index a97783247..97783ff93 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -113,6 +113,7 @@ {% endif %} {{ notification.status|format_notification_status_as_time( + notification.created_at|format_datetime_short, (notification.updated_at or notification.created_at)|format_datetime_short ) }}