-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
List View: add an indicator of when a position type is set for a block #49122
List View: add an indicator of when a position type is set for a block #49122
Conversation
Size Change: +7.37 kB (+1%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
Before this gets merged, lets do figure out where this info might be useful for screen readers. 👍 |
Flaky tests detected in aecbf4f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5306460928
|
Absolutely! Happy for any feedback or ideas on how best to word the label for screen readers. |
It definitely becomes tricky when there's both anchor and sticky chips. But that treatment still seems the most compelling to me, outside of a better icon. Question: would it be inappropriate for the "Sticky" chip to take precedence if the list view contains both a sticky chip, and an anchor chip? The latter feels more quality of life to me, and is also surfaced in the inspector. Alternatively, is it possible to hide these chips, one by one, as the title of the block grows longer, or nesting grows deeper? This is probably something to consider separately. |
I'm not sold we need a sticky chip. Why should sticky be called out here, when nearly all other attributes for the group aren't? I'm more in favor of adapting the block icon to indicate stickiness, if we need/should do this. |
Thanks for the feedback and ideas, folks!
I agree — I think it's the clearest indicator visually, and when I attempted to get both the anchor and the sticky chip/pill to be on the same row, I very quickly ran into styling issues with the limited real estate we have there. If we agree on a sticky chip, I think the most pragmatic option is to hide the anchor if it exists, and like you mention, we can always look at seeing if we can get them both displayed in a follow-up if need be.
That's a good question — I think the position type is worth calling out within the list view because unlike other block supports / styling settings, position affects where the block appears within the page. In a sense it breaks out of its position within the flow of the document, so I think it'd be good to find ways that make it easier for the user to get back to that element or select it. Also, there'll likely be very few elements on the page that are sticky, so it hopefully won't result in too much visual clutter.
The main drawbacks I can see for adjusting the block icon are:
I've added the "Needs Design" label — I'm happy to have a go at implementing whichever idea we think will work best here, but since it's a little fiddly to work with, if anyone's free to put together a mockup of how it'd ideally look, that would be very helpful! Some ideas:
I'll keep chipping away at this PR next week! |
Some of the motivation is the lack of an immediate visual effect when toggling sticky. But to your point, which is a good one, there are other ways to do this. I do like your idea of an icon change. Figma has this toggle for abs position: In the layers panel, the icon then gets those 4 corners added. In this example, a symbol instance before and after stickied: That does seem less intrusive, and perhaps simpler to implement? |
I've read the other associated issues /iteration as well. IMHO the main stakes, from an end user /editorial point of view, are :
here is what i can see on some of my websites with Senff's plugin (https://wordpress.org/plugins/sticky-block/), just to compare Would be nice to check what other builders do. |
Thanks for the feedback and ideas, folks! I've had a go at trying @jasmussen's idea of adding an indicator on top of the block icon. I don't mind it, but I'm also a little torn — it does add an indicator, but it also looks a little busy to my eyes, and isn't clear to me what it does. But perhaps it's clearer from context when a user interacts with it. Here's screenshot of the current state of this PR: And a screengrab of switching position types: If people like this approach, then I think the next steps would be:
Thanks again for the collaboration! |
Thanks for trying that. Outside some treatment details, I think it's interesting. What do you think, @WordPress/gutenberg-design |
It seems plausible that we might want to indicate other position properties like Maybe just a single stroke at the top of the svg?
I feel this too. It would be nice to avoid additional text, but it doesn't seem like the end of the world to me, especially as (I assume) we'll at least need a VisuallyHidden label for a11y. |
A big +1 from me for using something that makes it immediately obvious what it means. I agree that additional text here is not the end of the world and would improve clarity in a UI that is designed to improve clarity of the entire document structure. |
Yes, I think I'm leaning toward returning to text display of the position status, too. If we switch this PR back to using text to indicate the position type, do we have a preference for:
|
Definitely worth trying! I'm happy to give it a go. |
8dab1b1
to
1cd0e32
Compare
Can we try a plain icon next to the ellipsis menu like we communicate "locked" state? This seems very ad hoc |
Sure, I really like that idea. I think earlier feedback from @richtabor in #49122 (comment) indicated trying to make it less prominent, but that was when the discussion was about a text-based chip rather than a separate icon. If any designers have an icon to share that I can play with, I can put it together! |
The challenge has always been coming up with a compelling icon to indicate sticky positioning. Iirc this is why the text label was originally suggested. There are metaphors like pins / map markers, anchors, magnets... but for one reason or another they don't feel quite right. Trying to communicate the behavior more literally is very tricky in such a small space. |
@jameskoster I think it's more about adding a marker so the block stands out. Once you associate it to the behavior it flows more easily. |
personaly i vote for a strict vertical pin ;-) |
The pin icon next to the lock icon looks like a good approach to try to me! I'll have a go at that next week unless there are any other ideas in the meantime. |
Great work! Another idea would be a mix of the lock icon and a vertical or horizontal line around it depending on which part is sticky. For example:
And versions for mix of positions for example: sticky to both right and bottom. Other sticky positions aren't available yet (it’s only sticky to the top if I’m not wrong) and still need to be considered in terms of UI as listed in #47043:
But it might be a good thing to take those future options in consideration for creating the proper icon(s). |
Thanks for the feedback, all! I think for simplicity sake, trying out a sticky icon that is similar in size to the lock icon could be good, and possibly a pragmatic path forwards. I've updated this PR to use the existing If this direction seems okay, then I think the next step would be for us to add a |
@andrewserong i love this new iteration / icon, keep it up ! good job |
Thanks @andrewserong, I think we can slightly scale up the shape in the
|
Thanks @jameskoster, that's looking good to me! I've added a I think this should be ready for a final review now. |
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 working well for me.
Thanks for the review @jameskoster, and for minifying the SVG. I'll merge this in now. |
WordPress#49122) * List View: add a position label when a position type is set for a block * Move get label function to the useBlockDisplayInformation file * Try using icon over the block icon instead of a text label to indicate sticky status * Try more subtle icon / styling idea, and add in a tooltip * Try provided svg icons * Don't change color when hovered * Try separate sticky icon instead * Add pinSmall icon * Minify pinSmall * Add additional optional chaining because attributes can technically be null --------- Co-authored-by: James Koster <james@jameskoster.co.uk>
What?
Part of #47043
Add an indicator to the block icon in the list view to flag a position type when one is set.
This follows on from feedback in #49085 (comment)
Why?
When a user sets a Group to sticky, it would be helpful for the sticky state to be visible from within the list view.
How?
useBlockDisplayInformation
hook to retrieve and pass along the position label and position type.pinSmall
icon.sticky
, display a pin icon next to where the lock icon is displayed.Testing Instructions
appearanceTools
set to true), add a Group block at the root level of a post, or at the root level of a template.Screenshots or screencast