-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 ?large to also control size of dialog #156
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Converting every dimension/length in all dialog styling to be in terms of a 10px font size (chosen to be easy: 250px => 25em, 4px => .4em) makes it easy to adjust the overall size of things by changing just that one base font size unit that every dimension and length is in terms of. For example, a one-line change of adjusting the base font size unit to 15px would scale everything up by a factor of 150%. For the div, we can simply set the fontSize and convert all lengths from px to em. Since the div is mostly just a container for the iframe, it has few descendants and none override the fontSize (which is important because em wouldn't be 10px on such a descendant). For the splash page in the iframe, several elements do override the font size, so instead we set the font-size of the root element ('html') and convert all lengths from px to rem, aka "root em". Browser support for rem is pretty good, no IE8 but IE9+ and all modern browsers: http://caniuse.com/rem The one browser issue to deal with is that in IE9-10, rem doesn't work in the 'font' CSS shorthand property (documented on "Can I use" and also http://stackoverflow.com/q/16157342/362030 and http://codersblock.com/blog/font-shorthand-bug-in-ie10/ ) but the fix is pretty trivial.
Passed along from ?large to control the badge size, they now also control the iframe and dialog size because after all, if you're on a page with larger font-size that demands a larger badge, the small dialog with small text is gonna look out of place: https://git.io/v2Xy4 The large badge is about 1.5x the size of the small badge (30px to 20px; the other ratios aren't as exact), but I found that making the large dialog 1.4x the size of the small dialog looked better than 1.5x (that is, bumping the em/rem unit font size from 10px to 14px rather than 15px): font-size: 14px https://git.io/v2XDh vs font-size: 15px https://git.io/v2XyJ
Please rebase. I like this. I don't like the empty space at the bottom as much |
Conflicts with rauchg#153 feature.sign-in-link-in-iframe: lib/index.js Added `large` param to splash() call, to which rauchg#153 had added `org` lib/splash.js Converted px to rem where rauchg#153 had dedented
@rauchg Done! |
@laughinghan is the empty space still there? |
Amazing. Thank you so much. |
rauchg
added a commit
that referenced
this pull request
Mar 8, 2016
Fix ?large to also control size of dialog
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Passed along from
?large
to control the badge size, they now alsocontrol the iframe and dialog size because after all, if you're on a
page with larger
font-size
that demands a larger badge, the small dialogwith small text is gonna look out of place:
The large badge is about 1.5x the size of the small badge (30px to 20px;
the other ratios aren't as exact), but I found that making the large
dialog 1.4x the size of the small dialog looked better than 1.5x
(that is, bumping the
em
/rem
unit font size from 10px to 14px ratherthan 15px), I think because at 1.5x the text in the dialog becomes bigger
than the text in the badge:
font-size: 14px
vs
font-size: 15px
See also 6108430 for discussion of the
em
/rem
technique