Show one field at a time on send yourself a test #1277

quis · 2017-05-17T10:39:15Z

Screenshots

Before

After

Background on the Send yourself a test feature

We built it because it’s useful for:

constructing an email/text message/letter without uploading a CSV file
seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF)
learning the concept of placeholders, ie understanding they’re thing that gets populated with stuff

The problem with it

The current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address.

Why we have this problem

The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach both:

replacement of placeholders
link between placeholders and spreadsheet columns

The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side.

Why this solution

Showing them one-at-a-time works well because:

it’s really obvious, even on first use, what the page is asking you to do
as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition

leohemsted

some visible 500s.

think the tests are pretty good but my eyes kinda glazed over halfway through

leohemsted · 2017-05-18T10:09:35Z

app/main/forms.py

@@ -1,6 +1,7 @@
+import copy


unused import

(also from flask_login import current_user below)

Removed the copy and squashed into 8c03feb.

Removed other unused import in 5da2428.

leohemsted · 2017-05-18T10:20:03Z

app/main/views/send.py

+    ))
+
+
+@main.route("/services/<service_id>/send/<template_id>/test/step-<int:step_index>", methods=['GET', 'POST'])


You get a 500 when you get to the end if you manually navigate to a /step-1 or higher URL, but didn't fill in the previous steps

You also get a 500 if you navigate to /step-99999 or another number greater than the amount of placeholders

You also get a 500 if you manually navigate to a later step, even if the previous steps that you didn't fill in were all optional (ie: you fill in addr line 1 + 2, then manually go to /step-6 to fill in the postcode)

Think I’ve addressed all of those in 2f8e3e1

leohemsted · 2017-05-19T12:19:54Z

tests/app/main/views/test_send.py

+    mock_has_permissions,
+    mock_get_users_by_service,
+    mock_get_detailed_service_for_today,
+    fake_uuid


there's definitely a whole bunch of fixtures here that we don't need - i'll leave it up to you to decide whether you can be bothered going and excising them. but i think that we only need logged_in_client, fake_uuid and mock_get_service_template out of these?

[and in other tests in this file i assume]

Fixtures addressed in 4072658

The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.

You might need to scroll this page quite a lot to see where a placeholder appears in your template – especially if you have a long email or letter. One of the things I’m trying to stop happening so much is a lot of scrolling back and forth. This would happen if you were scrolling down to see the placeholder, then back up to fill in its value. So this commit makes the textbox ‘sticky’, ie it always stays at the top of the viewport, even when you scroll down. This lets you see the placeholder and the textbox side by side, no matter how long the template is. The code to do this mostly comes from the GOV.UK Frontend Toolkit (documented here: https://github.com/alphagov/govuk_frontend_toolkit/blob/d9489a987086471fe30b4b925a81c12cd198c91d/docs/javascript.md#stick-at-top-when-scrolling). I had to add some extra CSS to make it look good when it overlaps the content of the page, which the GOV.UK Frontend Toolkit implementation doesn’t really anticipate.

People are going to hammer through this form _fast_, so not making them click into the form field on every page load is a nice enhancement. Reuses the code written to do this on the page where you enter your verification code.

If we don’t do this then you can’t see where in the composed message the value for your placeholder will appear.

Calculating the number of pages in a letter is quite slow. And the send yourself a test pages need to load _fast_. Since filling in placeholders is very unlikely to change the number of pages in the resultant letter, it’s pretty safe to cache that count, and makes the subsequent pages load a lot faster.

The combination of `service_one` and `logged_in_client` takes care of most of the permissions stuff.

Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.

Previous implementations of this functionality mutated the base form class, which broke a bunch of stuff. I want to make sure that getting this form for one placeholder doesn’t change other forms that have already been instantiated for other placeholders. Mutation is scary.

quis force-pushed the send-test-page-per-thing branch from 761a15d to 40e2c04 Compare May 17, 2017 16:02

quis mentioned this pull request May 17, 2017

Ideas for how we can deal with templates that have a lot of placeholders #1258

Closed

leohemsted requested changes May 19, 2017

View reviewed changes

quis force-pushed the send-test-page-per-thing branch from 40e2c04 to 5614a43 Compare May 22, 2017 09:49

quis added 6 commits May 22, 2017 10:50

Focus form field on send yourself a test

7169a7f

People are going to hammer through this form _fast_, so not making them click into the form field on every page load is a nice enhancement. Reuses the code written to do this on the page where you enter your verification code.

Unfill current placeholder if revisiting page

16c7d17

If we don’t do this then you can’t see where in the composed message the value for your placeholder will appear.

Remove unused import

5da2428

quis force-pushed the send-test-page-per-thing branch from 5614a43 to 4072658 Compare May 22, 2017 09:53

quis added 3 commits May 22, 2017 12:12

Clean up unused test fixtures

c8a64ae

The combination of `service_one` and `logged_in_client` takes care of most of the permissions stuff.

quis force-pushed the send-test-page-per-thing branch from 5125a27 to 1d76b22 Compare May 22, 2017 11:12

leohemsted approved these changes May 23, 2017

View reviewed changes

quis merged commit 1aebb10 into master May 23, 2017

quis deleted the send-test-page-per-thing branch May 23, 2017 11:45

quis mentioned this pull request Aug 8, 2018

Notify could be simpler for users who only send messages alphagov/notify-design-archive#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show one field at a time on send yourself a test #1277

Show one field at a time on send yourself a test #1277

quis commented May 17, 2017 •

edited

Loading

leohemsted left a comment

leohemsted May 18, 2017

quis May 22, 2017 •

edited

Loading

leohemsted May 18, 2017

leohemsted May 18, 2017

leohemsted May 18, 2017

quis May 22, 2017

leohemsted May 19, 2017

quis May 22, 2017 •

edited

Loading

		))


		@main.route("/services/<service_id>/send/<template_id>/test/step-<int:step_index>", methods=['GET', 'POST'])

Show one field at a time on send yourself a test #1277

Show one field at a time on send yourself a test #1277

Conversation

quis commented May 17, 2017 • edited Loading

Screenshots

Before

After

Background on the Send yourself a test feature

The problem with it

Why we have this problem

Why this solution

leohemsted left a comment

Choose a reason for hiding this comment

leohemsted May 18, 2017

Choose a reason for hiding this comment

quis May 22, 2017 • edited Loading

Choose a reason for hiding this comment

leohemsted May 18, 2017

Choose a reason for hiding this comment

leohemsted May 18, 2017

Choose a reason for hiding this comment

leohemsted May 18, 2017

Choose a reason for hiding this comment

quis May 22, 2017

Choose a reason for hiding this comment

leohemsted May 19, 2017

Choose a reason for hiding this comment

quis May 22, 2017 • edited Loading

Choose a reason for hiding this comment

quis commented May 17, 2017 •

edited

Loading

quis May 22, 2017 •

edited

Loading

quis May 22, 2017 •

edited

Loading