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

fix(list, list-item): support keyboard sorting in screen readers #9038

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

driskull
Copy link
Member

@driskull driskull commented Apr 2, 2024

Related Issue: #7426

Summary

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 2, 2024
@driskull driskull marked this pull request as ready for review April 2, 2024 16:44
@driskull driskull requested a review from a team as a code owner April 2, 2024 16:44
@jcfranco
Copy link
Member

jcfranco commented Apr 3, 2024

@driskull Can you clarify which comment or link from the StackOverflow post is referenced? Is it this?

@driskull
Copy link
Member Author

driskull commented Apr 3, 2024

Sorry, I was referring to this one:

https://stackoverflow.com/a/52264128

It probably won't fire because NVDA intercepts the keys if you are in browse mode, which is normal. If you were to manually switch to focus mode (insert + space -- you'll hear a "typewriter" sound), the code should get passed through.

and this one:

https://stackoverflow.com/a/55793672

Add role="application" in your widget html and try it, it is working for me.

Role of application does seem to fix it. I'm not sure if there are any downsides to doing that.

This may also be helpful: https://accessibleculture.org/articles/2012/09/aria-widgets-and-focus-forms-mode-support/

@jcfranco
Copy link
Member

jcfranco commented Apr 3, 2024

Awesome, thanks!

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once explainers are added, this looks good to merge!

aria-pressed={this.disabled ? null : toAriaBoolean(this.selected)}
class={{ [CSS.handle]: true, [CSS.handleSelected]: !this.disabled && this.selected }}
onBlur={this.handleBlur}
onKeyDown={this.handleKeyDown}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on nvaccess/nvda#7898 (comment), I'm curious if screen readers are triggering a click vs keydown event here. Could you team up w/ @geospatialem to test this out? If it works, it could help avoid the host role.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geospatialem could you review this one?

We don't want to add a click event to this element but we could change its role if we need to. Switching from "button" to "radio" or whatever as suggested by that article.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's right. There's no click handling logic here. Ignore my suggestion then. Changing the inner element's role makes sense.

@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 3, 2024
@geospatialem geospatialem self-requested a review April 3, 2024 22:36
Copy link
Member

@geospatialem geospatialem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great with both JAWS and NVDA! Nice work, @driskull! ✨

@driskull
Copy link
Member Author

driskull commented Apr 3, 2024

@jcfranco can you review again?

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very noice! 🏆

@driskull driskull added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Apr 5, 2024
@driskull driskull merged commit b2e1b9b into main Apr 5, 2024
15 checks passed
@driskull driskull deleted the dris0000/nvda-space-bar-support-handle branch April 5, 2024 22:27
@github-actions github-actions bot added this to the 2024-04-30 - Apr Release milestone Apr 5, 2024
geospatialem pushed a commit that referenced this pull request May 1, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-design-tokens: 2.2.0</summary>

##
[2.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-design-tokens@2.1.2...@esri/calcite-design-tokens@2.2.0)
(2024-04-30)


### Features

