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.

Sign up for GitHub

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

Jump to bottom

Blocks: Broken layout when multiple floated blocks in succession #3870

Closed
gziolo opened this issue Dec 8, 2017 · 4 comments
Closed

Blocks: Broken layout when multiple floated blocks in succession #3870

gziolo opened this issue Dec 8, 2017 · 4 comments
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Milestone
Merge: Editor

Comments

@gziolo
Copy link
Member

gziolo commented Dec 8, 2017

Steps to Reproduce (for bugs)

  1. Add an image block.
  2. Make it aligned to left.
  3. Add a button block.
  4. Make it aligned to left.
  5. Add another block.

Browser: Chrome
System: macOS

Expected Behavior

Blocks are properly aligned.

Current Behavior

Broken layout when multiple floated blocks in succession.

Screenshots / Video

screen shot 2017-11-30 at 10 09 05

screen shot 2017-12-08 at 15 18 24

/cc @jasmussen
@gziolo gziolo added [Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended labels Dec 8, 2017
@jasmussen jasmussen mentioned this issue Dec 22, 2017
Merged
@jasmussen
Copy link
Contributor

Great ticket.

Yes, this seems to have happened because we have no max-width on the main container, and so we have emulated the floats using left margins instead.

We can "hack our way through this" by using .alignleft + .alignleft to remove the left margin, or we can rethink not having a max-width on the main container.

It's been suggested a number of times that iframing the editor is not feasible (#2420). But if we were able to do that, at least it would simplify the math used to calculate the widths of wide and full-wide images. Right now that math is made complex by the plethora of responsive calculations we have to make in order to accommodate multiple widths of the left and right sidebars at responsive breakpoints.

A while back Matías had a version where wide and fullwide images were sized using JavaScript. Not the most ideal thing, but perhaps it's time to revisit that?

CC: @youknowriad @aduth @mtias

@youknowriad
Copy link
Contributor

A while back Matías had a version where wide and fullwide images were sized using JavaScript. Not the most ideal thing, but perhaps it's time to revisit that?

I also think we should revisit using a fixed width for the content container and use JavaScript to compute the wide and full-wide widths.

@karmatosed karmatosed added this to the Merge Proposal milestone Jan 25, 2018
@jasmussen
Copy link
Contributor

This issue seems to have been almost completely fixed by the recent floats refactor, yay!

multiple floats

As is apparent from the GIF above, the remaining issue is with the block toolbar, which there simply isn't room for in a nice way, as that toolbar also respects the floats.

I'm going to look into that, there may be an easy fix.

jasmussen pushed a commit that referenced this issue Feb 23, 2018
Fix issue with floated block toolbar on adjecent floats
8bfcef8 
Fixes #3870

When you have multiple small blocks floated left, each blocks get a more and more indented block toolbar. At some point the block toolbar will be cropped by the viewport itself.

This PR adresses that by making it always leftalign. There appear to be no side-effects, but it would be good to test.
@jasmussen jasmussen mentioned this issue Feb 23, 2018
Merged
@gziolo
Copy link
Member Author

gziolo commented Feb 23, 2018

Awesome work, one fix to close them all 🗡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
Merge: Editor
Development

No branches or pull requests
4 participants
@karmatosed @youknowriad @gziolo @jasmussen