-
Notifications
You must be signed in to change notification settings - Fork 9
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
Theme previewer: Use listbox for theme patterns & style variation lists to improve selection UX #126
Conversation
…es to match design
I've deployed this so that it can be tested on the live preview site— but I would still appreciate any feedback on the code or design. You can follow the test instructions above. cc @ndiego @StevenDufresne @adamwoodnz @WordPress/meta-design |
this.selected = state.initialSelected > 0 ? state.initialSelected : null; | ||
this.current = null; |
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.
It's not immediately clear to me what the difference is between these two; I'm gaining an understanding as I read through, but I think some doc comments would help.
For keyboard navigation it's definitely a little jarring moving between style variations and patterns on the single theme page (tab stops), so would be good to update the variations to match. |
border-color: var(--wp--preset--color--light-grey-1); | ||
} | ||
|
||
.wp-block-wporg-screenshot-preview:not(.is-preview-image):hover img { |
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.
I find it a bit strange when hovering not to get a pointer style cursor, for elements I can interact with.
Goes for both variations and patterns.
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.
Yeah, the cursor was raised in #128, so that'll be fixed.
Should the theme homepage option then be selected? For me the previewer changes and the item I deselected loses selected state, but the homepage does not become selected. |
Works well for me with keyboard and mouse, however when I have VoiceOver activated I can't select style variations or patterns using my keyboard. I can tab to the lists and they are announced, and I can then navigate within the lists, but enter or space both fail to load the selected item. In this video I am hitting enter or space on each item after it is focused: Kapture.2024-06-18.at.13.06.54.mp4 |
Yeah, I think that would be a good post-launch task.
I'm not sure what you mean by this, there is no "homepage option" for patterns. If you're on the theme homepage, you're not viewing a pattern (some themes do have patterns that reflect parts of the homepage, but those are not the same).
Strange, it was working for me yesterday, I'll try again. |
I think the voiceover issue is not actually an issue— I can trigger the selection by hitting VO+Space which is the way most VO users would use it. Space by itself doesn't work, but I think that's a VO compat issue. |
This updates the "Theme patterns" and "Theme style variations (items)" blocks to use the listbox UI pattern for interaction. In this pattern, the current item has a "selected" role, and is visually (and semantically) flagged. The current selected item can also be (optionally) unselected. This also introduces new styles for hover, focus, and selected states.
The change also applies to the Patterns list on single theme pages, where selecting an item triggers the navigation to open the pattern in the previewer (which is the current behavior). Some of the style changes (hover state) also apply to style variations on the single theme page, but I didn't change the interaction pattern here.
Fixes #113.
Tech details: This introduces a new helper script,
wporgListbox
, a JS class which sets up the listbox behavior for the two blocks. It sets up the interaction listeners (focus, keydown, click), and triggers a custom event for selection that other blocks can listen for. The new listbox elements (ul
s) are a single tabstop in the page, and to navigate through the items you use a keyboard (much nicer on themes with many patterns).Note that I did not use the Interactivity API for this— I ran into an initial roadblock, and decided the
wporgListbox
class would be more understandable.Screenshots
ScreenFlow.mp4
How to test the changes in this Pull Request:
Optionally try this with a screenreader or mobile device.