* Ensure all components inherit font-family
([#8388](#8388))
([90f8923](90f8923))
</details>

<details><summary>@esri/eslint-plugin-calcite-components:
1.2.0</summary>

##
[1.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/eslint-plugin-calcite-components@1.1.0...@esri/eslint-plugin-calcite-components@1.2.0)
(2024-04-30)


### Features

* **enforce-ref-last-prop:** Add fixer
([#8671](#8671))
([688bc51](688bc51))


### Bug Fixes

* **slider:** Improve snapping + step logic
([#8169](#8169))
([8b83042](8b83042))
</details>

<details><summary>@esri/calcite-components: 2.8.0</summary>

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.7.1...@esri/calcite-components@2.8.0)
(2024-04-30)


### Features

* Add calciteInvalid event for form validation
([#9211](#9211))
([a504508](a504508))
* Add validity property to form components
([#9210](#9210))
([e49902b](e49902b))
* **carousel:** Add Carousel and Carousel Item components
([#8995](#8995))
([b1ba428](b1ba428))
* **enforce-ref-last-prop:** Add fixer
([#8671](#8671))
([688bc51](688bc51))
* Ensure all components inherit font-family
([#8388](#8388))
([90f8923](90f8923))
* **input-time-zone:** Add readonly support
([#9111](#9111))
([153a122](153a122))
* **input-time-zone:** Allow clearing value
([#9168](#9168))
([193bb7d](193bb7d))
* **list-item-group:** Update list item group styles
([#9072](#9072))
([c734849](c734849))
* **navigation-logo:** Add heading level
([#9054](#9054))
([c2beba8](c2beba8))
* **pagination:** Add navigation methods
([#9154](#9154))
([5ca6a5f](5ca6a5f))
* **slider:** Add support for custom label formatting
([#9179](#9179))
([710d1ee](710d1ee))
* **slider:** Allow configuring fill behavior
([#9170](#9170))
([1b2cdbf](1b2cdbf))
* **tile, tile-group:** Support single, multi, single-persist, none
selection modes and icon and border selection appearances
([#9159](#9159))
([2d77470](2d77470))


### Bug Fixes

* **action:** Maintain equal height when text is not enabled in a small
scale
([#9051](#9051))
([407824a](407824a))
* **chip-group:** Improve programmatic Chip selection behavior
([#9213](#9213))
([b7a4c77](b7a4c77))
* **combobox:** Update the focus and hover chevron icon color
([#9187](#9187))
([a1317da](a1317da))
* **combobox, input-time-zone:** Improve readOnly behavior and styling
([#9222](#9222))
([606d80f](606d80f))
* **combobox:** Fix aria-role for proper voiceover support
([#9039](#9039))
([eebe8ca](eebe8ca))
* **combobox:** Update the focused chevron icon color
([#9202](#9202))
([27ac02d](27ac02d))
* **date-picker:** Ensure `proximitySelectionDisabled` resets range on
post-range-commit selection
([#9171](#9171))
([f466b14](f466b14))
* **date-picker:** Restore focus on date when navigating month with
arrow/page keys
([#9063](#9063))
([db109e0](db109e0))
* Fix memory leak affecting components using conditionally-rendered
slots
([#9208](#9208))
([28701b6](28701b6))
* **input-date-picker:** Update the focus and hover chevron icon color
([#9146](#9146))
([ca895e9](ca895e9))
* **input-time-zone:** Ensure selected item is properly displayed when
there are other items with the same offset
([#9134](#9134))
([1f94903](1f94903))
* **input, input-number, input-text, input-date-picker,
input-time-picker, filter, menu-item:** Ignore canceled events and
enforce cancellations where needed
([#8952](#8952))
([d0fa977](d0fa977))
* **list, list-item, list-item-group:** Improve drag experience by
indenting items
([#9169](#9169))
([88aab49](88aab49))
* **list, list-item:** Support keyboard sorting in screen readers
([#9038](#9038))
([b2e1b9b](b2e1b9b))
* **list:** Closed list-items no longer render extra space
([#9031](#9031))
([3985004](3985004))
* **panel:** Support cancelling the esc key event to prevent closing a
panel
([#9032](#9032))
([ecfa5f1](ecfa5f1))
* **preset:** Update the focus outline color
([#9098](#9098))
([725f47c](725f47c))
* **radio-button:** Display validation message in parent group
([#9218](#9218))
([b1e869f](b1e869f))
* **select:** Update the focus and hover chevron icon color and select
focus and hover background color
([#9160](#9160))
([b187340](b187340))
* **slider:** Fix fill placement and tick styling when ranged
([#9204](#9204))
([a84d831](a84d831))
* **slider:** Improve snapping + step logic
([#8169](#8169))
([8b83042](8b83042))
* **slider:** Rename `highlightPlacement` to `fillPlacement`
([#9195](#9195))
([9e5a713](9e5a713))
* **slider:** Style ticks according to the fill placement
([#9196](#9196))
([5eb4e10](5eb4e10))
* **stepper-item:** Remove delay in highlighting item
([#8996](#8996))
([bcf23dd](bcf23dd))
* **tree-item:** Do not select when chevron clicked
([#9115](#9115))
([99ad8f1](99ad8f1))


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @esri/calcite-design-tokens bumped from ^2.2.0-next.0 to ^2.2.0
* @esri/eslint-plugin-calcite-components bumped from ^1.2.0-next.1 to
^1.2.0
</details>

<details><summary>@esri/calcite-components-angular: 2.8.0</summary>

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.7.1...@esri/calcite-components-angular@2.8.0)
(2024-04-30)


### Miscellaneous Chores

* **@esri/calcite-components-angular:** Synchronize components versions


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^2.8.0-next.20 to ^2.8.0
</details>

<details><summary>@esri/calcite-components-react: 2.8.0</summary>

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.7.1...@esri/calcite-components-react@2.8.0)
(2024-04-30)


### Miscellaneous Chores

* **@esri/calcite-components-react:** Synchronize components versions


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^2.8.0-next.20 to ^2.8.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants