mi6 / ic-ui-kit

Intelligence Community UI Kit (based on StencilJS)
MIT License
25 stars 27 forks source link

Text field renders ic-input-validation element in the HTML when validationStatus is set to an empty string #2140

Open GCHQ-Developer-847 opened 3 months ago

GCHQ-Developer-847 commented 3 months ago

Summary of the bug

When a text field has validationText set but validationStatus is set to an empty string, the ic-input-validation subcomponent is rendered in the HTML (even though no validation behaviour is actually visible on the text field on the screen).

It is likely that this is causing problems with the aria-live region that is used in the ic-input-validation component.

🪜 How to reproduce

  1. Go to this StackBlitz.
  2. Inspect the text field.
  3. See in the HTML that the ic-input-validation is being rendered:

🧐 Expected behaviour

If the validationText prop has a value and validationStatus is set to an empty string, the ic-input-validation element should not be rendered at all. I.e. it should only be rendered when we want the validation status / message to be visible.

Additional info

This appears to be causing issues with the aria-live region within the ic-input-validation component. Apparently, if an aria-live region is rendered on page load, it is known to be unreliable (e.g. the info doesn't get read out when the text is updated).

Developers are likely to set the validation behaviour on text fields a bit like this:

validation-status={invalidInput ? "error" : ""}
validation-text="Error message"

And then change the validation-status after users have interacted with the page. Which means the aria-live region is present on page load. Fixing the logic to not display the ic-input-validation component when validation-status is an empty string should resolve issues I have seen with aria-live regions being flaky when using VoiceOver.

NOTE: It is probably worth checking that ic-input-validation is also not rendered when validation-status is set to null or undefined, just to cover all bases.

GCHQ-Developer-847 commented 3 months ago

It is probably just a case of updating the logic here (line 662 in ic-text-field.tsx):

GCHQ-Developer-299 commented 2 months ago

Consider whether a new test can be written to check that the field isn't rendered when not necessary?