zestia / ember-select-box

:capital_abcd: A faux select box for Ember apps
MIT License
64 stars 14 forks source link

[a11y] Axe finds violations #40

Closed frykten closed 1 year ago

frykten commented 2 years ago

Hi!

We use ember-a11y-testing to ensure some technicalities in the a11y of the components/pages we develop. We recently upgraded to the last major of this addon (we were late by a lot though) and it appears there are multiple violations in the current state of the addon.

Have you tried testing it?

I'm pasting the list we have on a very basic a11y test of the component & 3 options + a label:

The page should have no accessibility violations. Violations:

[critical]: Elements must only use allowed ARIA attributes 
Violated 1 time. Offending nodes are: 
<div data-component="select-box" aria-expanded="false" aria-busy="false" aria-disabled="false" aria-multiselectable="false" id="ID-select-box" role="combobox" tabindex="-1">
https://dequeuniversity.com/rules/axe/4.2/aria-allowed-attr?application=axeAPI

[critical]: Certain ARIA roles must be contained by particular parents 
Violated 3 times. Offending nodes are: 
<div data-component="selected-option" aria-current="false" id="select-box-el-ID1" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID2" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID3" role="option">
https://dequeuniversity.com/rules/axe/4.2/aria-required-parent?application=axeAPI

[critical]: ARIA attributes must conform to valid values 
Violated 1 time. Offending nodes are: 
<input data-component="input" aria-multiline="false" autocomplete="off" role="searchbox" aria-controls="select-box-el-ID4" id="ID-linked-input-chips" type="text">
https://dequeuniversity.com/rules/axe/4.2/aria-valid-attr-value?application=axeAPI

[serious]: Ensure interactive controls are not nested 
Violated 3 times. Offending nodes are: 
<div data-component="selected-option" aria-current="false" id="select-box-el-ID1" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID2" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID3" role="option">
https://dequeuniversity.com/rules/axe/4.2/nested-interactive?application=axeAPI
amk221 commented 2 years ago

Thanks,

Can you share the entire component?

amk221 commented 2 years ago

Ping

frykten commented 2 years ago

Hello!

I present to you my apologies for not responding sooner.

I cannot share exactly "the entire component" as it is property of my company and some would be useless to you, being some side stuff only needed for our application.

However, here's a simpler implementation of the select-box component without all our data: it still bears almost all of the errors shared above, but the Ensure interactive controls are not nested. That was happening due to us: we use another component inside the selected-option, and it has nested interactive controls, my bad for not noticing. The component:

<SelectBox
  id={{this.selectBoxId}}
  @value={{this.value}}
  @searchMinChars={{0}}
  @searchDelayTime={{100}}
  @onFocusIn={{optional @onFocusIn}}
  @onFocusOut={{optional @onFocusOut}}
  @onPressEscape={{this.onPressEscape}}
  @onPressEnter={{this.addOnPress 'Enter'}}
  @onPressTab={{fn this.addOnPress 'Tab'}}
  @onPressBackspace={{this.pressedBackspace}}
  @onPressDown={{this.pressedDown}}
  @onPressUp={{this.pressedUp}}
  @onPressLeft={{this.pressedLeft}}
  @onPressRight={{this.pressedRight}}
  @onSelect={{this.addItem}}
  @onSearch={{fn this.updateAvailableItems this.onSearch}}
  @disabled={{this.disabled}}
  as |sb|
