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 modal overflow on IE11 #8176

Conversation

carlosparreno
Copy link

@carlosparreno carlosparreno commented Mar 24, 2021

Closes #6919

  • Modal content scrolls is not displayed. Instead, content overflows the header and footer containers.
  • display: grid, grid-template-columns and grid-template-rows are not supported in IE11. Instead we need to use their correspondent IE11 values: -ms-grid, -ms-grid-row and -ms-grid-column
  • Solution based on this article https://elad.medium.com/supporting-css-grid-in-internet-explorer-b38669e75d66

Changelog

Changed

  • _modal.scss file

Testing / Reviewing

{{ Add descriptions, steps or a checklist for how reviewers can verify this PR works or not }}

@carlosparreno carlosparreno requested a review from a team as a code owner March 24, 2021 15:27
@carlosparreno carlosparreno changed the title fix(modal): fix overflow on IE11 Fix modal overflow on IE11 - NOT READY FOR REVIEW YET Mar 24, 2021
@netlify
Copy link

netlify bot commented Mar 24, 2021

Deploy preview for carbon-elements ready!

Built with commit 75acfa4

https://deploy-preview-8176--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Mar 24, 2021

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 75acfa4

https://deploy-preview-8176--carbon-components-react.netlify.app

@carlosparreno carlosparreno changed the title Fix modal overflow on IE11 - NOT READY FOR REVIEW YET Fix modal overflow on IE11 Mar 24, 2021
@carlosparreno
Copy link
Author

Hi @tw15egan,

I'm trying to use the Netlify deployed version https://deploy-preview-8176--carbon-components-react.netlify.app/ on IE11 unsuccessfully. Is there any known issue with this? It seems to happen in all recent Carbon versions (not in the old ones, e.g. I tried one in July)

Is this a known issue? If so, can you please advise on how to test this?

Thanks in advance

@tw15egan
Copy link
Member

tw15egan commented Mar 24, 2021

@carlosparreno I will pull it down locally and verify. I believe some of the theme storybook additions (which are only in the prod deploy) may not be IE11 compatible

@carlosparreno
Copy link
Author

Great!! Thanks, @tw15egan !

@tw15egan
Copy link
Member

@carlosparreno upon taking a look, I'm seeing this is adding IE-specific media queries. In your application, when you compile the styles, these IE media queries should be added automatically when using something like autoprefixer.

Can you confirm that you are using an autoprefixer as part of your build process, and still seeing this issue?

@tw15egan
Copy link
Member

What is weird, though, is that Modal does not seem to be picking up grid-specific media queries for IE...

modal

You can see on the left that it is getting them for flex though. @tay1orjones any ideas?

@carlosparreno
Copy link
Author

Hi @tw15egan, thanks for your reply:

We are not using autoprefixer. As per my understanding, all the css in carbon is ready to consume as you import carbon-components. Is there any pre-req I'm missing?

As you say, the Modal does not seem to be picking up grid-specific media queries for IE. This is reproducible here if you extend the content text until it overflows the footer section: https://the-carbon-components.netlify.app/?nav=modal

Could you please advise on how to compile this in my application or otherwise where to look at it in Carbon and fix it myself?

Thanks TJ!

@carlosparreno
Copy link
Author

Hi @tw15egan,

Just wondering if you had a chance to look at my comment above ⬆️ .

Thanks a lot in advance 🤗

@tw15egan
Copy link
Member

@carlosparreno I just added #8215 that updates our browser matrix to include the ie11 grid options for autoprefixer to our compiled css.

In terms of enabling autoprefixer as part of your build process, you can take a look at their docs here: https://github.com/postcss/autoprefixer

The PR I mentioned updates our gulpfile use of autoprefixer, but you can also use it in a webpack setup.

@tay1orjones
Copy link
Member

@carlosparreno Could you confirm #8215 @tw15egan created fixes the issue for you? It has been released in v10.32.0.

I think we can close this one in favor of that fix.

@carlosparreno
Copy link
Author

Hi @tay1orjones ,

I did a quick test in the vanilla version https://the-carbon-components.netlify.app/?nav=modal and I still see the issue. See image:

image

The modal buttons are hidden when the content is too large and the CSS IE11 specific is not present.

Maybe the link above isn't still updated with the latest Carbon version ?

Also, @tw15egan mentioned to me that I needed to use the autoprefixer in my app. I'm a little bit confused by that. As per my understanding, the components are working in IE11 out of the box. Also, I didn't see any step in the install documentation regarding autoprefixer either https://www.carbondesignsystem.com/developing/react-tutorial/step-1/. Therefore, my guess is that autoprefixer adds the CSS browser-specific in Carbon as part of the build process and the output is shipped supporting IE11. Am I understanding this right?

Thanks a lot in advance.

@tay1orjones
Copy link
Member

As per my understanding, the components are working in IE11 out of the box.

@carlosparreno Yes, if you import and use the compiled css. Search/find there for .bx--modal-container and you'll see the display: -ms-grid;. If you compile/build from .scss files, you'll need to include autoprefixer in your build pipeline.

This codesandbox uses the css directly and so should show it with the prefixes in IE11.

@carlosparreno
Copy link
Author

carlosparreno commented Apr 8, 2021

Cool, I get it. And it's working now as you could also show in your codesandbox working and I also get what you guys meant with the autoprefixer.

All good now, I'm closing this.

Thank you all!

@carlosparreno carlosparreno deleted the modal-overflow-ie11 branch April 8, 2021 13:38
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2021

DCO Assistant Lite bot All contributors have signed the DCO.

@carlosparreno
Copy link
Author

I have read the DCO document and I hereby sign the DCO.

@tw15egan
Copy link
Member

tw15egan commented Apr 9, 2021

@carlosparreno Awesome! Glad we could help you get that sorted out, and thanks for pointing out that issue 👍🏻

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.

Modals with lots of content don't size correctly on IE
3 participants