WordPress / openverse

Openverse is a search engine for openly-licensed media. This monorepo includes all application code.
https://openverse.org
MIT License
235 stars 186 forks source link

Focus style visible in active tab in the attribution box #532

Open fcoveram opened 2 years ago

fcoveram commented 2 years ago

Description

In the single result view, the active tab in "Credit the creator" section keeps its focus style after clicking on it to see the attribution content.

Reproduction

  1. Search for anything.
  2. Click on any result to see its details.
  3. Click on any inactive tab in the "Credit the creator" section. You will see it working correctly.
  4. Refresh the page.
  5. Click again on any inactive tab.
  6. See error.

Screenshots

Resolution

krysal commented 2 years ago

What should be the expected behavior?

fcoveram commented 2 years ago

To change the tab style without adding the focus ring. If you follow all steps except step 4, you will see that it works correctly. Here is how I see it working properly.

krysal commented 2 years ago

The focus style is an a11y feature that we implemented consciously on WordPress/openverse-frontend#1316, following W3's guidelines. The error seems to be on the focus style not appearing on client-side navigation 😅

Do you want to rethink this style?

zackkrida commented 2 years ago

@krysal this sounds like a bug where our focus-visible styles aren't working on the server-rendered version of the page then!

Current Behavior

krysal commented 2 years ago

@zackkrida The examples of the W3's ARIA Authoring Practices Guide (APG) show the focus style with clicking as well.

zackkrida commented 2 years ago

It does show it, but it's not clear to me if that's a requirement or just a suggestion. The text in the "Accessibility Features" section is a little vague to me.

Either way, I think we should definitely show it on focus-visible and that there's an odd discrepancy in our code that we should figure out. Let's wait until we confirm with @panchovm about if we want to show the focus ring on click or not though before trying to fix the bug. Whichever outcome we want, the behavior should be the same in each rendering mode.

fcoveram commented 2 years ago

Maybe @sarayourfriend know if it is a requirement or not.

Showing the focus ring in the active state makes sense when navigating with the keyboard, where jumping across elements needs to be shown. Otherwise, the active style of the tab (without the ring, as this gif) seems enough to indicate the tab state

sarayourfriend commented 2 years ago

I don't think showing a focus ring is a requirement of active tabs. Some active tab indication is necessary and I think we already do have that with the outline changes, though those might not be significant enough to be considered accessible. The contrast of the outline, for example, is not accessible and therefore it may not serve appropriately as an active tab visual indication just to have the outline change to the active tab.

My suggestion would be to use background colors as the primary accessible active tab indication and to change the focus rings to be focus-visible so that they're only activated by keyboard navigation (as suggested in the original version of this issue).

Just my "two cents" though.

sarayourfriend commented 2 years ago

Although, now that I'm looking at other accessible tab implementation examples, I think @krysal is probably correct that the focus style should also be preserved. https://design-system.service.gov.uk/components/tabs/ for example also preserves the focus style when clicking. https://ariakit.org/examples/tab on the other hand, does not, and I do tend to trust Ariakit's implementations of things.

I'll keep my recommendation the same, but an alternative would be to also keep the focus style. Regardless I think additional work is needed to make the currently active tab's visual indication accessible under all conditions (focused and unfocused).

obulat commented 1 month ago

Blocked by the code freeze during Nuxt 3 migration.