calcite-design-system icon indicating copy to clipboard operation
calcite-design-system copied to clipboard

feat(list, list-item, list-item-group)!: Use "treegrid" role for improved accessibility

Open driskull opened this issue 2 years ago • 20 comments

Related Issue: #4676

Discussion: https://github.com/Esri/calcite-components/discussions/4356

Summary

feat(list, list-item, list-item-group)!: Use "treegrid" role for improved accessibility

BREAKING CHANGE: Listen to the calciteListItemSelect event instead of onClick for the calcite-list-item.

Details

Follows treegrid role: https://w3c.github.io/aria-practices/examples/treegrid/treegrid-1.html

List

  • Deprecates headingLevel property
  • Adds label property to specify an accessible name for the component.
  • Adds loading property
  • Adds selectionMode property
  • Adds selectionAppearance property

ListItemGroup

  • Deprecates headingLevel property
  • Adds disabled property

ListItem

  • Deprecates nonInteractive property
  • Adds calciteListItemSelect event
  • Adds selected property
  • Adds value property
  • Adds open property

Questions

  • Do we want to implement a "selected" style or prop to the list or let wrapping components implement that? Selection is different from what is actively focused. Need to meet to discuss this and come up with solution(s) A: Introduce property to toggle between selection style with border vs selection style with checkbox/indicator. Only "active" selection is source of truth for aria-checked. Any slotted buttons/actions are secondary.
  • Do we want dragEnabled built into list or will wrapping components implement it?: A: Yes, but delayed till post-1.0.0
  • Do we want filterEnabled built into list or will wrapping components implement it? : A: Yes, but delayed till post-1.0.0
  • ~~Should list-item have loading property built in?~~ We can always add later.

driskull avatar May 10 '22 23:05 driskull

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

github-actions[bot] avatar May 21 '22 01:05 github-actions[bot]

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

github-actions[bot] avatar Jun 11 '22 02:06 github-actions[bot]

@geospatialem do you think you could do some a11y testing on this branch for the list component? I'd like to get some initial feedback.

driskull avatar Jun 18 '22 00:06 driskull

@geospatialem do you think you could do some a11y testing on this branch for the list component? I'd like to get some initial feedback.

Looking good, @driskull! 💪🏻

A few considerations below, includes @benelan and @macandcheese experience and feedback.

General feedback

  • Keyboard nav feels good. When tabbing, the middle "rich content" list-item focuses the actions-start slot as well as the handle component within the slot. It doesn't happen in any of the other actions-* slots.

https://user-images.githubusercontent.com/5023024/175310343-5655b91a-72c2-4b64-a974-13c7b6c1f660.mp4

Rich content focus action-start 2

Rich content focus action-start 1

  • Clicking the input on the inline editing demo doesn't focus the input (tabbing does)
  • An item's focus ring is overlapped by an adjacent list item
  • No hover style for the list's content (e.g. simple list) - may be intentional?
  • The list-item's focus ring is a different color or opacity than its actions-* slot's focus ring
  • Saw a difference in when the "full row highlight" kicks in - tabbing through in this video, the first list's first child item gets highlighted, the second list's second child item gets fully highlighted.
  • For the editable inline - is that supposed to swap out for the top? I like that UI a lot, would it be a part of the list item component ?
  • For the other list type - those have a "removable" prop which renders a close internally, that would be a separate hit area than the actions-end examples here? Looks really good besides those two things and the above!

All screen readers

  • Rich/Non-interactive examples: Doesn't announce the value when interacting with the actions (e.g., Princess Bubblegum move handle)
  • Rich/Non-interactive examples: Can use the arrow keys to navigate the list, but to exit the list, must navigate through individual actions of the last element (e.g., move handle, ... and x)
  • Nested/Grouped and nested examples: Can expand/collapse when selected, but unable to collapse/expand when the button is focused.

JAWS with Chrome browser

Since we're not using a list role, as expected there are not any lists associated on the page. However there are two table roles associated with the Rich and Non-interactive examples. So we should document this for designers/developers so they know of the roles associated with the component when implementing in their solutions.

tables with JAWS screenshot

Voiceover with Safari browser

  • All examples: Doesn't announce the number of records present (e.g., item 1 of 3)
  • Rich/Non-interactive examples: Doesn't announce the values (e.g., Princess Bubblegum)

NVDA with Firefox browser

  • NVDA is pretty good. It seems to correctly read the list-item label and description as well as the actions-* slots. It does call stuff "table" instead of "list" which I guess is to be expected. It also calls all of the nested items "level one".
  • See above considerations under "All screen readers"

geospatialem avatar Jun 23 '22 13:06 geospatialem

@geospatialem @jcfranco @benelan I think this is ready to turn over to a designer to style the focus outlines, spacing, etc.

I still have these questions that need answers...

  • Do we want dragEnabled built into list or will wrapping components implement it?
  • Do we want filterEnabled built into list or will wrapping components implement it?
  • Do we want to implement a "selected" style or prop to the list or let wrapping components implement that? Selection is different from what is actively focused.

driskull avatar Jul 01 '22 15:07 driskull

@geospatialem @jcfranco @benelan I think this is ready to turn over to a designer to style the focus outlines, spacing, etc.

I still have these questions that need answers...

  • Do we want dragEnabled built into list or will wrapping components implement it?
  • Do we want filterEnabled built into list or will wrapping components implement it?
  • Do we want to implement a "selected" style or prop to the list or let wrapping components implement that? Selection is different from what is actively focused.

I think we had considered going from "list", "value list", and "pick list" to this new list, and a selection list. Currently the three lists have inconsistent group display, and slot availability, so it'd be nice to have them standardized. @asangma thoughts on continuing down that path?

macandcheese avatar Jul 01 '22 16:07 macandcheese

@driskull Did a short follow-up on screen readers with the list component. Really digging the updates - the navigation is great!

If additional actions, buttons, and/ selections are present on the list, we'd need to announce the list item's content to provide context to users.

A few samples:

'Move' the 'Princess Bubblegum' list item, currently item 1 of 3 'Open menu' for the 'Princess Bubblegum' list item 'Remove' button for 'Princess Bubblegum' list item

geospatialem avatar Jul 01 '22 20:07 geospatialem

If additional actions, buttons, and/ selections are present on the list, we'd need to announce the list item's content to provide context to users.

These are slotted elements that could be anything. i'm not sure how we would announce them.

driskull avatar Jul 01 '22 21:07 driskull

If additional actions, buttons, and/ selections are present on the list, we'd need to announce the list item's content to provide context to users.

These are slotted elements that could be anything. i'm not sure how we would announce them.

Understood on the slot, and appreciate the background. What about announcing the list item label prop with the associated element to provide context?

geospatialem avatar Jul 01 '22 22:07 geospatialem

What about announcing the list item label prop with the associated element to provide context?

Yeah, I could add an aria-label and aria-description. Do you know which elements those should be on?

driskull avatar Jul 02 '22 00:07 driskull

Yeah, I could add an aria-label and aria-description. Do you know which elements those should be on?

Could we add in to all associated child elements from the list-item, perhaps with the slot(s)?

geospatialem avatar Jul 05 '22 13:07 geospatialem

Can you test it now @geospatialem? Thanks

driskull avatar Jul 05 '22 16:07 driskull

Can you test it now @geospatialem? Thanks

Much improved, nice work, @driskull! Now able to get context immediately on the list.

It seems the slots don't seem to be getting the parent label associated with it across screen readers though. Is that achievable?

If a user starts interacting with a list-items action, they don't receive context of the accessible label or description for subsequent list-items or their actions. They do receive feedback of the column and row, but the screen readers aren't provided the aria-label or aria-description.

Here's an example from the JAWS transcript (similar experience in NVDA and VoiceOver as well):

Column 1, Row 1
test TreeGrid 
Princess Bubblegum row 
1 of 3
List
drag
Button 
To activate press Enter.
menu
menu
Button remove
remove
Button 
column 3 row 1
To activate press Enter.
menu
menu
Button remove
remove
Button 
To activate press Enter.
drag
drag
Button 


column 1 row 2
To activate press Enter.
menu
label
Button remove
remove
Button 
To activate press Enter.
menu
label
Button remove
remove
Button 
To activate press Enter.
drag
drag
Button 


column 1 row 3
To activate press Enter.
menu
menu
Button remove
remove
Button 
To activate press Enter.
menu
menu
Button remove
remove
Button 
To activate press Enter.

geospatialem avatar Jul 05 '22 22:07 geospatialem

It seems the slots don't seem to be getting the parent label associated with it across screen readers though. Is that achievable?

@geospatialem I'm not sure. I'm following the W3C for element structure. Let me know if it needs to change somehow.

driskull avatar Jul 05 '22 22:07 driskull

@geospatialem updated!

driskull avatar Jul 06 '22 21:07 driskull

I think this is ready for either @asangma or @macandcheese to take and do some styling to it.

@geospatialem let me know if there are any accessibility concerns I need to handle.

driskull avatar Jul 22 '22 21:07 driskull

@geospatialem let me know if there are any accessibility concerns I need to handle.

No concerns - a11y is looking fabulous. Nice work @driskull! 💯

geospatialem avatar Jul 22 '22 22:07 geospatialem

Do we want dragEnabled built into list or will wrapping components implement it? Do we want filterEnabled built into list or will wrapping components implement it? Should list-item have loading property built in?

Up for discussion, but if these components will be the base for other lists, we should consider enhancing to ensure all wrapping components are consistent. This also depends on current needs. I know some teams need filter/drag built into the list.

Do we want to implement a "selected" style or prop to the list or let wrapping components implement that? Selection is different from what is actively focused.

I think so, since the list already has a select event. Having a built-in style would be useful. cc @macandcheese

jcfranco avatar Aug 04 '22 18:08 jcfranco

I also think this can use the fix or feat type since it affected functionality and improved a11y.

jcfranco avatar Aug 04 '22 18:08 jcfranco

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

github-actions[bot] avatar Aug 18 '22 02:08 github-actions[bot]

cc @samanthahunter. This component could be used for "Layers".

driskull avatar Sep 23 '22 21:09 driskull

@jcfranco @macandcheese @ashetland Anything else I need to do here?

driskull avatar Oct 19 '22 21:10 driskull

Looking great, I think final design sign off is on @ashetland and @SkyeSeitz so the changes can be tracked and implemented in Figma.

macandcheese avatar Oct 19 '22 22:10 macandcheese