Allow selecting options by passing current values #2616
Conversation
CHANGELOG.md
Outdated
|
|
||
| This change was introduced in [pull request #2616: Allow selecting options by passing current values](https://github.com/alphagov/govuk-frontend/pull/2616). | ||
|
|
||
| #### Select an option in a Select by using the `selectedValue` option |
There was a problem hiding this comment.
THIS IS A NIGHTMARE. Two of these words have two different meanings and we use both meanings in the same heading.
Select (verb) an option (an <option> HTML element within a <select>) in a Select (component name / HTML element name) using the selectedValue option (a 'Nunjucks macro option')
Suggestions welcome.
There was a problem hiding this comment.
Tricky one alright! Ok, first pass:
Choose an option in a select component by using the
selectedValueparameter
Who is it that does the 'choosing' or 'selecting' of the option? Is the idea here that the service pre-selects something to save the end user time?
There was a problem hiding this comment.
It might be to save the user time, or more likely it might be because the user has gone back to change an answer they previously gave.
For example, if a user was in the middle of a transaction and went 'back' to a previous page that used a select, the answer they previously gave should be selected when the page loads.
Likewise, if the user was checking their answers and decided to change an answer they previously gave, when they click change the page that loads should pre-select the answer that they'd previously given.
|
@EoinShaughnessy the changelog entry for this PR is a bit gnarly – could do with your thoughts on it when you have a mo. There's no rush on it though, I don't think we should try and get this into v4.1.0 so it can wait for the next release. |
|
I've raised a PR with a proof of concept of how this could be extended in the Prototype Kit to allow pre-selecting radios and checkboxes from the |
I'd argue strongly for calling it |
Interesting! I understand your argument but I'd argue that using We also have a principle that says we should mimic HTML attribute names and although it's not explicitly stated there I think it also makes sense to avoid using common HTML attribute names when the option doesn't translate directly to that attribute on any of the elements within the component. (Having said that… we already use Fundamentally… I'm wondering whether we're better off trying to 'abstract away' some of the messiness and inconsistencies in the HTML spec, or to try to expose it and be consistent with it? 🤔 Using
|
36degrees
force-pushed
the
checked-values
branch
3 times, most recently
from
7d78d62
to
c23444f
Compare
There was a problem hiding this comment.
Nice work! This is a bit similar to a filter we use to make the macros simpler.
Note you probably want the ability for a single item to have checked: false - at the moment the logic will overwrite these. Basically - if a single item has a value for checked or selected you should honour that and not override it.
This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected.
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
| @@ -69,7 +69,7 @@ params: | |||
| - name: checked | |||
| type: boolean | |||
| required: false | |||
| description: If `true`, radio will be checked. | |||
| description: If `true`, the radio will be checked when the page loads. | |||
There was a problem hiding this comment.
Same comment on communicating the override
There was a problem hiding this comment.
| description: If `true`, the radio will be checked when the page loads. | |
| description: Whether the radio should be checked when the page loads. If you set this option, the `value` option will not affect the checked state of the radio. |
| @@ -23,7 +23,7 @@ params: | |||
| - name: selected | |||
| type: boolean | |||
| required: false | |||
| description: Sets the option as the selected. | |||
| description: Sets the option as selected when the page loads. | |||
There was a problem hiding this comment.
Same comment on communicating the override
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
At the minute the only way to pre-select or check specific items when using the Nunjucks macros for these components is to pass a boolean flag for each item, which then maps directly to the checked / selected HTML attribute for the <input> / <option>.
This means the user has to do the work to map the previous form data back to a boolean, resulting in code that might look something like this:
```
{{ govukSelect({
id: "sort",
name: "sort",
label: {
text: "Sort by"
},
items: [
{
value: "published",
text: "Recently published",
selected: (data['sort'] == "published")
},
{
value: "updated",
text: "Recently updated",
selected: (data['sort'] == "updated")
},
{
value: "views",
text: "Most views",
selected: (data['sort'] == "views")
},
{
value: "comments",
text: "Most comments",
selected: (data['sort'] == "comments")
}
]
}) }}
```
This is prone to introducing errors (especially when prototyping) – for example, any changes to the name of the field or the value of the item also need corresponding changes made to the boolean logic, potentially in multiple places.
Allow the user to pass the current value (or array of values, for checkboxes), and using that to work out which item(s) should be checked or selected.
The above example can then be done like this:
```
{{ govukSelect({
id: "sort",
name: "sort",
label: {
text: "Sort by"
},
items: [
{
value: "published",
text: "Recently published"
},
{
value: "updated",
text: "Recently updated"
},
{
value: "views",
text: "Most views"
},
{
value: "comments",
text: "Most comments"
}
],
value: data[‘sort’]
}) }}
```
Co-authored-by: EoinShaughnessy <72507742+EoinShaughnessy@users.noreply.github.com>
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
|
I think I've addressed the comments from @owenatgov but it could do with another once over. Then I believe this is ready to merge once v4.1.1 is out. |
We've now decided to skip v4.1.1 and roll everything into v4.2.0, so this can be merged. |
If a checkbox, radio or select option has the checked or selected option set, always use its value to set the checked or selected state, even when it’s falsey. As suggested by @edwardhorsford [1]: > This is helpful when most of the items just need to check if the stored value equals the value, but one or two need more complex logic to determine if they should be selected. [1]: #2616 (review)
At the minute the only way to pre-select or check specific items when using the Nunjucks macros for these components is to pass a boolean flag for each item, which then maps directly to the checked / selected HTML attribute for the / .
This means the user has to do the work to map the previous form data back to a boolean, resulting in code that might look something like this:
This is prone to introducing errors (especially when prototyping) – for example, any changes to the name of the field or the value of the item also need corresponding changes made to the boolean logic, potentially in multiple places.
Allow the user to pass the current value (or array of values, for checkboxes), and using that to work out which item(s) should be checked or selected.
The above example can then be done like this:
Naming the new option has presented a few challenges – I wanted to avoid usingcheckedorselectedso that we don’t have options at different ‘levels’ with the same name but accepting different types of values – booleans for individual items and strings / arrays at the top level.I also felt thatvaluewas not an accurate description for what the option did, and may also be confused withitem.value(and the top-levelvalueoption on other form controls such as text inputs).I’ve ended up combining the two whilst also trying to be consistent with the underlying HTML attribute, and also reflecting whether the option accepts a string or an array. This does unfortunately mean that the option name is different for each of the three components:checkedValuefor Radios, where a string is passed to check a single radio buttoncheckedValuesfor Checkboxes, where an array of strings is passed to check one or more checkboxesselectedValuefor the Select, where a string is passed to select a single option.I think on balance, although I can see how this may be confusing for users, it is logical and consistent within each individual component.After further discussion within the team off the back of #2616 (comment) we ended up going with
value/values.Closes #2449.