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

Add rem support with pixel fallbacks to vertical-rhythm module #775

Closed
ry5n opened this issue Mar 13, 2012 · 16 comments
Closed

Add rem support with pixel fallbacks to vertical-rhythm module #775

ry5n opened this issue Mar 13, 2012 · 16 comments

Comments

@ry5n
Copy link
Contributor

ry5n commented Mar 13, 2012

Currently the vertical-rhythm module supports setting the unit of font-size to rems, but doesn't provide pixel fallback declarations for older browsers and has some other issues. More complete support for rems, including pixel fallbacks, would be an awesome addition to this module. See #766. #771 is also relevant.

Additional rationale:

  • Would help authors by removing the need to ever pass the $font-size of the current context. This can be a pain not only because the context can't be computed but because it makes the API more complex and harder to learn.
  • Would improve maintainability of projects as it removes hard-coded $font-size contexts.

Considerations:

  • Using rems means doing extra compilation math and also doubles up on style rules for legacy support (not as much bloat as vendor prefixes mind you)
  • Using rems requires setting the base font size on the root element rather than on the body. There may be issues with older browsers in doing so.
@JohnAlbin
Copy link
Contributor

Its definitely telling that we were both working on this at the same time. :-)

@chriseppstein
Copy link
Member

I think rem is a great unit for developer authoring, but I don't see a need to change the output unit to rem. I would support any changes needed to make it simple for authors to pass rem-based values to the compass mixins but when using absolute units -- let's keep the px output. Please let me know if I'm missing something.

Also, seems like this could be done as non-breaking addition to stable. If not, it'll need to go onto master for 0.13.

@ry5n
Copy link
Contributor Author

ry5n commented Mar 14, 2012

@chriseppstein I'm not 100% sure what you're suggesting and I'd like to understand. Are you suggesting we allow rem inputs (with rem outputs) but no pixel fallbacks? I think that that's already supported (except for +establish-baseline operating on the body rather than root).

Maybe I can restate the rationale for this better by laying bare my thought process:

  1. Sizing fonts in px is bad (received wisdom; mea culpa – is the only reason because of IE6's font-resize issues?)
  2. Sizing fonts in ems is tricky (context has to be tracked by the author)
  3. Rems solve everything :)
  4. Except they aren't supported by older browsers :(
  5. But we can use rems today and offer some support to older browsers by writing pixel fallback values (and yes oldIE will still
    have issues): http://blog.typekit.com/2011/11/09/type-study-sizing-the-legible-letter/
  6. If this is at all a sensible approach***, it would be great to have as an option in vertical-rhythm.

***$64,000 question. cc @JohnAlbin

@chriseppstein
Copy link
Member

I'm saying we don't need pixel fallbacks. As long as the author tells us that 1rem = 16px (or whatever), then compass can still output pixels, and the author can pass rem-based numbers to all the compass mixins. As far I can tell, rem is a authoring convenience and has no other semantic differences over using pixels than the mental model.

@JohnAlbin
Copy link
Contributor

As far I can tell, rem is a authoring convenience and has no other semantic differences over using pixels than the mental model.

That is incorrect in the real world. Otherwise ems would just be an “author convenience”. By only using ems or rems or percentages for font-size in our stylesheets, we are allowing the user to change the default font size. The UAs give a default default font size of 16px; they also allow user's to set the default font-size for the UA.

This is what @ericam was pointing out in #766. Currently the vertical-rhythm outputs a px value on the body tag (except for IE6). That overrides any default font size settings the user has specified in the UA. Compass users should have the option to use relative sizes (even in the establish-baseline() output) so that we can respect the UA users' choices.

So if we give the authors the option to output in ems or rems (and we should), then when we output rem we have to have a px backup for browsers that don't support rem.

compass can still output pixels, and the author can pass rem-based numbers

Actually, I think most authors would want the opposite. Input numbers in px and have the Compass mixin calculate the rem values, while still outputing px as a back-up for non-rem-capable browsers. That's what I want.

Our tools (Photoshop, font faces, etc.) all describe font sizes in pts or pxs. If we were forced to enter font sizes in rems then we'd be doing the calculation in our head. (Or using Sass math to calculate it. Isn't that what a mixin is for?)

I don't know of any tools that describe fonts in terms of percentages of a base font.

So, to reiterate…

  1. we need a flag to have font size output in relative or fixed units; since we are currently only outputting fixed units in establish-baseline(), I'd suggest that flag be a new parameter.
  2. And we need a flag to specify whether we want to output px backup when using rem output.

@chriseppstein
Copy link
Member

