gunndabad / govuk-frontend-aspnetcore

ASP.NET Core integration for GOV.UK Design system
MIT License
30 stars 8 forks source link

Difficultly passing additional values to aria-describedby #246

Closed sussexrick closed 1 year ago

sussexrick commented 2 years ago

I have some logic related to custom error messages that means I want to supply an id to aria-described-by only if there is an error.

Using govuk-input I would expect that any of the following would be ignored, but in fact they generate an empty attribute:

<govuk-input described-by="@("")">
<govuk-input described-by="@(null)">
<govuk-input input-aria-describedby="@("")">
<govuk-input input-aria-describedby="@(null)">

The only method I've found to achieve the result I want is to pass a dictionary of attributes to <govuk-input input-attributes="myDictionary">.

There is a problem with the input-attributes approach. If the field is already in an error state and I pass it a dictionary via input-attributes that contains an aria-describedby attribute I get an error saying that there is already an attribute called aria-describedby which is put there by the built-in error state. This is different from the behaviour if I pass a string to the described-by attribute, where the new value is merged with the one from the built-in error state. I would expect the value from the dictionary to be merged in the same way.

image

<govuk-textarea /> and <govuk-character-count /> do not have the same described-by attribute that <govuk-input /> and <govuk-select /> have.

Although I'm noting this primarily with govuk-input and aria-describedby I would expect this would apply in many places where an attribute accepts a space-separated tokenised value, but not for attributes that can legitimately be empty like <img alt="" />.

gunndabad commented 2 years ago

@sussexrick Thanks for reporting this.

I've made a change that should make this work for both:

<govuk-input described-by="@("")">
<govuk-input described-by="@(null)">

There's a preview package with the change in at https://github.com/gunndabad/govuk-frontend-aspnetcore/actions/runs/3499069608 Does this solve the problem for you?

Doing a better job of merging attributes like aria-described-by and class is something I'll look at separately.

sussexrick commented 1 year ago

Sorry for taking a long time to respond - my work moved on and I don't currently have a use case for this. However, I've retested the five approaches on <govuk-input /> using beta4 and described-by with either empty string or null is still rendering an empty attribute, while input-aria-describedby with empty string or null resulted in the same InvalidOperationException that the dictionary produces.