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

DataViews: Patterns list: initial focus unexpectedly set on the FilterVisibilityToggle button #67726

Open
2 of 6 tasks
afercia opened this issue Dec 9, 2024 · 5 comments
Open
2 of 6 tasks
Labels
[Feature] DataViews Work 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). [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Dec 9, 2024

Description

When navigating with the keyboard the menu items in the DataViews navigation panel, focus is supposed to stay on the activated navigation item.

However, in the Patterns list, after navigation, focus goes to the FilterVisibilityToggle button. This is unexpected and very confusing for keyboard users.

Animated GIF to illustrate:

Image

To my understanding, #67003 introduced a fix to prevent a focus loss when removing the filters.

However, at least for me, the filters in the Patterns list are initially open. As such, focus is moved to the FilterVisibilityToggle button when it renders. I'm not sure why the filters are open in the first place. This is the only view where the filters are open by default. Anyways, the fact they are open by default is incompatible with the logic for hte focus introduced in #67003 which moves focus to the button on first render.

Cc @ntsekouras

Step-by-step reproduction instructions

  • Go to the Site editor.
  • Use the keyboard to navigate.
  • Navigate the items in the navigation panel on the left e.g. go through the Templates categories.
  • Observe that when activating one of the navigation items by pressing the Enter key, focus stays on the active item.
  • Go to Patterns and navigate the categories.
  • Observe that when activating one of the navigation items by pressing the Enter key, focus unexpectedly moves to the filters toggle button.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@afercia afercia added [Feature] DataViews Work 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). [Type] Bug An existing feature does not function as intended labels Dec 9, 2024
@ntsekouras
Copy link
Contributor

focus introduced in #67003 which moves focus to the button on first render.

Actually that PR had logic to move focus on unmount, but it seems the whole DataViews are unmounted for some reason before and creates the bug. I'll look into an alternative.

@ntsekouras
Copy link
Contributor

I spent a lot of time today trying to figure out how to properly make this work properly by fixing this issue but also handle the focus loss on resetting/removing filters. I tried lots of stuff without success, like:

  1. using useFocusReturn with an onFocusReturn argument which contained a buttonRef on Filters component. That buttonRef was also being passed in FiltersToggle.
  2. creating a ref for Filters component which was passed in FiltersToggle and try to see if the previous active element was inside filters and only then focus (some similar logic with the internals of useFocusReturn).
  3. passing around refs of FiltersToggle button and try to handle focus inside Filters on onClick event of resetting/removing.
  4. creating a custom JS event and dispatch either onClick of reset or on unmount of Filters.
    In any case the problem lies to the FiltersToggle and Filters being in different components and Filters unmounts always first, so we don't have the right ref for FiltersToggle or affects other solutions (like 2) because the ref is not even in DOM at that point.

Additionally we can't rely on a useEffect that does something with focus on unmount because the whole DataViews component unmounts very early before the final rendering and I'm not sure if we can avoid that somehow (that's actually is the reason for this issue).

Those components being separate is making the solution harder and another nuance is that FiltersToggle in some cases renders a menu and in others a button.

Any ideas would be really welcome. I'll -cc just a few folks for possible ideas @ciampo @tyxla @youknowriad @jsnajdr

@afercia
Copy link
Contributor Author

afercia commented Dec 11, 2024

@ntsekouras thanks for your thorough investigation.

I'm not sure why the filters are open in the first place

I noticed focus moves to the FiltersToggle also in the Templates list, while it doesn't on the Pages list. However, one difference between Patterns and Templates is that in the Templates list the filters UI stays closed. Any idea why in the Patterns the filters UI is expanded by default and insteead in Templates it's collapsed?

GIF:

Image

@ntsekouras
Copy link
Contributor

Any idea why in the Patterns the filters UI is expanded by default and insteead in Templates it's collapsed?

The reason is that in patterns the filter is a primary one (API).

@afercia
Copy link
Contributor Author

afercia commented Dec 11, 2024

The reason is that in patterns the filter is a primary one (API).

Thanks for your feedback @ntsekouras. It's not entirely clear to me what that means because I'm not familiar with the DataViews. Regardless, I'm wondering whether presenting this UI expanded by default only for Patterns makes sense from an user perspective. I'm not sure it does. As an user, I don't understand the difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] DataViews Work 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). [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants