Skip to content
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

Add a new hasInlineControls option to Image component #164

Open
fabiankaegy opened this issue Oct 27, 2022 · 4 comments
Open

Add a new hasInlineControls option to Image component #164

fabiankaegy opened this issue Oct 27, 2022 · 4 comments

Comments

@fabiankaegy
Copy link
Member

There often are instances where we either need to have multiple images in one block or where the toolbar replaces flow isn't ideal. For those cases, it would be great to also offer an inline solution for replacing and or removing images.

The idea here would be to add a new optional property to the Image component which is a boolean to enable this additional behavior.

When this boolean is present the block will show inline controls when the block is currently selected. These controls should be absolutely positioned so that they don't create any sort of layout shift.

These controls should also take into account whether the image is marked as optional or not. If it is optional the remove button should also be shown inline. Otherwise only the replace button should be shown.

@fabiankaegy
Copy link
Member Author

@xavortm If you have some time and are interested I'd love to get some design input on these controls. I'm struggling with whether they should only be icons that are displayed or buttons with an overlay to dim the image that only get shown when the image is hovered? If so how do we make this accessible etc.

@ncoetzer
Copy link
Contributor

@xavortm If you have some time and are interested I'd love to get some design input on these controls. I'm struggling with whether they should only be icons that are displayed or buttons with an overlay to dim the image that only get shown when the image is hovered? If so how do we make this accessible etc.

@fabiankaegy If I could add my 5cents on this topic 😊 - I think using the overlay concept will look really great! To help with a11y, we could use opacity (and perhaps pointer-events) to hide the buttons/icons, instead of using display: none, so that they are still accessible to readers. Then, to unhide them, we change those properties on focus, focus-within, and hover. If using icons instead of text buttons, we'll need to add some screen reader text to define their context better.

@xavortm
Copy link
Member

xavortm commented Nov 17, 2022

Hey! Here are a few UI examples:

The idea is to use existing Gutenberg UI elements - in this case just the toolbar and position it absolutely, visible just on hover/focus as you've suggested. The three dots is a bonus element for an "additional options" that a developer might want to add.

The second and third example showcase the toolbar position. In the second example, I show the "center" position of the toolbar. It works well if the image has rounded corners. I think it would be great to give a readme example of how to set it when a "rounded corners" style is selected for the image.

The third example is automatic change - if the image is so small that the toolbar won't fit, we change it's position to be outside of the image.

PS: I decided to go with text vs icons as I find it more straightforward and easy to understand + I think it should fit in all regular use cases. I would also suggest that we add an option to replace the "Remove" and "Replce" with icons if needed, but this can be a secondary update to the component

image

image

image

@fabiankaegy
Copy link
Member Author

fabiankaegy commented May 24, 2023

Just noting here that Core recently refined how this gets handled for the post-featured image in WordPress/gutenberg#50269. I think we can draw some inspiration from the implementation there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants