Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit the value attribute from select options with no value #3773

Merged
merged 3 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ If you’re not using Nunjucks macros, update your warning text HTML to replace

This change was introduced in [pull request #3569: Remove unnecesary class from Warning Text component](https://github.com/alphagov/govuk-frontend/pull/3569).

#### Check that selects work as expected

The `govukSelect` macro will no longer include an empty value attribute for options that do not have a value set.

This means that the value of the select if that option is selected will now be the text content of the option, rather than an empty string.

If you need to maintain the existing behaviour, you can set the value to an empty string.

This change was introduced in [pull request #3773: Omit the value attribute from select options with no value](https://github.com/alphagov/govuk-frontend/pull/3773).

## 4.6.0 (Feature release)

### New features
Expand Down
48 changes: 44 additions & 4 deletions packages/govuk-frontend/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,46 @@ 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: without values with selected value
hidden: true
data:
name: colors
id: colors
label:
text: Label text goes here
items:
- text: Red
- text: Green
- text: Blue
value: Green

- 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
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@
{%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}>
{% for item in params.items %}
{% if item %}
<option value="{{ item.value }}"
{{-" selected" if item.selected | default(params.value and item.value == params.value) }}
{# Allow selecting by text content (the value for an option when no value attribute is specified) #}
{% set effectiveValue = item.value | default(item.text) %}
<option {%- if item.value !== undefined %} value="{{ item.value }}"{% endif %}
{{-" selected" if item.selected | default(params.value and effectiveValue == params.value) }}
{{-" disabled" if item.disabled }}
{%- for attribute, value in item.attributes %} {{ attribute }}="{{ value }}"{% endfor -%}>{{ item.text }}</option>
{% endif %}
Expand Down
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

expect($firstItem.toString()).not.toContain('value')
})

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

Expand All @@ -61,6 +91,13 @@ describe('Select', () => {
expect($selectedItem.attr('selected')).toBeTruthy()
})

it('selects options with implicit value using selected value', () => {
const $ = render('select', examples['without values with selected value'])

const $selectedItem = $("option:contains('Green')")
expect($selectedItem.attr('selected')).toBeTruthy()
})

it('allows item.selected to override value', () => {
const $ = render('select', examples['item selected overrides value'])

Expand Down Expand Up @@ -90,7 +127,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