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

Set display:block on main wrapper #863

Merged
merged 2 commits into from
Jul 3, 2018
Merged

Set display:block on main wrapper #863

merged 2 commits into from
Jul 3, 2018

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented Jul 2, 2018

Because the main element is not recognised – meaning it's not defined in IE's default style sheet,
so it uses CSS initial value, which is inline.

Fixes: #862

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-863 July 2, 2018 20:00 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-863 July 2, 2018 20:03 Inactive
@kr8n3r kr8n3r added the fix label Jul 2, 2018
@@ -19,6 +19,8 @@
@mixin govuk-main-wrapper {
@include govuk-responsive-padding(6, "top");
@include govuk-responsive-padding(6, "bottom");
// IE11 supports `main` but doesn't set it to block without HTML5shiv
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know html5shiv isn't involved here - IE11 does support the <main> element 'natively', it's just not styled correctly. The shiv is only invoked for Internet Explorer 6-9 according to their readme.

@@ -23,6 +23,11 @@

🔧 Fixes:

- Apply `display:block` to `.govuk-main-wrapper`
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're intending to add a line-break here, you need to leave a blank line. At the minute it wraps onto one line and reads like this:

Apply display:block to .govuk-main-wrapper In IE11 main element is set to display:inline so padding an margins aren't applied.

Also, should be and instead of an.

Copy link
Author

Choose a reason for hiding this comment

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

updated

Because the main element is not recognized  –
meaning it's not defined in IE's default style sheet,
so it uses CSS initial value, which is inline.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-863 July 3, 2018 08:25 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👍

@kr8n3r kr8n3r merged commit 6979369 into master Jul 3, 2018
@kr8n3r kr8n3r deleted the fix-main-wrapper-ie11 branch July 3, 2018 08:54
@NickColley
Copy link
Contributor

NickColley commented Jul 3, 2018

Because the main element is not recognised

This worries me, I would have thought this would have been fixed by the HTML5 shiv?

@kr8n3r kr8n3r added this to the v1.1.0 milestone Jul 4, 2018
@NickColley NickColley mentioned this pull request Jul 13, 2018
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