-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Align Gallery block movers with the standard block mover style #17316
Conversation
Nice, I like the improved consistency. This might belong in a separate PR, but shouldn't the "X" button use a trash icon instead? "X"s are usually associated with closing, not deleting, and Gutenberg already uses a trash icon for removing blocks. |
That's an interesting idea, thanks for bringing it up. I would say no, though. The X, I would suggest, is for dismissing, not closing, whereas the trash can as you note is for deleting. In the context of the gallery you aren't actually deleting an image from your media library, you are dismissing it from the gallery. So I would say the X is still appropriate. But pretty good observation regardless. |
@jasmussen If the images in galleries became nested blocks, wouldn't they then use a trash icon for removal? You're not deleting an image in that case either, but you are deleting an instance of the image. And I'd argue you're doing the same thing here: deleting an instance of an image. |
Another solid observation. Depending on how the child block implementation for gallery looks, it might be more or less relevant. While the benefit of using child blocks is that we get things like the movers, drag and drop and other block properties "for free", to an end user I don't know that they care what those innerblocks are made of, so in a way the UI could and possibly should be the same. There are some further thoughts around those lines in #17267. I'm not strongly attached to this opionion, though, it's mostly an instinct, and if there's agreement the trash icon is the way to go, then blaze a trail. Personally though, I still think logic favors the X. |
I agree that when/if the Gallery block becomes full of nested blocks, a trash can might make more sense there. But in the near term, it makes sense to keep the |
Very nice change. I do wonder about the direction of the arrows — they are always horizontal, but due to the nature of the Gallery block they can sometimes move an image vertically. As for the “x” icon, it’s worth mentioning that this closely matches the design for managing galleries in the Media Modal: |
Yeah, that's definitely a little weird. I'm not sure there's much we can do about that just yet (without a ton of extra code at least), so it may need to wait until we make this a nested block situation, when these movers will be rewritten anyway. In the meantime, I think this PR is good to go. Does someone mind giving it a final review and ✅? |
Followup from #17216.
This PR updates the Gallery block's individual item controls to align with the style used for block movers. As part of that, it switches from using Dashicons for the icons to Material SVGs to match.
Before
After