From 72b9c7109fad8399504d2bda33dbd6c598b9bed3 Mon Sep 17 00:00:00 2001
From: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
Date: Fri, 9 Jun 2023 11:12:08 +0100
Subject: [PATCH 1/3] Omit value attribute from options with no value

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.
---
 .../src/govuk/components/select/select.yaml   | 35 ++++++++++++++++---
 .../src/govuk/components/select/template.njk  |  2 +-
 .../govuk/components/select/template.test.js  | 34 ++++++++++++++++--
 3 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/packages/govuk-frontend/src/govuk/components/select/select.yaml b/packages/govuk-frontend/src/govuk/components/select/select.yaml
index 1eb34e9a63..d82dcc42b0 100644
--- a/packages/govuk-frontend/src/govuk/components/select/select.yaml
+++ b/packages/govuk-frontend/src/govuk/components/select/select.yaml
@@ -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
@@ -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:
@@ -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:
diff --git a/packages/govuk-frontend/src/govuk/components/select/template.njk b/packages/govuk-frontend/src/govuk/components/select/template.njk
index 5a71db49f9..718bf8f306 100644
--- a/packages/govuk-frontend/src/govuk/components/select/template.njk
+++ b/packages/govuk-frontend/src/govuk/components/select/template.njk
@@ -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>
diff --git a/packages/govuk-frontend/src/govuk/components/select/template.test.js b/packages/govuk-frontend/src/govuk/components/select/template.test.js
index 86dc9e9b0b..74e53c51ca 100644
--- a/packages/govuk-frontend/src/govuk/components/select/template.test.js
+++ b/packages/govuk-frontend/src/govuk/components/select/template.test.js
@@ -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)
 
@@ -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)

From e1bab47321616770fd4e68e1d21e9ee39d146d41 Mon Sep 17 00:00:00 2001
From: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
Date: Fri, 9 Jun 2023 11:23:51 +0100
Subject: [PATCH 2/3] Allow selecting options based on implicit value

Allow the top-level `value` option on a `govukSelect` to work with options that have no explicit value set.

The value of an option element when there is no `value` attribute is the text content of the element, so use that instead.
---
 .../src/govuk/components/select/select.yaml         | 13 +++++++++++++
 .../src/govuk/components/select/template.njk        |  4 +++-
 .../src/govuk/components/select/template.test.js    |  7 +++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/packages/govuk-frontend/src/govuk/components/select/select.yaml b/packages/govuk-frontend/src/govuk/components/select/select.yaml
index d82dcc42b0..da5ffcee1a 100644
--- a/packages/govuk-frontend/src/govuk/components/select/select.yaml
+++ b/packages/govuk-frontend/src/govuk/components/select/select.yaml
@@ -312,6 +312,19 @@ examples:
         - 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:
diff --git a/packages/govuk-frontend/src/govuk/components/select/template.njk b/packages/govuk-frontend/src/govuk/components/select/template.njk
index 718bf8f306..77348750c3 100644
--- a/packages/govuk-frontend/src/govuk/components/select/template.njk
+++ b/packages/govuk-frontend/src/govuk/components/select/template.njk
@@ -44,8 +44,10 @@
     {%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}>
   {% for item in params.items %}
     {% if item %}
+      {# 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 item.value == params.value) }}
+        {{-" 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 %}
diff --git a/packages/govuk-frontend/src/govuk/components/select/template.test.js b/packages/govuk-frontend/src/govuk/components/select/template.test.js
index 74e53c51ca..6bf15e4bc1 100644
--- a/packages/govuk-frontend/src/govuk/components/select/template.test.js
+++ b/packages/govuk-frontend/src/govuk/components/select/template.test.js
@@ -91,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'])
 

From d8222a662c128627a28cc6d6faa6c6e403276034 Mon Sep 17 00:00:00 2001
From: Oliver Byford <oliver.byford@digital.cabinet-office.gov.uk>
Date: Fri, 9 Jun 2023 11:33:35 +0100
Subject: [PATCH 3/3] Document in CHANGELOG

---
 CHANGELOG.md | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8476105665..08cf39de3f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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