That is incorrect in the real world.

Let's avoid the loaded language. We're all operating in the real world here :)

they also allow user's to set the default font-size for the UA.

Excellent. This was the point I was missing. Thank you for clarifying it.

Actually, I think most authors would want the opposite.

Maybe so, but I vastly prefer to author in ems because the font size (rhythm) of the page is such an natural unit of measure. If something doesn't look right on a finished design, my designer usually says "can you add a half-lines worth of padding there?" Not "please add 8px there". But I hate how ems are relative to their context, so I end up translating that to px. So I guess both input modes are important to support.

we need a flag to have font size output in relative or fixed units; since we are currently only outputting fixed units in establish-baseline(), I'd suggest that flag be a new parameter.

This is not correct. We have a flag for this ($relative-font-sizing) and we have the $base-line-height which could be a value set in rems. So I don't think we need any new public facing flags... we should be able to discerns the right behavior from that.

And we need a flag to specify whether we want to output px backup when using rem output.

If the author has specified rem as the output unit, I don't see any harm in setting the px fallback. Users expect compass to do the "right thing" with respect to legacy browser support.

@JohnAlbin
Copy link
Contributor

That is incorrect in the real world.
Let's avoid the loaded language. We're all operating in the real world here :)

Sorry! What I meant was to compare what the CSS spec says versus user experience in browser realities. But I definitely did not phrase that correctly. I apologize.

my designer usually says "can you add a half-lines worth of padding there?"

Really? That'd be nice! I used to work with a designer who wanted me to move something over "a pixel to the left". x-p

So I don't think we need any new public facing flags... we should be able to discerns the right behavior from that.

You could be right, but I'm not sure. Let's see what @ry5n and I come up with first. :-) @ry5n, I'm going to be at the Drupalcon Denver conference next week, so it will probably be after that before I get a chance to work with you on this.

If the author has specified rem as the output unit, I don't see any harm in setting the px fallback. Users expect compass to do the "right thing" with respect to legacy browser support.

Good point!

@ry5n
Copy link
Contributor Author

ry5n commented Mar 15, 2012

@JohnAlbin @chriseppstein Sounds like we're agreed on the basics then – improve rem support and allow for outputting pixel fallbacks when using rems as the internal font unit. John, I think that will be fine, I may do a bit of work in the meantime but let's check in when you're back. Have a great time in Denver. :)

@ry5n
Copy link
Contributor Author

ry5n commented Apr 4, 2012

@JohnAlbin I now have a working dev environment for Compass, and just pushed a vastly improved rem mixin that should prove useful. I’m ready to move ahead; how about you?

@mirisuzanne
Copy link
Member

@JohnAlbin, @chriseppstein, and @ry5n: I've addressed one of the issues mentioned above in #844 - using $relative-font-sizing to set the font as % rather than px on the html tag. If there are no objections to that, I'll go ahead and merge it into master.

That's a blocker for the next version of Susy, so I'd like to take care of it asap.

@ry5n
Copy link
Contributor Author

ry5n commented Apr 14, 2012

Thanks Eric. I notice you'd fixe this and skimmed the solution. Looked good as I recall. I'm hoping to update that issue wih my progress this weekend.

Ryan

On 2012-04-13, at 4:34 PM, Eric Meyerreply@reply.github.com wrote:

@JohnAlbin, @chriseppstein, and @ry5n: I've addressed one of the issues mentioned above in #844 - using $relative-font-sizing to set the font as % rather than px on the html tag. If there are no objections to that, I'll go ahead and merge it into master.

That's a blocker for the next version of Susy, so I'd like to take care of it asap.


Reply to this email directly or view it on GitHub:
#775 (comment)

@pdewouters
Copy link

will it be possible to use REM for type unit and ems for leading?

@ry5n
Copy link
Contributor Author

ry5n commented Apr 23, 2012

@pdewouters The way the VR partial works is it will output rhythm values based on a couple of configurable variables, so you can (I believe) set your $base-font-size and $base-line-height in ems (or rem, px, %) and by default the mixins will output ems. But you can also change the $font-unit variable which determines the output unit.

Not sure if that answers your question. Do you have a particular use case in mind?

@ry5n
Copy link
Contributor Author

ry5n commented May 9, 2012

Bumping myself. Been working on this and almost have something for review, but not confident it’s the right approach. @JohnAlbin will you have a bit of time in the next week to review some code?

@drejohnson
Copy link

bump

@mirisuzanne
Copy link
Member

This conversation isn't dead, it just moved. See Pull Request #896 for discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants