You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Data Views] Don't fully disable buttons that will change state based on user action #56396
Changing the setting of any user interface component does not automatically cause a change of context unless the user has been advised of the behavior before using the component.
A 'change of context' is defined as something "that, if made without user awareness, can disorient users who are not able to view the entire page simultaneously". This includes focus.
Disabled controls cannot natively have focus. This means that if the act of interacting with a control (for example pressing a button) disables it, then focus will move somewhere else. Without careful focus management, this can be anywhere, and usually nowhere related to the task at hand.
There are several buttons used by data views that fall into this trap.
For example, when the "first page" button in the pagination controls is pressed, the user is taken to the first page of results, and the button is disabled, but it is not clear where focus moves to. The same also applies to the "previous", "next" and "last" buttons.
What is your proposed solution?
Rather than set disabled on these buttons, we should instead use aria-disabled. This will leave the buttons focusable, but announced as disabled to assistive technologies.
The most appropriate way to do this is by setting both disabled and __experimentalIsFocusable on the Button component.
Note
There is a problem with focus rings on some focusable disabled buttons, but this is noted and being fixed.
This means that the "reset filters" button, for example, visually appears to have this focus problem, but in fact does not.
Thanks for the issue! I opened a quick PR to fix this, but I'm wondering if there are cases where we don't need this for Buttons, since it's the best a11y practice. Should this be the default behavior for Button component? 🤔 --cc @ciampo
We could take this into account, yeah. @andrewhayward@afercia , do you see any potential drawbacks if the Button component had the __experimentalIsFocusable prop true by default?
Late to the party. This is very similar to what happens in the Patterns explorer modal dialog, see #55110 / #56162
do you see any potential drawbacks if the Button component had the __experimentalIsFocusable prop true by default?
Not sure that would be ideal. I'll create a separate issue or maybe a discussion.
As an aside, there are two more problems to consider for this pagination:
The number filed triggers an unexpected change of context because it uses onChange. As soon as the number value is changed via keyboard, the view updates. That's a context change that needs to be avoided and onChange in these cases is an accessibility anti=pattern that needs to be banned.
Both the data views and the Patterns axplorer have a Pagination component. They are very similar and basically they are code duplicaiton. They should abstracted to a separate, reusable, component.
The number filed triggers an unexpected change of context because it uses onChange. As soon as the number value is changed via keyboard, the view updates. That's a context change that needs to be avoided and onChange in these cases is an accessibility anti=pattern that needs to be banned.
Just to make sure folks can look into improving this, what would be a better solution? Could we use an aria-live region to notify users about what updated?
Just to make sure folks can look into improving this, what would be a better solution? Could we use an aria-live region to notify users about what updated?
I don't think that would solve the issue. It's not just about screen readers. It's general interaction: changing the value should never trivver a page update. Say I want to go from page 1 to page 10. That would trigger 10 page updates. As a user, that's not what I want.
This fields should not use onChange. It should allow users to change the value and submit.
We'd need to replicat ethe same experience and interaction as it is used in the core admin pagination, where pagination triggers on submit.
The number filed triggers an unexpected change of context because it uses onChange. As soon as the number value is changed via keyboard, the view updates.
Strictly speaking, onChange doesn't fire until the user hits enter, or the field loses focus. Which, to me, would be sufficient in this case to indicate a user-initiated and intended action.
That being said, it looks like we're actually using onInput (or at least replicating it), and the data is being reloaded with every single keypress, which I agree is not a good experience.
That's a context change that needs to be avoided and onChange in these cases is an accessibility anti=pattern that needs to be banned.
I'm not sure as I entirely agree with this. As per the spec...
Tip
A change of content is not always a change of context. Changes in content, such as an expanding outline, dynamic menu, or a tab control do not necessarily change the context, unless they also change one of the above (e.g., focus).
Important
Opening a new window, moving focus to a different component, going to a new page (including anything that would look to a user as if they had moved to a new page) or significantly re-arranging the content of a page are examples of changes of context.
From my perspective, we're not doing anything that would qualify as a change in context here, though I'm happy to hear any thoughts on why we might be. We're not reloading or navigating elsewhere, focus is remaining correctly, and the content is not being significantly rearranged (to follow the examples given).
The only part of the page that changes is the data table, which, honestly, I would entirely expect on a page transition. The rest of the supporting UI doesn't change at all. I don't think the change in content here changes the meaning of the page, or its context.
Strictly speaking, onChange doesn't fire until the user hits enter, or the field loses focus. Which, to me, would be sufficient in this case to indicate a user-initiated and intended action.
That being said, it looks like we're actually using onInput
Oh yes, I think I had in mind the onChange event for select elements, thanks for correcting me.
From my perspective, we're not doing anything that would qualify as a change in context here
Well... in the spec you quoted there is a relevant part. It should link to the WCAG 2.2 though, as that's the current spec.
going to a new page (including anything that would look to a user as if they had moved to a new page) or significantly re-arranging the content of a page
To me the parts
'including anything that would look to a user as if they had moved to a new page'
'or significantly re-arranging the content of a page'
fully qualify updating the table as a change of context.
The only part of the page that changes is the data table, which, honestly, I would entirely expect on a page transition. The rest of the supporting UI doesn't change at all. I don't think the change in content here changes the meaning of the page, or its context.
I kindly disagree. The 'only' part that changes is actualy the main content of the page. All the rest is the UI frame, which is not relevant to the information provided by the page.
I don't think the change in content here changes the meaning of the page
Hum, it does change it. It's a new page, with new data presented to the user. I would agree the structure of the page does not change. The actual data changes though.
I don't think such a change would be accessible for, for example:
First of all, I don't think that a field losing focus should be interpreted as user intent to change context. It can just as easily be a user changing their mind or exploring the page to get more information before they commit to a page change. If this change occurs on a focus loss, the user effectively has no way to leave the field without updating the results.
Even if resetting the value to the current page would avoid updating results, that depends on the user a) knowing what page they were on when they started changing the value and b) requires the user to lose the value they had typed in. Both of these are problems for accessibility; in the first case, screen readers may need to explore the page to identify the current page and in the second case, users with memory or cognitive impairments may struggle to adjust state.
As far as the actual definition of a context change, I agree that this is fuzzy. However, I would argue that going to a new page of results is not just something like a user going to a new page, it is literally a new page. All of the relevant content of the page has now changed.
Most importantly, to me, a change that happens when focus moves is unexpected, and I'm uncomfortable with any change that isn't explicitly requested by a user. Users shouldn't expect moving focus to result in content changing.
Yeah, this is not a good interaction. I understand it from a matter of web development but this doesn't make sense for keyboard users. The only way a pagination input like this works is with a submit button. E.g.
<labelfor="input1">Go to page</label><inputtype="number" id="input1" min="1" max="50" value="1" /><inputtype="submit" value="Go to page" />
Expecting someone to know there is an event listener on a field is the very problem with front-end development today. Not saying anyone specific to WordPress, just in general. Actions should be explicit, not guesses.
I'm uncomfortable with any change that isn't explicitly requested by a user
This. Thanks Joe and Alex.
For history: prior Trac ticket about the use of onchange event. Although related to <select> elements, the root problem is the same: https://core.trac.wordpress.org/ticket/40925
Activity
ntsekouras commentedon Nov 22, 2023
Thanks for the issue! I opened a quick PR to fix this, but I'm wondering if there are cases where we don't need this for Buttons, since it's the best a11y practice. Should this be the default behavior for
Buttoncomponent? 🤔 --cc @ciampociampo commentedon Nov 23, 2023
We could take this into account, yeah. @andrewhayward @afercia , do you see any potential drawbacks if the
Buttoncomponent had the__experimentalIsFocusableproptrueby default?afercia commentedon Nov 27, 2023
Late to the party. This is very similar to what happens in the Patterns explorer modal dialog, see #55110 / #56162
Not sure that would be ideal. I'll create a separate issue or maybe a discussion.
As an aside, there are two more problems to consider for this pagination:
onChange. As soon as the number value is changed via keyboard, the view updates. That's a context change that needs to be avoided andonChangein these cases is an accessibility anti=pattern that needs to be banned.Paginationcomponent. They are very similar and basically they are code duplicaiton. They should abstracted to a separate, reusable, component.ciampo commentedon Nov 28, 2023
Just to make sure folks can look into improving this, what would be a better solution? Could we use an aria-live region to notify users about what updated?
afercia commentedon Nov 30, 2023
I don't think that would solve the issue. It's not just about screen readers. It's general interaction: changing the value should never trivver a page update. Say I want to go from page 1 to page 10. That would trigger 10 page updates. As a user, that's not what I want.
This fields should not use
onChange. It should allow users to change the value and submit.We'd need to replicat ethe same experience and interaction as it is used in the core admin pagination, where pagination triggers on submit.
andrewhayward commentedon Nov 30, 2023
Strictly speaking,
onChangedoesn't fire until the user hits enter, or the field loses focus. Which, to me, would be sufficient in this case to indicate a user-initiated and intended action.That being said, it looks like we're actually using
onInput(or at least replicating it), and the data is being reloaded with every single keypress, which I agree is not a good experience.I'm not sure as I entirely agree with this. As per the spec...
Tip
A change of content is not always a change of context. Changes in content, such as an expanding outline, dynamic menu, or a tab control do not necessarily change the context, unless they also change one of the above (e.g., focus).
Important
Opening a new window, moving focus to a different component, going to a new page (including anything that would look to a user as if they had moved to a new page) or significantly re-arranging the content of a page are examples of changes of context.
From my perspective, we're not doing anything that would qualify as a change in context here, though I'm happy to hear any thoughts on why we might be. We're not reloading or navigating elsewhere, focus is remaining correctly, and the content is not being significantly rearranged (to follow the examples given).
The only part of the page that changes is the data table, which, honestly, I would entirely expect on a page transition. The rest of the supporting UI doesn't change at all. I don't think the change in content here changes the meaning of the page, or its context.
afercia commentedon Dec 4, 2023
Oh yes, I think I had in mind the
onChangeevent forselectelements, thanks for correcting me.Well... in the spec you quoted there is a relevant part. It should link to the WCAG 2.2 though, as that's the current spec.
To me the parts
fully qualify updating the table as a change of context.
I kindly disagree. The 'only' part that changes is actualy the main content of the page. All the rest is the UI frame, which is not relevant to the information provided by the page.
Hum, it does change it. It's a new page, with new data presented to the user. I would agree the structure of the page does not change. The actual data changes though.
I don't think such a change would be accessible for, for example:
joedolson commentedon Dec 7, 2023
First of all, I don't think that a field losing focus should be interpreted as user intent to change context. It can just as easily be a user changing their mind or exploring the page to get more information before they commit to a page change. If this change occurs on a focus loss, the user effectively has no way to leave the field without updating the results.
Even if resetting the value to the current page would avoid updating results, that depends on the user a) knowing what page they were on when they started changing the value and b) requires the user to lose the value they had typed in. Both of these are problems for accessibility; in the first case, screen readers may need to explore the page to identify the current page and in the second case, users with memory or cognitive impairments may struggle to adjust state.
As far as the actual definition of a context change, I agree that this is fuzzy. However, I would argue that going to a new page of results is not just something like a user going to a new page, it is literally a new page. All of the relevant content of the page has now changed.
Most importantly, to me, a change that happens when focus moves is unexpected, and I'm uncomfortable with any change that isn't explicitly requested by a user. Users shouldn't expect moving focus to result in content changing.
alexstine commentedon Dec 7, 2023
Yeah, this is not a good interaction. I understand it from a matter of web development but this doesn't make sense for keyboard users. The only way a pagination input like this works is with a submit button. E.g.
Expecting someone to know there is an event listener on a field is the very problem with front-end development today. Not saying anyone specific to WordPress, just in general. Actions should be explicit, not guesses.
Thanks.
afercia commentedon Dec 13, 2023
This. Thanks Joe and Alex.
For history: prior Trac ticket about the use of
onchangeevent. Although related to<select>elements, the root problem is the same: https://core.trac.wordpress.org/ticket/40925