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

SelectItem with empty text produces invalid HTML5 #223

Open
pangiole-hmrc opened this issue Feb 21, 2023 · 1 comment
Open

SelectItem with empty text produces invalid HTML5 #223

pangiole-hmrc opened this issue Feb 21, 2023 · 1 comment

Comments

@pangiole-hmrc
Copy link

pangiole-hmrc commented Feb 21, 2023

Symptoms

The following example of usage:

import uk.gov.hmrc.govukfrontend.views.viewmodels.select.{Select, SelectItem}

val select = Select(
  id = "example",
  items = Seq(
    Select(text = "")
    Select(value="1", text = "hello")
  )
)

would produce the following HTML fragment:

<select id="example">
  <option value=""></option>
  <option value="1">hello</option>
</select>

which, when fed into an accessibilty tester, would report the following HTML5 validatation error

Element “option” without attribute “label” must not be empty.

Diagnose

By Googling around, I understood that each <option> element must have not only a value but also a display name. That name can be specified either as text content (such as hello) or via the label attribute.

The above HTML5 fragment is invalid because of the first <option> element. Accoding to the HTML5 specs, there would be no way to derive the display name because: (1) the text content is given as empty and (2) at the same time there's no additional label attribute .

Quick and dirty solution

Just pass the &nbsp; value as text content in Scala code:

import uk.gov.hmrc.govukfrontend.views.viewmodels.select.{Select, SelectItem}

val select = Select(
  id = "example",
  items = Seq(
    Select(value="1", text = "hello"),
    Select(text = "&nbsp;")
  )
)

that seems to trick the validator into believing the HTML code is valid. Also, it does not seem to break the selection widget on screen.

Clean solution

???

pangiole-hmrc added a commit to hmrc/economic-crime-levy-registration-frontend that referenced this issue Feb 21, 2023
This commit attempts a workaround for the issue filed at
hmrc/play-frontend-hmrc#223

Resolves: ECL-252
pangiole-hmrc added a commit to hmrc/economic-crime-levy-registration-frontend that referenced this issue Feb 21, 2023
This commit attempts a workaround for the issue filed at
hmrc/play-frontend-hmrc#223

Resolves: ECL-252
pangiole-hmrc added a commit to hmrc/economic-crime-levy-registration-frontend that referenced this issue Feb 21, 2023
This commit attempts a workaround for the issue filed at
hmrc/play-frontend-hmrc#223

Resolves: ECL-252
pangiole-hmrc added a commit to hmrc/economic-crime-levy-registration-frontend that referenced this issue Feb 21, 2023
* Fix all accesibility issues

  - move the `h1` (header) and `p` (paragraph) elements to the above
    and outside of the form element

* Add labels styled as hidden headings

With the intent to pass the accessibility tests, we agreed upon adding
the `label` element with the same text content as the heading. We are
also styling it as hidden (so to put it out of sight)

* Tweak empty select item

This commit attempts a workaround for the issue filed at
hmrc/play-frontend-hmrc#223

Resolves: ECL-252

---------

Co-authored-by: peter-hazell <18005888+peter-hazell@users.noreply.github.com>
@oscarduignan
Copy link
Contributor

Hi @pangiole-hmrc, apologies, totally missed this! We have a few places where we've added some @require() assertions on parameters to prevent invalid states (like stopping people from marking two radio buttons as checked at the same time.) This might be a good place for something like that - I will put it on our tech debt board internally and flag it with our product owner

We also have some helper methods for building up the parameters required by components, so I could imagine paired with an assertion to prevent the invalid state, we could have a helper method that was something like Select().withAnEmptyOptionAtStart() or something like that, to save people having to remember what a valid empty option looks like

I'm not sure what the impact to users would be in terms of accessibility though, even if it is invalid html, it might be that browsers are mostly able to accomodate this - so I can't say how quickly it will be prioritised amongst other work. We have to be careful with additions like this that narrow the scope of allowable parameters because they are technically breaking changes that might be missed on big services and lead to worse user experience / accessibility than what we're trying to avoid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants