Skip to content

Commit

Permalink
Omit value attribute from options with no value
Browse files Browse the repository at this point in the history
In HTML, if you omit the `value` attribute from an `<option>`the value defaults to the text contained inside the element [1].

However if you omit the value from an option when using the `govukSelect` macro, we currently include an empty value attribute on the `<option>` element. This would mean that when submitting a form using a select with options with no value set, the value submitted would be the empty string.

Instead, omit the `value` attribute if no `value` option has been provided. We then mimic the default HTML behaviour, as the option value will then naturally fall back to the text content.

We still need to ensure that the `value` attribute is included when the value option is passed but has a falsey value (like an empty string, 0, or boolean false).

Fixes #3440.

[1]: https://html.spec.whatwg.org/multipage/form-elements.html#the-option-element:~:text=The%20value%20attribute%20provides%20a%20value%20for%20element.%20The%20value%20of%20an%20option%20element%20is%20the%20value%20of%20the%20value%20content%20attribute%2C%20if%20there%20is%20one%2C%20or%2C%20if%20there%20is%20not%2C%20the%20value%20of%20the%20element%27s%20text%20IDL%20attribute.
  • Loading branch information
36degrees committed Jun 9, 2023
1 parent bc139a3 commit 7d646d2
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 7 deletions.
35 changes: 31 additions & 4 deletions src/govuk/components/select/select.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ params:
- name: value
type: string
required: false
description: Value for the option item. Defaults to an empty string.
description: Value for the option. If this is omitted, the value is taken from the text content of the option element.
- name: text
type: string
required: true
Expand Down Expand Up @@ -231,11 +231,11 @@ examples:
attributes:
data-attribute: GHI
data-second-attribute: JKL
- name: with falsey values
- name: with falsey items
hidden: true
data:
id: with-falsey-values
name: with-falsey-values
id: with-falsey-items
name: with-falsey-items
label:
text: Label text goes here
items:
Expand Down Expand Up @@ -300,6 +300,33 @@ examples:
- value: 2
text: GOV.UK frontend option 2

- name: without values
hidden: true
data:
name: colors
id: colors
label:
text: Label text goes here
items:
- text: Red
- text: Green
- text: Blue

- name: with falsey values
hidden: true
data:
name: falsey-values
id: falsey-values
label:
text: Label text goes here
items:
- text: Empty string
value: ''
- text: Boolean false
value: false
- text: Number zero
value: 0

- name: item selected overrides value
hidden: true
data:
Expand Down
2 changes: 1 addition & 1 deletion src/govuk/components/select/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
{%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}>
{% for item in params.items %}
{% if item %}
<option value="{{ item.value }}"
<option {%- if item.value !== undefined %} value="{{ item.value }}"{% endif %}
{{-" selected" if item.selected | default(params.value and item.value == params.value) }}
{{-" disabled" if item.disabled }}
{%- for attribute, value in item.attributes %} {{ attribute }}="{{ value }}"{% endfor -%}>{{ item.text }}</option>
Expand Down
34 changes: 32 additions & 2 deletions src/govuk/components/select/template.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,43 @@ describe('Select', () => {
expect($items.length).toEqual(3)
})

it('renders item with value', () => {
it('includes the value attribute', () => {
const $ = render('select', examples.default)

const $firstItem = $('.govuk-select option:first-child')
expect($firstItem.attr('value')).toEqual('1')
})

it('includes the value attribute when the value option is an empty string', () => {
const $ = render('select', examples['with falsey values'])

const $firstItem = $('.govuk-select option:nth(0)')
expect($firstItem.attr('value')).toEqual('')
})

it('includes the value attribute when the value option is false', () => {
const $ = render('select', examples['with falsey values'])

const $secondItem = $('.govuk-select option:nth(1)')
expect($secondItem.attr('value')).toEqual('false')
})

it('includes the value attribute when the value option is 0', () => {
const $ = render('select', examples['with falsey values'])

const $thirdItem = $('.govuk-select option:nth(2)')
expect($thirdItem.attr('value')).toEqual('0')
})

it('omits the value attribute if no value option is provided', () => {
const $ = render('select', examples['without values'])

const $firstItem = $('.govuk-select option:first-child')
// Ideally we'd test for $firstItem.attr('value') == undefined but it's
// broken in Cheerio – https://github.com/cheeriojs/cheerio/issues/3237
expect($firstItem.toString()).not.toContain('value')
})

it('renders item with text', () => {
const $ = render('select', examples.default)

Expand Down Expand Up @@ -90,7 +120,7 @@ describe('Select', () => {
})

it('renders without falsely items', () => {
const $ = render('select', examples['with falsey values'])
const $ = render('select', examples['with falsey items'])

const $items = $('.govuk-select option')
expect($items.length).toEqual(2)
Expand Down

0 comments on commit 7d646d2

Please sign in to comment.