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

GB 2.0 - Toolbar - missing align / justffy / float button #1408

Open
diggeddy opened this issue Oct 29, 2024 · 8 comments
Open

GB 2.0 - Toolbar - missing align / justffy / float button #1408

diggeddy opened this issue Oct 29, 2024 · 8 comments
Assignees
Milestone

Comments

@diggeddy
Copy link
Collaborator

Description

The toolbar button for text-alignment and image floats are missing from GB 2.0 blocks
For reference these are the the 1.0 toolbar buttons:

Screenshot 2024-10-29 at 18 59 54 Screenshot 2024-10-29 at 19 00 06
@diggeddy diggeddy added the triage Awaiting review label Oct 29, 2024
@diggeddy diggeddy added this to the 2.0.0 milestone Dec 2, 2024
@diggeddy
Copy link
Collaborator Author

diggeddy commented Dec 2, 2024

Float options should really have an option in Styles > Layout

@iansvo iansvo self-assigned this Dec 3, 2024
@iansvo iansvo removed the triage Awaiting review label Dec 3, 2024
@iansvo
Copy link
Collaborator

iansvo commented Dec 3, 2024

@tomusborne Do you hate the idea of just...using the same thing? It seems like this was working previously.

@diggeddy Is there an alternative implementation to the previous one that makes sense?

@diggeddy
Copy link
Collaborator Author

diggeddy commented Dec 3, 2024

A better implementation would be to include Floats as a property in the style tab.
We can leave the toolbar option out, as that would suggest the chosen option "will" work on the chosen block. When that is not necessary the case eg. text-alignment on the figure vs img ....

@tomusborne
Copy link
Owner

I've added text align to the Container and Text blocks, as well as wide/full align support to the Container block. It should be easy to add these to other blocks as well if we want (although I hope we don't have to add wide/full support to any others lol).

I reverted my changes to add float to the image block. Using float was causing some issues in the editor that were tricky (but not impossible) to figure out. However, there's a bigger issue (that @diggeddy mentioned above) of what happens when you float an image inside of a caption. Really, you should be floating the surrounding block instead of the image. Not sure how we want to approach that.

a) We add the float alignment to the Figure and remove from the image if it has a figure.
b) We check if a figure exists when we use the float options in the image, and target that block to set the floats instead of the image.

@iansvo
Copy link
Collaborator

iansvo commented Dec 7, 2024

My only concern is that someone may want to use an image container that isn't a figure. So we ideally need some way to indicate the wrapper is related to the image.

I would strongly caution against making assumptions about markup structure that we don't actually enforce.

@diggeddy
Copy link
Collaborator Author

diggeddy commented Dec 9, 2024

Aside from issues within the editor ... can we just add a Float style to the inspector controls.
Avoid adding logic for things like figure or other wrappers as the user can apply them directly if need be.
We can revisit the UX headaches at a later date, perhaps using patterns

@tomusborne
Copy link
Owner

Floating in the editor can cause some issues without additional logic (weird z-index things), but I think we can go ahead and do it. I'm assuming we'd need a float and clear option?

@diggeddy
Copy link
Collaborator Author

I'm assuming we'd need a float and clear option?

Yeah we need both. We already display: flow-root so thats in place.

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

No branches or pull requests

3 participants