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

Fix IE11 layout problem caused by flex-basis: auto #2763

Closed
wants to merge 1 commit into from

Conversation

phacks
Copy link
Contributor

@phacks phacks commented Jan 12, 2019

As mentioned in #2676, IE11 suffers from several layout problems where the user would have to horizontally scroll to see all content and/or images would get absurdly distorted.

philipwalton/flexbugs#170 seems to show that IE11 shows inconsistency when using flex-basis: auto (or the shorthand version: flex: X Y auto).

I tried changing auto by 0px in such statements, and it looks like it fixed many of those layout problems in the demo app:

🙁Before (IE11) 🙂After (IE11)
image image
image image
image image

I also had to remove the display: flex and flex-wrap: wrap on SegmentsField because IE11 has many problems with flex-wrap: wrap. The wrapping still works (Chrome + IE11) though.

Navigating the demo app on Chrome showed me no visual regression, and <Aside /> components (for which the flex-basis: auto were introduced) still work as expected:

image
image

Copy link
Contributor

@Kmaschta Kmaschta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Kmaschta Kmaschta added this to the 2.6.2 milestone Jan 14, 2019
@fzaninotto
Copy link
Member

This needs thorough testing across multiple browsers

@phacks
Copy link
Contributor Author

phacks commented Jan 14, 2019

@fzaninotto of course — how would you like me to proceed?

@fzaninotto
Copy link
Member

We use tools like BrowserStack. But I'll do the tests on this one to make sure it breaks nothing - we've had regressions in the past due to small changes in the layout CSS.

@fzaninotto fzaninotto removed this from the 2.6.2 milestone Jan 21, 2019
@phacks
Copy link
Contributor Author

phacks commented Jan 29, 2019

@fzaninotto Hi! Did you have time to run some browser testing on this issue?

Let me know if I can be of any help 🙂

@phacks
Copy link
Contributor Author

phacks commented Feb 12, 2019

@fzaninotto Hi! Do you think you will have some time to test this PR soon? cc @Kmaschta

Thanks!

@Kmaschta Kmaschta self-assigned this Feb 12, 2019
@Kmaschta
Copy link
Contributor

Kmaschta commented Feb 12, 2019

I booked 15min tomorrow to test that branch locally with Browserstack

@Kmaschta
Copy link
Contributor

I tested these layouts on several browsers (latest version), it is OK for me 👍

Here are the screenshots for each of them:

Google Chrome (Windows)

image

image

image

Internet Explorer 11 (Windows)

image

image

image

Mozilla Firefox (Windows)

image

image

image

Safari 12 (MacOS)

image

image

image

Edge (Windows)

image

image

image

Opera (Windows)

image

image

image

@Kmaschta Kmaschta removed their assignment Feb 13, 2019
@phacks
Copy link
Contributor Author

phacks commented Feb 13, 2019

@Kmaschta thanks for the review! @fzaninotto Do you think this PR can make it for the 2.7.2 / 2.8.0 milestones?

@djhi djhi added this to the 2.7.2 milestone Feb 13, 2019
@fzaninotto
Copy link
Member

Not good: I see an additional scrollbar at the bottom of the Comment List in the simple demo (Chrome, Ubuntu):

image

and at the bottom of the Posters list in the demo (Safari, macOS)

image

and on Windows as well:

image

So I can't merge this PR as is.

@phacks
Copy link
Contributor Author

phacks commented Feb 13, 2019

@fzaninotto Oops, I can work on it next week, will keep you updated!

@phacks
Copy link
Contributor Author

phacks commented Mar 6, 2019

@batbyR found another, cleaner solution that get rid of the superfluous scrollbars at the bottom! You may have a look over at #2969

@fzaninotto fzaninotto closed this Mar 7, 2019
@phacks phacks deleted the fix-ie11-layout-problems branch March 7, 2019 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants