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

Using both an offset and column size causes unexpected width. #75

Open
kybishop opened this issue Jul 27, 2016 · 5 comments · Fixed by #99
Open

Using both an offset and column size causes unexpected width. #75

kybishop opened this issue Jul 27, 2016 · 5 comments · Fixed by #99
Labels

Comments

@kybishop
Copy link
Collaborator

kybishop commented Jul 27, 2016

Applying an offset class to a box with any given column size causes the box's width to shrink. I can't seem to find any documentation on this CSS behavior, but it appears consistent over both Chrome and Safari.

Switching to width over max-width fixes this issue, though I'm guessing there might be a particular reason max-width is being used?

@runspired
Copy link
Collaborator

@kybishop it depends on the flex-direction and is related to flex-basis bugs which I'm also looking into fixing (I have a solution in mind).

@kybishop kybishop changed the title Using max-width with margins causes sizing errors Using both an offset and column size causes unexpected width. Dec 16, 2016
@kybishop kybishop reopened this Dec 16, 2016
@kybishop
Copy link
Collaborator Author

kybishop commented Dec 16, 2016

From my investigations:

This is a bug in Chrome/Safari related to flex-direction: column.

The bug boils down to inconsistencies between how percentage-based max-width and width properties determine the width of an element when flex-direction is set to column

Width calculations on Chrome and Safari when flex-direction: column:

  • with max-width as a percentage: (parentWidth - childMargins) * max-width.
  • with width as a percentage: parentWidth * width

Firefox [correctly] ignores margins in both cases

Solutions

1. We do the same as Bootstrap and use width to determine column sizes.
- This is a foolproof approach consistent among all browsers. This gets us a fix right away
- One could argue this is more "correct". Should a column ever be expected to shrink? If not, width is more appropriate.
- If the goal is to ease people from Bootstrap to Flexi, it might be advantageous to implement grids in a similar way. Using width provides a more consistent drop-in solution.
- CON: This is a breaking change for people relying on the old column behavior

EDIT: It turns out bootstrap isn't using flexbox, and this fix doesn't actually apply.
2. Hope Chrome/Safari can fix the bug in a timely manner. Bug report here.

@kybishop
Copy link
Collaborator Author

kybishop commented Jan 4, 2017

This bug unfortunately remains. PR #99 breaks wrapping; wrapped flex children take up the entire row. Will be opening a PR to revert #99.

EDIT: reverted in #108

@kybishop kybishop reopened this Jan 4, 2017
@kybishop kybishop added the bug label Jan 4, 2017
@RobbieTheWagner
Copy link
Member

@runspired could you elaborate on the solution you had in mind or should we just wait for browsers to fix themselves to support this?

@kybishop
Copy link
Collaborator Author

kybishop commented Feb 7, 2017

Here is an example of the row-wrapping bug: http://codepen.io/kybishop/pen/NdBMpE

Change max-width to width on the .half class and note how the third box takes up the entire row. This unfortunately means we must use max-width, which makes the offset bug persist thanks to the bug in various browsers.

Thankfully the browser fixes are slowly being rolled out. Chrome v56 contains a bugfix, and has already been in the wild for about a month. Safari, sadly, has yet to respond to their bug report https://bugs.webkit.org/show_bug.cgi?id=166061

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

Successfully merging a pull request may close this issue.

3 participants