>
  <sb.SelectedOptions>
    {{#each this.values as |item index|}}
      <sb.SelectedOption
        @onActivate={{fn (mut this.activeSelectedOption) item}}
        {{on 'click' (fn sb.activateSelectedOptionAtIndex index)}}
      >
        {{item.name}}
      </sb.SelectedOption>
    {{/each}}
    {{#if this.showInputField}}
      <sb.Input
        disabled={{this.disabled}}
        id={{this.inputId}}
        maxlength={{this.maxTagLength}}
        placeholder={{this.placeholder}}
        type='text'
        @onInput={{pipe this.onInput sb.open}}
        @onDelete={{fn this.onDelete this.values.lastObject sb}}
        {{on 'keydown' (fn this.pressedDel sb)}}
        {{on 'click' (fn this.reveal sb this.onSearch)}}
      />
    {{/if}}
  </sb.SelectedOptions>
</SelectBox>

With that component and the same version of ember-a11Y-testing ("ember-a11y-testing": "^4.0.7"), I still get:

The page should have no accessibility violations. Violations:

[critical]: Elements must only use allowed ARIA attributes 
Violated 1 time. Offending nodes are: 
<div data-component="select-box" aria-expanded="false" aria-busy="false" aria-disabled="false" aria-multiselectable="false" id="ID-select-box" role="combobox" tabindex="-1">
https://dequeuniversity.com/rules/axe/4.2/aria-allowed-attr?application=axeAPI

[critical]: Certain ARIA roles must be contained by particular parents 
Violated 3 times. Offending nodes are: 
<div data-component="selected-option" aria-current="false" id="select-box-el-ID1" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID2" role="option">
<div data-component="selected-option" aria-current="false" id="select-box-el-ID3" role="option">
https://dequeuniversity.com/rules/axe/4.2/aria-required-parent?application=axeAPI

[critical]: ARIA attributes must conform to valid values 
Violated 1 time. Offending nodes are: 
<input data-component="input" aria-multiline="false" autocomplete="off" role="searchbox" aria-controls="select-box-el-ID4" id="ID-linked-input-chips" type="text">
https://dequeuniversity.com/rules/axe/4.2/aria-valid-attr-value?application=axeAPI
amk221 commented 2 years ago

Thank you very much for that, no worries about the delay - keen to sort any issues!

Your component looks a little unusual, it doesn't have any options? What good is a select box with no options? :)

The general gist would be:

<SelectBox @onSelect={{@onSelect}} as |sb|>
  <sb.SelectedOption>
    {{#if sb.value}}
      You selected {{sb.value}}
    {{else}}
      Please select...
    {{/if}}
  <sb.SelectedOption>

  {{#each @options as |option|}}
    <sb.Option @value={{option}}>
      {{option}}
    </sb.Option>
  {{/each}}
</SelectBox>

If it was a multiple select box, you could render multiple sb.SelectedOptions to represent what was selected.


Regarding the critical violations - when I render your example I don't get them. So I wonder if there is something else going on?

Would you be able to paste the "To solve this issue, you need to..." part from Axe? Because the violation descriptions don't mention which things to fix - it just says "certain aria roles" without saying which :)

frykten commented 2 years ago

Oh, I deleted too much: we actually usually yield them:

{{yield
    (hash
      options=sb.Options
      option=sb.Option
    )
  }}

With or without, it did not change the errors in the tests.


Hmm. Strange. I tried upgrading our ember-a11y-testing from 4.0.7 to the last major 5.0.0 but I still get the same errors.

Here's the list from the console logging:

critical: Elements must only use allowed ARIA attributes https://dequeuniversity.com/rules/axe/4.2/aria-allowed-attr?application=axeAPI
HTML: <div data-component="select-box" aria-expanded="false" aria-busy="false" aria-disabled="false" aria-multiselectable="false" id="ID-select-box" role="combobox" tabindex="-1">
Fix any of the following:
  ARIA attribute is not allowed: aria-multiselectable="false"
critical: ARIA attributes must conform to valid values https://dequeuniversity.com/rules/axe/4.2/aria-valid-attr-value?application=axeAPI
HTML: <div data-component="selected-option" aria-current="false" id="select-box-el-ID1" role="option">
HTML: <div data-component="selected-option" aria-current="false" id="select-box-el-ID2" role="option">
HTML: <div data-component="selected-option" aria-current="false" id="select-box-el-ID3" role="option">
Fix any of the following:
  Required ARIA parent role not present: listbox
HTML: <div data-component="select-box" aria-expanded="false" aria-busy="false" aria-disabled="false" aria-multiselectable="false" id="ID-select-box" role="combobox" tabindex="-1">
Fix any of the following:

(weirdly, this last one does not have a "fix description", tho its attributes look pretty good and as expected to me)

amk221 commented 2 years ago

Ah ok I was just using Axe in the developer console. I will try the addon.

amk221 commented 2 years ago

Can I just check what version of @zestia/ember-select-box you are using? I see reference to data- attributes, which I think imply a fairly old version?

amk221 commented 2 years ago

I've just noticed the first one ("Elements must only use allowed ARIA attributes") was fixed in August, but not released yet: https://github.com/zestia/ember-select-box/commit/6d45adb775323c5029447ebe3a0c6dd530d4e828

frykten commented 2 years ago

Damn. I may have made you lose your time: we recently upgraded select-box and I thought it had been upgraded to the last version... Turns out we're still late, ie.: "@zestia/ember-select-box": "^14.0.0",...

I'll have to present you my apologies, I guess we can close this issue.

amk221 commented 2 years ago

No problem what so ever, keep the issue open, there may still be something up :)

amk221 commented 1 year ago

Closing, as aria support is better in 16x

Please re-open if you find anything else