-
Notifications
You must be signed in to change notification settings - Fork 64
Add utilities for consistent focus rings #2067
Conversation
Storybook and Tailwind configuration previews: Ready Storybook: https://wordpress.github.io/openverse-frontend/_preview/2067 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
Size Change: -2.93 kB (0%) Total Size: 873 kB
ℹ️ View Unchanged
|
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.
Thank you for adding this, it's really helpful!
We need to support color customization for the focus ring: for the internal header modal, the ring is yellow on a dark-charcoal background. Also, which variant should we use for the filter sidebar which has a lighter charcoal background?
Hmm, I assumed the 2x2 matrix of focus styles shared by @panchovm in the comment was exhaustive! I'll go through the mockups and see which styles are missed, starting with the two cases you shared @obulat, thanks! |
The checkbox component is fascinating when it comes to focus styles. The version in the WordPress design library is very different from our implementation. In these, the variant seems to be slim-transparent (as the border itself gets colored on focus) but with a white offset appearing INSIDE the component (which affects the inner square's size). |
Thanks @obulat for mentioning the color variant. That was the only remaining point I missed. The color accents we are using are The focus styles designed are based on the WordPress example you shared @dhruvkb. Although the inside border is They look like this in Figma WordPress Design LibraryOpenverse Design Library |
@panchovm two follow up questions
|
To your questions @dhruvkb
Yes. Openverse focus styles are fully based on the WordPress one. The checkbox, and probably a few other UI elements that I am missing, are pulled from the WordPress Design Library directly, which is why they do not exist in Openverse DL.
To not block any code work, I can update this diagram with the new values: Otherwise, I need to check all components in the Design Library and update the in progress Figma files. Design files marked as done are not updated nor reviewed since the PRs related to those mockups were merged. What do you think? |
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.
This will be such a great dev-ex change! I've had hard times with the focus rings on several occasions.
I think there are just two things I'd like address here: the dark background visual regression tests, and the checkbox border color.
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.
LGTM!
Thank you for explaining the checkbox state changes.
Note on the Playwright tests: the pages tests can be flaky. The page is rendered correctly, but when you take the snapshot, the page content is shifted to the left side. I think the easiest quick fix for this is to set |
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.
Storybook
Stories are accurate 👌 they look awesome.
Dev
All components look excellent except for the Filters button.
In the resting state, I see the grey border during focus. In the active state, the slim-filled
style does not mask the content, it shows pink and white borders as outside borders.
Here is a screen recording from Chrome inspector.
CleanShot.2022-12-29.at.10.13.17.mp4
Forgot to mention that, in this comment, I added an updated version of the focus styles diagram that has the color variant and the |
Playwright Failure Test Results It looks like some of the Playwright tests failed. You can download the Playwright trace https://github.com/WordPress/openverse-frontend/actions/runs/3799307570 Read more about how to use this artifact here: https://github.com/WordPress/openverse-frontend/blob/main/test/playwright/README.md#debugging |
@panchovm in this PR I decided not to update the Button component because that was a huge change affecting lots of components. I'll handle that separately. |
Merging to unblock working on the |
No problem 👍 Excellent work. |
Fixes
Related to the comment WordPress/openverse#479 by @panchovm
Description
This PR
VCheckbox
component to match the WordPress component libraryThis PR does not
These new utilities do not include any transitions on the shadows. This is intentional as the focus movement reacting to the Tab must be as fast as possible. If needed in some specific cases, it can be restored with the
.transition-shadow
utility.Testing Instructions
rounded-sm
,rounded
androunded-full
to see how the ring behaves.i.
Filters button in the new header (slim filled style)ii. Checkboxes in the filters sidebar (slim transparent style with a slight mod)
iii. Image cell in the search results (bold filled style)
iv. Audio cell in the search results (bold filled style)
v. Audio row in the search results (slim transparent style)
Screenshots
With additional
rounded-full
style using the 'Controls' panel.Checklist
Update index.md
).main
) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin