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

Render the main element consistently in IE #730

Closed
wants to merge 1 commit into from
Closed

Render the main element consistently in IE #730

wants to merge 1 commit into from

Conversation

thasmo
Copy link

@thasmo thasmo commented Feb 15, 2018

Fixes #727 by rendering the main element as block-element for IE.

@jonathantneal
Copy link
Contributor

@thasmo, nice, but would you specify the target browser(s)?

@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

Sure, where exactly? In the PR title, commit message, comment? :)

@thasmo thasmo changed the title Render the main element consistently Render the main element consistently for IE 9+ Feb 15, 2018
@jonathantneal
Copy link
Contributor

jonathantneal commented Feb 15, 2018

Line 28, where you specified all browsers. 55f0ce1#diff-bb3dde41d97f19be8ab7b4780a915d5eR28

Before it was removed, I had specified Add the correct display in IE here:

https://github.com/necolas/normalize.css/blob/6.0.0/normalize.css#L49

@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

@jonathantneal, done. 👍

@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

Oh, I see. On it.

@jonathantneal
Copy link
Contributor

Just putting IE will do, because that means all versions. I hope other IE11 styles weren’t lost, unless this project no longer supports IE11 either.

@jonathantneal
Copy link
Contributor

jonathantneal commented Feb 15, 2018

I know it sounds really picky, but IE 9-11 would not be accurate, because IE8 did not support main 🤷‍♂️ and this project doesn’t support IE9 or IE10 AFAIK.

CORRECTION: It was updated, this project supports IE10+:

https://github.com/necolas/normalize.css/blob/4ab3de5bdd26b161c3c82a5a2f72df3e57a8e4bf/CHANGELOG.md
https://github.com/necolas/normalize.css/blob/master/CHANGELOG.md

@thasmo thasmo changed the title Render the main element consistently for IE 9+ Render the main element consistently for IE Feb 15, 2018
@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

@jonathantneal, last chance. :P

@thasmo thasmo changed the title Render the main element consistently for IE Render the main element consistently in IE Feb 15, 2018
@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

Actually, last chance now. ;)

@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

Typed enough Is and Es today. 👍

@thasmo
Copy link
Author

thasmo commented Feb 15, 2018

I believe IE 10 is supported, at least that's what's documented but I'd be happy to only have support for IE 11.

@jonathantneal
Copy link
Contributor

jonathantneal commented Feb 15, 2018

Yea, it’s not complicated. The comment just needs to cover all IE versions.

It will also need to use the same voice as the other comments, which would be:

Add the correct display in IE

That’s not the message you’ve written, as you’ve introduced new terms like Render and consistently. As picky as it sounds, you’d likely lose that one line in git blame 😄 but otherwise this looks great and I hope it’s merged.

I really have zero control over this. I’m just looking out for the project as best I can.

nzec
nzec approved these changes Feb 26, 2018
Copy link

@nzec nzec left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

@fritzmg
Copy link

fritzmg commented Apr 5, 2018

👍 this caused some headache for me in a current project where the <main> element was used and the padding for example had no effect in IE. I didn't realize IE 10+ does not treat <main> as a block level element automatically.

@thasmo
Copy link
Author

thasmo commented Apr 13, 2018

🤔

@Tetonne
Copy link

Tetonne commented Apr 14, 2018

get standard scrollbar (safari)

@garrettw
Copy link

@thasmo @Tetonne please do not post meaningless/spam comments.

@fritzmg
Copy link

fritzmg commented Jul 5, 2018

Geez, exactly 3 months later I stumbled over the same issue - at least it only cost me half an hour this time, until I figured it out... again 😁

Why has this not been merged and released in 8.x?

@necolas necolas closed this in df07c00 Nov 5, 2018
@fritzmg
Copy link

fritzmg commented Nov 8, 2018

@necolas the NPM package still only has 8.0.0: https://www.npmjs.com/package/normalize.css?activeTab=versions
Could you may be check why?

@necolas
Copy link
Owner

necolas commented Nov 9, 2018

Forgot to publish. Should be up now. Thanks

@nzec
Copy link

nzec commented Nov 10, 2018

@necolas Thanks! ❤️

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.

None yet

8 participants