-
Notifications
You must be signed in to change notification settings - Fork 13.5k
feat(picker): picker column is easier to select with assistive technology #29371
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
newOption = this.findPreviousOption(); | ||
break; | ||
case 'PageUp': | ||
newOption = this.findPreviousOption(5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A stride of 5 was based on how browsers handle PageUp/PageDown in scroll snapping containers
if (!this.disabled && option.disabled) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is this if statement had more checks than it needed to. According to the comment, the picker column's disabled state has no bearing on whether or not an option should be rendered as active, so we don't need to check it.
@@ -206,6 +215,10 @@ export class PickerColumn implements ComponentInterface { | |||
} | |||
} | |||
|
|||
connectedCallback() { | |||
this.ariaLabel = this.el.getAttribute('aria-label') ?? 'Select a value'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be convinced to not have a default value here since the label isn't super helpful. However, I was concerned about breaking the existing experience since not using an aria-label
on the slider role could cause tools such as Axe to start failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a label is better than no label here. It is also incredibly straightforward for developers to customize it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving into device testing next, small suggestion on a type adjustment for stricter types.
@@ -206,6 +215,10 @@ export class PickerColumn implements ComponentInterface { | |||
} | |||
} | |||
|
|||
connectedCallback() { | |||
this.ariaLabel = this.el.getAttribute('aria-label') ?? 'Select a value'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a label is better than no label here. It is also incredibly straightforward for developers to customize it.
@@ -2045,6 +2051,7 @@ export class Datetime implements ComponentInterface { | |||
|
|||
return ( | |||
<ion-picker-column | |||
aria-label="Select a day period" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with the naming here.
Co-authored-by: Sean Perkins <[email protected]>
Issue number: resolves #25221
What is the current behavior?
The wheel picker is hard to use with screen readers because users need to manually swipe over every option. Not only is this tedious, it also means it's impossible to move between columns/leave the picker by swiping only because the act of swiping selects a value.
What is the new behavior?
We did investigations into how native handles the wheel picker, and both iOS and Android do it differently. Unfortunately, neither of those patterns can be implemented exactly using web tech (at least not without a lot of brittle/hacky code).
As a result, we settled on a solution that follows web accessibility best practices and addresses the original issue:
role="slider"
which means that screen readers should synthesize keyboard events when swiping. This allows us to resolve the original issue. Users can also double tap to slide up/down to adjust values instead.Note: Chrome currently does not synthesize keyboard events. However, this is something that could be fixed in the future (since Chrome is not following the spec right now). Android users can double tap to slide up/down in the meantime.
Does this introduce a breaking change?
Other information
Dev build:
8.0.0-dev.11713893672.15a2b23e
Reviews: Please test this on iOS and Android!
I recommend testing on
src/components/datetime/test/prefer-wheel
since it lets you the core functionality as well as the datetime-specific integrations (the aria labels)Docs PR: ionic-team/ionic-docs#3612