Auto data session 4 #340

joelanman · 2017-01-11T12:16:50Z

this updates #214 Auto data session 3 so it can be merged - copied from that PR:

This version of automatically storing and showing data across pages does not rely on macros, but instead using the checked function directly (see examples below). I'm not sure it's as user-friendly as the version with macros, but it may take a while to finalise our approach to macros.

Usage

This will make the kit automatically store all data from forms, for use later in your prototype.

For example if you have an input like this:

<input name="first-name">

You can put this on the next page, or any other page:

<p>Hello {{data['first-name']}}</p>

Population

To populate a text field with the value a user entered is simple:

<input name="first-name" value="{{data['first-name']}}">

To set radios or checkboxes use this:

{{ checked(name, value) }}

Using the name and value of the input. For example:

<input type="radio" name="over-18" value="Yes" {{checked("over-18", "Yes")}}>

<input type="checkbox" name="send-spam" value="No" {{checked("send-spam", "No")}}>

Dealing with checkboxes

This adds javascript so that when a user submits a form, the script looks for checkboxes in that form. For any checkbox, or group of checkboxes with the same name attribute, it adds a hidden input of the same name, with value "_unchecked". If the server only receives "_unchecked", it clears any data associated with that name.

joelanman · 2017-01-12T14:46:51Z

Important note - this only works with POST. Do we want it on GET too?

joelanman · 2017-01-12T21:55:14Z

Problem: HTML allows hyphens in the name attribute, but a hyphen cannot exist in a variable name in javascript, and therefore Nunjucks.

Solutions:

Convention - say that hyphens are not allowed in the name attribute in prototypes
Change the output syntax from

{{ date-of-birth }} // this is invalid for hyphenated names

to

{{ data['date-of-birth'] }} // this is valid for all names

charlesrt · 2017-01-24T13:32:18Z

In the example the checkbox data passes when only one is checked.

If two are checked, the value passed is empty "".

If all three are checked, only the first value is passed.

joelanman · 2017-01-24T15:10:03Z

@charlesrt thanks, that sounds well broken, investigating..

chrisadesign · 2017-02-17T15:25:42Z

Using this, it's awesome. Please merge :)

joelanman · 2017-02-17T18:08:40Z

@charlesrt I think thats fixed now - I wasn't checking if "_unchecked" was being submitted, and instead just removing elements of arrays - this is now behind a proper if statement. Oops

joelanman · 2017-03-27T16:04:01Z

I've changed the way you display data

Old

{{name}}

New

{{data['name']}}

This means hyphenated names work, where they were broken before.

I think this PR is ready to go, it's worth waiting for #365 as there's a Check your answers page in this PR that should use the new code.

edwardhorsford

Adding a load of stylistic and small points.

I'm in the process of recreating an existing prototype to test functionality - will add another comment tomorrow.

Other things:

I don't think it's clear how you pre-fill answers - this could be explicitly called out.
Should we mention to use on server side?
Should we mention how to not store data?

For those three, I wonder if the guide should have a final tutorial page that does goes over these items?

edwardhorsford · 2017-03-30T10:07:48Z

docs/views/examples/pass-data/vehicle-features.html

+          <fieldset>
+
+            <legend class="visuallyhidden">
+              Which of these applies to your vehicle? (Select all that apply)


I think '(Select all that apply)' should be on a new line in regular body copy style.

edwardhorsford · 2017-03-30T10:09:51Z

docs/views/examples/pass-data/index.html

+        <div class="form-group">
+          <fieldset>
+
+            <label class="form-label" for="registration-number">Vehicle registration number</label>


Suggest this be form-label-bold - better matches our other examples, and helps it stand out on this page.

edwardhorsford · 2017-03-30T10:12:32Z

docs/views/examples/pass-data/vehicle-type.html

+            <p class="heading-medium">
+              What type of vehicle do you have?
+            </p>
+


I think you could add a paragraph here that describes that these radios have name="vehicle-type", and that this will be used later. Matches better to the check your answers page that uses the value.

edwardhorsford · 2017-03-30T10:13:37Z

docs/views/examples/pass-data/vehicle-features.html

+              Which of these applies to your vehicle? (Select all that apply)
+            </legend>
+
+            <p class="heading-medium">


Think this should be an h1, and probably placed outside of the fieldset.

edwardhorsford · 2017-03-30T10:13:53Z

docs/views/examples/pass-data/vehicle-features.html

+            <p class="heading-medium">
+              Which of these applies to your vehicle? (Select all that apply)
+            </p>
+


I think you could add a paragraph here that describes that these checkboxes have name="vehicle-features", and that this will be used later. Matches better to the check your answers page that uses the value.

edwardhorsford · 2017-03-30T10:54:45Z

server.js

+  app.use(utils.forceHttps)
+}
+
+console.log({'autodata': useAutoStoreData})


Is this intentionally here? It outputs before the normal welcome messages.

edwardhorsford · 2017-03-30T10:56:18Z

server.js

+
+console.log({'autodata': useAutoStoreData})
+
+if (useAutoStoreData === 'true') {


I wonder if we should convert useAutoStoreData to boolean as we do with useDocumentation on line 30? We're not very consistent, so if you don't want to do now, that's fine.

edwardhorsford · 2017-03-30T10:59:08Z

server.js

+  app.use(function (req, res, next) {
+    // add nunjucks function to get values, needs to be here as they need access to req.session
+
+    nunjucksAppEnv.addGlobal('checked', function (name, value) {


Could this be pulled out as a function so it doesn't get duplicated for both apps?

edwardhorsford · 2017-03-30T11:03:07Z

server.js

+    next()
+  })
+
+  documentationApp.use(function (req, res, next) {


Should this check if documentationApp exists?

documentationApp currently always exists - its declared at the top of the file, we might want to change that

The app exists, but routes aren't set up for it.

Later in server.js we use if (useDocumentation) {} - do you think something similar would be appropriate here?

I don't have a strong preference here - if you think it's not needed that's fine.

edwardhorsford · 2017-03-30T11:31:38Z

docs/views/examples/pass-data/index.html

+          <fieldset>
+
+            <label class="form-label" for="registration-number">Vehicle registration number</label>
+            <input class="form-control" id="registration-number" name="vehicle-registration" type="text" value="{{vehicleRegistration}}">


Should the id match the name?

edwardhorsford · 2017-03-31T14:44:29Z

I've recreated an existing prototype, and all looks good! Have successfully tested:

Storing, retrieving, transforming data.
Prefilling answers, inc radios and checkboxes.
That submitting no checked items works.
Clearing data
Using _thing to not store data

Some further comments:

Seems to inconsistently use data['thing'] and data["thing"]
The checked function could have a comment to say what it's doing.

I'm not confident enough to read the javascript in server.js, server.js and auto-store-data.js. Perhaps @NickColley or @tvararu could spare 15 minutes to check those files.

tvararu · 2017-03-31T15:44:31Z

server.js

+        return inArray ? 'checked' : ''
+      } else {
+        return value === storedValue ? 'checked' : ''
+      }


If you want to refactor the 2 ternaries here you can try this:

var isChecked = false if (Array.isArray(storedValue)) { isChecked = storedValue.indexOf(value) !== -1 } else { isChecked = value === storedValue } return isChecked ? 'checked' : ''

Or terser:

var isChecked = Array.isArray(storedValue) ? storedValue.indexOf(value) !== -1 : value === storedValue return isChecked ? 'checked' : ''

(Just a thought, feel free to ignore)

thanks, I had another bash taking your first as a base, and ended up with this:

var checked = '' // if data is an array, check it exists in the array if (Array.isArray(storedValue)) { if (storedValue.indexOf(value) !== -1){ checked = 'checked' } } else { // the data is just a simple value, check it matches if (storedValue === value){ checked = 'checked' } } return checked

trying to avoid ternaries since they can be hard to read

tvararu · 2017-03-31T15:48:09Z

lib/utils.js

+
+exports.autoStoreData = function (req, res, next) {
+  if (!req.session.data) {
+    req.session.data = {}


We can use this common pattern too:

req.session.data = req.session.data || {}

joelanman · 2017-04-03T15:15:37Z

Note this now supports both POST and GET

edwardhorsford · 2017-04-03T15:43:18Z

docs/views/examples/pass-data/index.html

+      </p>
+
+      <p>
+        The code for the example is in this folder in the prototype kit:


Suggest this be pulled out in to a section like the other examples below.

edwardhorsford · 2017-04-03T15:46:20Z

docs/views/examples/pass-data/index.html

+      </pre>
+
+      <h3 class="heading-small">
+        Setting inputs


This heading feels jargony. What about something like "Pre-filling answers"

edwardhorsford · 2017-04-04T16:40:20Z

I think this is 👍 to merge.

Amazing work. Thanks so much @joelanman for this. It's a massive improvement to the kit.

timpaul · 2017-04-05T19:33:00Z

AMAZING WORK JOE

And Ed. And everyone. But mainly Joe.

joelanman added the in progress label Jan 11, 2017

This was referenced Feb 17, 2017

Auto data storage UKHomeOffice/home_office_design_kit#1

Closed

Auto data storage UKHomeOffice/home_office_design_kit#2

Merged

joelanman force-pushed the auto-data-session-4 branch from 57190bd to b4856ae Compare February 17, 2017 18:05

joelanman mentioned this pull request Feb 21, 2017

add 'check eligibility cost time' pattern #359

Closed

joelanman force-pushed the auto-data-session-4 branch from d906e59 to 1d04e8c Compare March 21, 2017 14:53

joelanman mentioned this pull request Mar 25, 2017

Auto data session 3 (do not merge) #214

Closed

joelanman force-pushed the auto-data-session-4 branch from 1d04e8c to 382f773 Compare March 27, 2017 15:57

edwardhorsford suggested changes Mar 30, 2017

View reviewed changes

tvararu reviewed Mar 31, 2017

View reviewed changes

joelanman force-pushed the auto-data-session-4 branch 2 times, most recently from 6bfd69d to 6ba6583 Compare April 3, 2017 14:48

joelanman force-pushed the auto-data-session-4 branch 3 times, most recently from 8f96526 to b155c61 Compare April 3, 2017 15:42

edwardhorsford reviewed Apr 3, 2017

View reviewed changes

joelanman force-pushed the auto-data-session-4 branch 2 times, most recently from c67f8ec to 610e42d Compare April 3, 2017 15:45

edwardhorsford reviewed Apr 3, 2017

View reviewed changes

joelanman added 2 commits April 3, 2017 16:47

add auto data store

8c3203d

add clear data link to docs app

b119452

joelanman added 6 commits April 3, 2017 16:47

only run autodata once, include JS in docs

474a6f8

use 'data' hash in locals

0f4cae3

use new multiple-choice css, use hyphens in names

49637a2

store GET data

3fed845

seperate guidance and example pages, added h1s

1df56c6

refactor checked function

825b7a7

joelanman force-pushed the auto-data-session-4 branch from 610e42d to 825b7a7 Compare April 3, 2017 15:47

joelanman added 3 commits April 3, 2017 16:57

use list for vehicle features

de09e98

fix return to examples link

1f0cea9

refactored docs

330b421

edwardhorsford mentioned this pull request Apr 3, 2017

Add template pages for content and questions #369

Merged

joelanman force-pushed the auto-data-session-4 branch from edbd8c7 to 51a0866 Compare April 4, 2017 14:42

set cookie name and timeout, use crypto

34ce07c

joelanman force-pushed the auto-data-session-4 branch from 51a0866 to 34ce07c Compare April 4, 2017 15:52

de-duplicate forceHttps

047065a

joelanman force-pushed the auto-data-session-4 branch from 2bb5fa2 to 047065a Compare April 4, 2017 16:29

edwardhorsford approved these changes Apr 4, 2017

View reviewed changes

joelanman merged commit 867b9ca into master Apr 4, 2017

joelanman deleted the auto-data-session-4 branch April 4, 2017 16:40

joelanman removed the in progress label Apr 4, 2017

joelanman mentioned this pull request Apr 5, 2017

v6.0.0 #370

Merged

joelanman mentioned this pull request Aug 10, 2021

Unchecked items not removed from data nhsuk/nhsuk-prototype-kit#198

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto data session 4 #340

Auto data session 4 #340

joelanman commented Jan 11, 2017 •

edited

Loading

joelanman commented Jan 12, 2017

joelanman commented Jan 12, 2017

charlesrt commented Jan 24, 2017

joelanman commented Jan 24, 2017

chrisadesign commented Feb 17, 2017

joelanman commented Feb 17, 2017

joelanman commented Mar 27, 2017

edwardhorsford left a comment

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

edwardhorsford Mar 30, 2017

joelanman Apr 3, 2017

edwardhorsford Apr 3, 2017

edwardhorsford Mar 30, 2017

edwardhorsford commented Mar 31, 2017

tvararu Mar 31, 2017

joelanman Apr 3, 2017 •

edited

Loading

joelanman Apr 3, 2017

tvararu Mar 31, 2017

joelanman commented Apr 3, 2017

edwardhorsford Apr 3, 2017

edwardhorsford Apr 3, 2017

edwardhorsford commented Apr 4, 2017

timpaul commented Apr 5, 2017


		console.log({'autodata': useAutoStoreData})

		if (useAutoStoreData === 'true') {

Auto data session 4 #340

Auto data session 4 #340

Conversation

joelanman commented Jan 11, 2017 • edited Loading

Usage

Population

Dealing with checkboxes

joelanman commented Jan 12, 2017

joelanman commented Jan 12, 2017

charlesrt commented Jan 24, 2017

joelanman commented Jan 24, 2017

chrisadesign commented Feb 17, 2017

joelanman commented Feb 17, 2017

joelanman commented Mar 27, 2017

edwardhorsford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardhorsford commented Mar 31, 2017

Choose a reason for hiding this comment

joelanman Apr 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelanman commented Apr 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edwardhorsford commented Apr 4, 2017

timpaul commented Apr 5, 2017

AMAZING WORK JOE

joelanman commented Jan 11, 2017 •

edited

Loading

joelanman Apr 3, 2017 •

edited

Loading