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

globalFormRadios Component Fieldset Accessibility Error #864

Open
danielclubb opened this issue Jan 11, 2023 · 2 comments
Open

globalFormRadios Component Fieldset Accessibility Error #864

danielclubb opened this issue Jan 11, 2023 · 2 comments
Labels
bug Something isn't working global global toolkit issue

Comments

@danielclubb
Copy link
Contributor

Hey team!

We think we may have found a bug in the implementation of global forms. We realised after we implemented Pa11y tests in our project which includes the CSAT widget.

The globalFormRadios component has a <fieldset> element inside of it, which uses the globalFormAttributes component to set data/aria attributes on the fieldset element dependent on the context.

However, inside the globalFormAttributes component there is the following line:
{{unless optional}}required aria-required="true"{{/unless}}

In our use case, Pa11y is throwing the following accessibility error:
"Elements must only use allowed ARIA attributes (https://dequeuniversity.com/rules/axe/4.2/aria-allowed-attr?application=axeAPI)"
because global forms is adding an aria-required attribute to a group <fieldset> element which it shouldn't be, it should only add this aria attribute to the individual input elements inside the fieldset.

This is happening because the unless logic on the globalFormAttributes component will always set this attribute unless optional is explicitly defined.

We feel the logic will need to be updated so that this attribute won't be set when globalFormAttributes is used on a group level element.

Thanks!

CC: Andreas Koscinski [email protected], Daniel Clubb [email protected], Morgan Cugerone [email protected]

@sturobson sturobson added bug Something isn't working global global toolkit issue labels Jan 11, 2023
@Heydon
Copy link
Contributor

Heydon commented Jan 11, 2023

Hi @danielclubb. Some points:

  • pa11y is correct that required is not available to fieldsets
  • however, this would not cause an accessibility barrier since it would simply be ignored by AT
  • the workaround is to add "optional": true to the globalFormRadios field in question.

The issue arises from applying common attributes to any field where present in the data, like {{> inputCommonAttributes}}. Otherwise we'd have a significant amount of code duplication. For a fieldset, you simply would not add attributes only suitable for inputs. However, required is set by default so has to be removed manually using the workaround above.

If handlebars was a more expressive templating language, we'd be able to write {{#if template !== 'globalFormRadios'}} or similar and use that logic. The only way to use expressions with Handlebars is with Handlebars helpers. Unfortunately, these would have to be maintained in Java and JavaScript and frontend-toolkits is not a Java repository/environment. 🤷‍♂️🤷‍♂️🤷‍♂️. Sorry for the inconvenience!

@morgaan
Copy link
Contributor

morgaan commented Jan 12, 2023

Hi @Heydon, thank you for your elaborate answer 🤗

Far from us to go down the more expressive templating language way, using helpers... as this may encourage sticking more logic in the view... hence the Java/Javascript overhead it brings...

After some processing about what you meant by:

the workaround is to add "optional": true to the globalFormRadios field in question.
We did just that and then got rid of the pa11y issue 🙌. It feels a bit counter intuitive, so maybe that would be worth a hint into the documentation 🤷? I'll leave this issue open just in case you want to use it as a reference for adding the hint, and if not, please go ahead and close it 🙏.

That fix made our day anyway so thanks again 🤗.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working global global toolkit issue
Projects
None yet
Development

No branches or pull requests

4 participants