Skip to content

[Data Views] Don't fully disable buttons that will change state based on user action #56396

Closed

Description

@andrewhayward

What problem does this address?

As per WCAG SC 3.2.2 (On input):

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.

A set of pagination controls, with disabled buttons for the first page, previous page, next page, and last page, all circled in red.

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.

A disabled button, titled 'Reset', circled in red.

This means that the "reset filters" button, for example, visually appears to have this focus problem, but in fact does not.

Activity

  1. added
    [Focus] Accessibility (a11y)Changes that impact accessibility and need corresponding review (e.g. markup changes).
    [Feature] DataViewsWork surrounding upgrading and evolving views in the site editor and beyond
    on Nov 21, 2023
  2. ntsekouras commented on Nov 22, 2023

    @ntsekouras
    Contributor

    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

  3. ciampo commented on Nov 23, 2023

    @ciampo
    Contributor

    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?

  4. afercia commented on Nov 27, 2023

    @afercia
    Contributor

    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.
  5. ciampo commented on Nov 28, 2023

    @ciampo
    Contributor
    • 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?

  6. afercia commented on Nov 30, 2023

    @afercia
    Contributor

    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.

  7. andrewhayward commented on Nov 30, 2023

    @andrewhayward
    ContributorAuthor

    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.

  8. afercia commented on Dec 4, 2023

    @afercia
    Contributor

    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:

    • screen reader users
    • low vision users
    • users with cognitive impairments
  9. joedolson commented on Dec 7, 2023

    @joedolson
    Contributor

    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.

  10. alexstine commented on Dec 7, 2023

    @alexstine
    Contributor

    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.

    <label for="input1">Go to page</label>
    <input type="number" id="input1" min="1" max="50" value="1" />
    <input type="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.

    Thanks.

  11. afercia commented on Dec 13, 2023

    @afercia
    Contributor

    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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

[Feature] DataViewsWork surrounding upgrading and evolving views in the site editor and beyond[Focus] Accessibility (a11y)Changes that impact accessibility and need corresponding review (e.g. markup changes).[Status] In ProgressTracking issues with work in progress[Type] EnhancementA suggestion for improvement.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    Sponsor
    SponsoredKunjungi sekarang
    Promo