Show a pointer/hint in the settings tab informing the user about the styles tab #47670
Conversation
talldan
added
[Type] Enhancement
talldan
requested review from
richtabor,
jasmussen,
joedolson,
aaronrobertshaw and
alexstine
talldan
added
Backport to WP 6.7 Beta/RC
|
I could do with some design advice:
And also accessibility feedback would be good. I'll copy what I posted in the description:
|
|
Size Change: +360 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 6f5fc38. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4081585737
|
There was a problem hiding this comment.
Thanks for picking this one up @talldan 👍
Potential design and a11y tweaks aside, this one is testing as advertised for me. Both via the mouse or keyboard navigation.
Appreciate the snippet to reset the preference as well, that saved me some time 🙇
Show the text "They've moved to the styles tab" be on a new line?
This would be a little neater in my opinion but for a language other than English it might lead to a worse outcome.
Do you want any kind of animation when dismissing the pointer?
This could be a follow-up task if there is some time pressure to land this early to help introduce users to the tabs.
Screen.Recording.2023-02-02.at.4.01.53.pm.mp4
There was a problem hiding this comment.
Nice, thanks for doing that!
A few very nitpicky details, then this looks great:
- Set
border-radius: $radius-block-ui;on the gray container - Set the padding to
$grid-unit-15instead of$grid-unit-20 - You might need to fiddle the dismiss button paddings down to the 05 unit to match
|
Context: Yeah, that's a bit difficult. In reading mode, a screen reader user would read the text and then find the dismiss button, but if they were navigating by tabs, they wouldn't. I wouldn't go overboard with providing context: something as simple as "Dismiss Notice" is enough to let a user know that they should explore the notice and let them know that this isn't a control to dismiss a control panel, etc. I favor lower verbosity, so that's what I would choose to do. @alexstine might have different preferences. Focus movement: focus should move to the nearest focusable point. Usually, that would be the next focusable element; but because this is at the end of the settings panel, I think it would make more sense to move the user to the previous focusable element, which should be the 'Advanced' settings toggle. The next focusable is the link to move to the currently selected block, and that would also be fine, though more surprising to sighted mouse users, who wouldn't normally see that control. Have you considered making the text 'styles tab' a link that can navigate the user into the styles tab? If a user does encounter this notice, it could be because they are looking for those settings, and offering an easy path to reach them immediately would be useful. |
@joedolson this would be a nice touch. Unfortunately, at the moment the |
There was a problem hiding this comment.
@talldan I agree with Joe, I think previous element to focus is okay and this makes sense due to the tab order. Someone would tab on advanced first before receiving the dismiss button.
Since this notice is not overly important, I do not believe we should add verbosity. There is an argument to be made though on how useful this notice would be at all considering a user would have to tab through the entire inspector to reach it. Before the new tabs, didn't the styles section appear before the advanced options? Might be worth moving that notice directly to the position where those style settings would have appeared in the DOM.
EDIT: One other note, can you think of something else to call this besides a settings pointer? Maybe a settings notice pointer? Looking at the code, the import statement, I would have no idea what this component is.
| label={ __( 'Dismiss' ) } | ||
| onClick={ () => { | ||
| setPreference( 'core', PREFERENCE_NAME, false ); | ||
| speak( __( 'Notice dismissed.' ) ); |
There was a problem hiding this comment.
I think this speak is not needed as long as you manage the focus. I think the context is good enough here with the label itself for a user to know what happened. The only time I really worry about notifying users is if it is urgent information/important information. Examples might include submitting a form and receiving a success/error message or providing navigation instructions in a non-native element, this one is closely related to Gutenberg select/edit modes.
talldan
force-pushed
the
add/split-inspector-pointer
branch
from
b381728
to
2e5c47d
Compare
|
Thanks for all the feedback. It was incredibly useful. I've made some changes:
@alexstine That's tricky to do as there's no space, and on the other hand there's lots of empty space after Advanced. Given the low prominence of this message, I think this is the right trade-off. |
talldan
force-pushed
the
add/split-inspector-pointer
branch
from
2e5c47d
to
6f5fc38
Compare
|
@talldan Fair enough. I just wanted to make sure it was not too undiscoverable for people who could not see it all the way down there. No reason to worry though I guess if it is not meant to be an essential message. I will try to give this a test this weekend or maybe tomorrow. Really behind on things after a winter storm ripped through Texas. Thanks. |
|
Thanks Alex, hope the storm hasn't caused you too many issues. |
There was a problem hiding this comment.
Thanks for addressing all the feedback @talldan 🚀
The recent changes look good and test as advertised. In particular, the focus management and moving focus to the previous element when the hint is dismissed works well.
I'm happy to approve this now but waiting for Alex's testing and feedback might be wise. To make that a little easier, perhaps we could update the PR's description and test instructions to reflect the current state of the PR?
Screen.Recording.2023-02-03.at.5.18.39.pm.mp4
Oh yep, thanks for the reminder 👍 |
|
Good job team! |
talldan
removed
Needs Design Feedback
…styles tab (#47670) * Show a pointer/hint in the settings tab informing the user about the styles tab * Add a spoken message when the notice is dismissed * Update margin around text and button * Remove spoken message * Rename to hint and add focus management code * Relabel dismiss button
|
Cherry-picked this PR to the wp/6.2 branch. |
ntsekouras
removed
the
Backport to WP 6.7 Beta/RC
What?
Fixes #47410
Why?
It's nice to let the user know about this change
How?
Noticewasn't used)Testing Instructions
Note - if you want to test this multiple times, you can run this snippet in the browser console to bring the dismissed pointer back again:
Testing Instructions for Keyboard
/groupand hitting the enter keyScreenshots or screencast