Skip to content

Various fixes to Chromeframe and a fix to bolded elements #86

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

Merged
merged 4 commits into from
Oct 21, 2013

Conversation

noremac13
Copy link
Contributor

Fixed the click event not hiding the frame and did some minor styling changes. I commented out 2 lines that I thought were unnecessary but left them in if you want to keep them. The box shadow isn't going to show up in <IE9 which is when the frame pops up. Also the white text shadow when the background was white didn't really make sense to me.

Also the 'strong' and 'b' tags weren't properly bolding elements because they were inheriting their font weight from header_weight.

Added a <span> inside the Chromeframe to notify users that they can click the frame to hide it.
The click function was still targeting the old class of .chromeframe.  Updated to .browsehappy.
-Moved padding off of the paragraph element and on to the actual div.

-Added span tag in with paragraph to center the new notification message.

-Commented out 2 lines but left them in for you to decide what to do with them.  The Chromeframe is targeting IE8 which doesn't support box shadow so I didn't see a reason for it to be there.  Also the white text shadow when the background color is also white.
-Made font size of the <span> in .browsehappy slightly smaller than the paragraph.
-Removed 'strong' and 'b' from line 74 so they would bold properly instead of inheriting the font weight from header_weight.
background #fff
border-bottom 1px solid #ddd
box-shadow inset 0 -1px 0 #fff
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't compatible with <IE9 which is everything the Chromeframe is targeting.

corysimmons pushed a commit that referenced this pull request Oct 21, 2013
Various fixes to Chromeframe and a fix to bolded elements
@corysimmons corysimmons merged commit 3a93d58 into mojotech:master Oct 21, 2013
@corysimmons
Copy link
Contributor

<3 my little bug finder. :)

@corysimmons
Copy link
Contributor

Cleaned it up a bit more 886cd32

@noremac13
Copy link
Contributor Author

Looks good except now there is a bigger space at the top because of the padding and the paragraph margin top. I guess you would also want something like &:first-of-type margin-top 0

@corysimmons
Copy link
Contributor

Why is there margin-top at all? Do p's get margin-top now? >=[

@noremac13
Copy link
Contributor Author

When I took out the conditional comment and looked at the frame in Chrome it wasn't there.

The paragraph metrics in Chrome look like this: http://puu.sh/4VSJb.png

Also the metrics in IE9 and IE10 look like this: http://puu.sh/4VSPo.png also no margin top.

But in IE8 it is getting a margin-top of about the same as the margin-bottom: http://puu.sh/4VSTK.png

It's funny that you only see this frame in IE8 and below and coincidentally that is when it is screwed up lol.

@noremac13
Copy link
Contributor Author

Also do you happen to know why the document standards aren't changing along with the browser mode in the IE dev console? Usually when I set the browser mode to IE8 for example the standards automatically switch to IE8 standards and that gets tagged as (page default). Now when I switch it always stays on IE10 Standards as page default.

Last time I encountered this it was because the page didn't have a valid <!doctype> but since there is a doctype I don't know what else would cause that. I don't think it has any effect on the rendering of the page... just more of a curiosity if anything.

@corysimmons
Copy link
Contributor

Wtfh IE8.. Y U APPLY MARGIN-TOP?!

Also, it's probably trying to use most recent IE because of https://github.com/CorySimmons/jeet/blob/master/index.html#L8

@corysimmons
Copy link
Contributor

I'm assuming it's borked in IE7 too. Can you add:

p
    margin-top 0

..to typography.styl at the bottom and see if IE7/8 act right?

Might just have to add this kinda crap for these edge cases. Seriously, so weird. Makes no sense at all to me.. :(

@noremac13
Copy link
Contributor Author

Yeah I added that in right before the mobile breakpoint stuff and that fixed it.

@noremac13
Copy link
Contributor Author

I tried it in the reset.styl and it works there as well.

@corysimmons
Copy link
Contributor

So weird because the star selector stuff should be getting it.

Maybe this fixes a bunch of that crap? Let me know if you stumble upon any other margin-top stupidness 9ad64a0#diff-72e135160bf03faca24bcb3251fd6307R72

@noremac13
Copy link
Contributor Author

The funny thing is I checked it out in IE7 and the margin-top isn't there. It was just there in IE8.

@corysimmons
Copy link
Contributor

So the new update doesn't work either? What the actual fuck? Bah screw it. I might look into this later if I ever get 5 seconds.

@noremac13
Copy link
Contributor Author

Oh wait no I was still looking at the old version. But yeah even in the old version the only problem was in IE8.

@corysimmons
Copy link
Contributor

So everything works now with the new version of Jeet?

@noremac13
Copy link
Contributor Author

I wouldn't say Everything™ but the stuff we were looking at works now. At least in IE 7,8, and 9. I haven't looked at it in like Safari or Opera and mobile browsers.

@corysimmons
Copy link
Contributor

Should be alright. :)

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.

2 participants