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

CSS-Lock mixin #129

Closed
arvigeus opened this issue Mar 29, 2017 · 34 comments
Closed

CSS-Lock mixin #129

arvigeus opened this issue Mar 29, 2017 · 34 comments

Comments

@arvigeus
Copy link

Based on this article: https://www.smashingmagazine.com/2016/05/fluid-typography/

@bhough
Copy link
Contributor

bhough commented Mar 30, 2017

Interesting @arvigeus. What would an api for a mixin/helper look like for this?

@arvigeus
Copy link
Author

I use something like this:

// minScreen, maxScreen, and units are optional
function responsiveFontSize({ minFont, maxFont, minScreen, maxScreen, units }) {
  if (!minScreen) minScreen = 320;
  if (!maxScreen) maxScreen = 1200;
  if (!units) units = "px";
  return { fontSize: `calc(${minFont}${units}+${maxFont - minFont}*(100vw-${minScreen}${units})/${maxScreen - minScreen})` };
}

Not sure for the other techniques mentioned in the article

@DJTB
Copy link

DJTB commented Mar 31, 2017

I use a very similar function and it's marvelous. Definitely worthy of consideration.

@xzilja
Copy link

xzilja commented Apr 2, 2017

Regarding #594 react-native responsive font support, something that would allow us to base font size on device screen width would be extremely useful i.e. font size that is 16% of screen width number

@bhough
Copy link
Contributor

bhough commented Apr 4, 2017

@iljadaderko does React native use standard media queries for something like that, or is there something specific to that implementation?

@bhough
Copy link
Contributor

bhough commented Apr 4, 2017

So @arvigeus I think we could get it compressed down to this:

function responsiveFontSize({ minFont, maxFont, minScreen = 320, maxScreen = 1200, units = "px" }) {
  return { 
    fontSize: `calc(${minFont}${units}+${maxFont - minFont} * (100vw-${minScreen}${units})/${maxScreen - minScreen})` 
  }
}

@xzilja
Copy link

xzilja commented Apr 4, 2017

@bhough react native exposes device width and height https://facebook.github.io/react-native/docs/dimensions.html I was trying something along the lines of:

const fontSize = (Dimensions.get('window').width * 18)

To simulate 18% of device width for font, but wanted to know if styled-components or polished offer easier solution.

@bhough
Copy link
Contributor

bhough commented Apr 5, 2017

@iljadaderko What would the easier solution in your opinion be here. Seems like opening an issue to add Device.get('window') support to our media query based mixins would be useful, but not sure what else we could do to simplify that? A general mixin that takes a traditional CSS media query and converts it for React Native maybe?

@xzilja
Copy link

xzilja commented Apr 5, 2017

I was thinking more along the lines of font-size: 10%; being converted into appropriate react-native unit if possible? Or via some mixin that takes in percentage value and spits out correct one.

@bhough
Copy link
Contributor

bhough commented Apr 5, 2017

I could definitely see a percentage mixin for React Native where you could do something along the lines of:

{
  'font-size': deviceWidth('10%'),
  'height': deviceHeight('100%'),
}

If that looks like it would be useful, I'll open a separate issue for this to get further feedback.

@souporserious
Copy link
Contributor

souporserious commented Apr 11, 2017

I converted this gist to JS and it seems to be working fairly well so far.

const unit = str => isNaN(str) ? str.substr(str.length - 2) : 'px';

export default function fluidFontSize(
  {
    minFontSize,
    maxFontSize,
    minScreenSize = 320,
    maxScreenSize = 800
  }
) {
  const u1 = unit(minFontSize);
  const u2 = unit(maxFontSize);
  const u3 = unit(minScreenSize);
  const u4 = unit(maxScreenSize);

  if (u1 === u2 && u1 === u3 && u1 === u4) {
    const units = u1;
    const styles = {
      fontSize: minFontSize
    };

    if (minScreenSize !== maxScreenSize) {
      styles[`@media only screen and (min-width: ${minScreenSize}${units})`] = {
        fontSize: `calc(${parseInt(minFontSize)}${units} + ${parseInt(maxFontSize) - parseInt(minFontSize)} * ((100vw - ${minScreenSize}${units}) / ${parseInt(maxScreenSize) - parseInt(minScreenSize)}))`
      };
    }

    styles[`@media only screen and (min-width: ${maxScreenSize}${units})`] = {
      fontSize: maxFontSize
    };

    return styles;
  }

  throw new Error(
    'Detected mixed units. Please use the same units for all parameters.'
  );
}

@bhough
Copy link
Contributor

bhough commented Apr 11, 2017

@souporserious Would you like to put this into a PR so we can properly consider it?

@souporserious
Copy link
Contributor

Yeah for sure :) I should be able to do it this week.

@souporserious
Copy link
Contributor

@bhough thoughts on making the function a little more abstract? Just read this article about CSS locks and it seems we could benefit from this technique in multiple areas.

@bhough
Copy link
Contributor

bhough commented Apr 19, 2017

@souporserious I'm not opposed to it, but the one thing I'll say is: Can we express what the helper is doing and what it's api is concisely? With the fluid typography version I can understand what is going on and the API is clear. With CSS Locks, that may be difficult. The concept hasn't really caught on much beyond a few think pieces last year (all written by Tim Brown), and depending on what the final API looks like, we wouldn't want users to have to read a 10k word blog post to understand how to use it.

@DJTB
Copy link

DJTB commented Apr 19, 2017

An abstract function does have more appeal I think.
A much shorter article, that's simpler to grok than the longform CSS Locks just came out as well:
https://css-tricks.com/between-the-lines/

perhaps

function responsiveFontSize({ minFont, maxFont, minScreen = 320, maxScreen = 1200, units = "px" })

becomes between or interpolate?

function between({ cssProp, minSize, maxSize, minScreen = 320, maxScreen = 1200, units = "px" })

@bhough
Copy link
Contributor

bhough commented Apr 19, 2017

Thanks @DJTB that is definitely much easier to grok. @souporserious something like the example @DJTB shared would definitely be a great starting point. Definitely want to discuss the method name a bit more, I'm not sure either between or interpolate are clear/describptive enough.

@DJTB
Copy link

DJTB commented Apr 19, 2017

Well, without passing in the optional screen ranges, the proposed function works similarly to the new CSS Grid function minmax(). If that's a reasonable starting point for naming.

@bhough
Copy link
Contributor

bhough commented Apr 24, 2017

@DJTB and @souporserious maybe something like lockRange or fluidBreakpoint, but we can continue to discuss the name. @DJTB's proposed API looks like a great starting point @souporserious.

bhough added a commit that referenced this issue May 11, 2017
Adds fluidRange mixin that allows the user to leverage css-locks.

#129
bhough added a commit that referenced this issue May 11, 2017
Adds fluidRange mixin that allows the user to leverage css-locks.

#129
@bhough
Copy link
Contributor

bhough commented May 11, 2017

@DJTB @souporserious Check out #169 and tell me what you think of that API/Functionality

@DJTB
Copy link

DJTB commented May 11, 2017

The output calc() will fail without a unit value:

"@media (min-width: 400px)": Object {
    "margin": "calc(-8.333333333333336 + 3.3333333333333335vw)",
    "padding": "calc(-33.33333333333334 + 13.333333333333334vw)",
 },

should be:
calc(-8.333333333333336px ...)
calc(-33.33333333333334px ...)

@DJTB
Copy link

DJTB commented May 11, 2017

I pumped in the units in devtools, and the scaling works great.

API wise, I'd probably prefer single params if only one property, similar to helpers like:
${directionalProperty('padding', '12px', '24px', '36px', '48px')}
and array for multiple props.

That said, the named object is great for clarity, and people (myself!) can compose fluidRange as they wish anyway. So despite my preference for shorter calling syntax - I'd say the API looks good and flexible! 👍

@bhough
Copy link
Contributor

bhough commented May 11, 2017

Good catch @DJTB.

Yeah I'm still torn on that. See #126 for some discussion. Technically, we can support all three, I'm just concerned about code bloat supporting so many different APIs.

What do you think about exporting between separately as a helper as well for those that just want the calculation?

@DJTB
Copy link

DJTB commented May 11, 2017

That sounds pretty reasonable. I'd probably want it to have default min/max, same as fluidRange.

function between(fromSize: string, toSize: string, unitlessMinScreen: number = 320, unitlessMaxScreen: number = 1200)

I understand the API woes. I like both the simple and the verbose versions depending on the familiarity with the function in question. I won't be much help in that discussion.

@bhough bhough added this to the 2.0 milestone May 18, 2017
@bhough bhough changed the title Fluid typography [V2] CSS-Lock mixin May 18, 2017
@leggomuhgreggo
Copy link

leggomuhgreggo commented May 27, 2017

Was wondering if this was on the radar -- awesome!

I've been messing with CSS Locks a lot lately, and judging from the last few comments looks like y'all are reckoning with the same csslock API scope creep that I've been.

Some helpful things to consider:

Root/HTML font-size:
For a11y reasons, it's recommended that the font not be defined in pixels, because it steps on font-size adjustment utilities, and there are some compelling accessibility/responsivity arguments in favor of making all sizing in terms of rem.

Non-font-size properties:
Csslocks are awesome for making the size values of other properties, like padding and margin, fluid. If you set the root font-size with csslocks, you can get pretty far setting padding and the like with rem, but that's limited by the parameters of the linear function used to set root font size.

So, in cases where rem doesn't cut it, you can use another csslock function which indicates:

  1. needing to factor for property value shorthand eg padding: 10px 20px
  2. Tracking which properties have calc support (border width doesn't for example)
  3. Ideally putting the calc in terms of rem

More than two breakpoints
Using more than two breakpoints or breakpoints that are different than the root breakpoints requires a certain level of interpolation, particularly if you're going to put lock calcs in terms of rem.


I wrote a scss mixin that accommodates the above concerns pretty well, but I wanted to use styled components, and so in preparing to do adapt my mixin, I wanted to see about the existing landscape -- that's what brought me here.

I think, perfect being the enemy of good, I can see just going with a simple fluidFontSize function, perhaps with some cautionary notes.

That said, I'm definitely angling to write a styled-components compatible function to address the more robust CSS Locks API. If y'all over here are interested I'd be down to collaborate.

Thanks!

@souporserious
Copy link
Contributor

@leggomuhgreggo that all sounds great!!! I know there is a PR currently open here, maybe @bhough could help out with that. I'm not sure where he's at with it though.

@DJTB
Copy link

DJTB commented May 27, 2017

Not that we need more information to complicate further... but @MadeByMike who kinda kicked a lot of this off, recently wrote another article regarding these techniques.

https://madebymike.com.au/writing/non-linear-interpolation-in-css/

Personally I've been setting a fluid fontSize on the body in rems. Elsewhere for padding/margins/borders etc I just use ems and rems to feed off of that without having to use more mixins.

@leggomuhgreggo
Copy link

Thanks @souporserious! Just reviewed the PR -- looks like some good work.

One cumbersome aspect required for putting sizing in terms of rem is it relies on knowing the pixel equivalent of rem, and if a root lock is set, knowing the width/size parameters. Those values would need to be a) configurable and b) referenceable in subsequent lock functions.

@DJTB nice link -- that interpolate mixin the article reference is really well written. Its also nice to hear from someone using your approach. It's something I've considered, but lamented the rem value being static -- it's nice to just set something to some rem value and have it be fluid, but maybe it's not worth the added complexity of factoring for root breakpoints.

@bhough
Copy link
Contributor

bhough commented May 28, 2017

Clearly this is a potentially very popular feature 👍🏻

Thanks for sharing that latest article @DJTB. The fun with this feature is balancing features with not creating a completely inaccessible API. Likely we will need to settle on a "core" use case that has a simple API. Then we can expand it via a more complicated API (things like changing the units, adding custom interpolation types as outlined in @MadeByMike's article). I definitely like that approach vs a complicated api that accepts different ranges and mix/maxes for the same value.

One possible approach to using rems is to keep the current px based implementation in #169 but leverage our rem helper on the output to convert it after calculation. Theoretically, we can offer the output in rem or em without any additional calculation.

@leggomuhgreggo definitely down to collaborate. I'm working on some under the hood changes to the library that will be needed for any new modules. But once that is done, this mixin is near the top of the list. I think the right approach is:

  1. Finish the current implementation (finalize api, separate out between linear method, test coverage).
  2. If theory/approach holds up on rem/em, add option to convert output to rem or em.
  3. Start introducing custom interpolations as stand alone methods.

@bhough bhough added the WIP label May 29, 2017
@leggomuhgreggo
Copy link

leggomuhgreggo commented Jun 2, 2017

@bhough Sweet, sounds like a solid roadmap.

Question: Is there a prefered methodology for referencing global variables defined by a mixin?

Lock calculations that reference rem will need to be able to know it's pixel equivalent. Default is of course 16px, but it'd be nice to support both the easier to reason about 1rem = 10px methodology, as well as a root lock where rem would be a range.

Options I was considering for referencing custom root font size definitions:

  1. Config file with defaults that could be overwritten by a user defined config file
  2. Some sort of integration with <ThemeProvider />
  3. Environmental variable

I was also thinking that root font size could be set using the injectGlobal helper, but wasn't sure if there was a way to ensure root font size vars could be referenced by lock functions in other files that might be called earlier.

Thanks!

@leggomuhgreggo
Copy link

leggomuhgreggo commented Jun 3, 2017

Update: I've been looking at a11y articles for the past several hours on the subject adaptive technology and font-size, and so far most of the evidence is pointing towards not needing to calculate non-root font sizes (or other sizing) in terms of rem, since browser zoom works so much better. Makes sense since so few sites adopt a rem/em-only sizing methodology, as opposed of pixels.

I've contacted a few authorities on the subject to check, but it's looking like browser zoom covers the a11y needs. This would alleviate a huge amount of complexity, including the need to reference root font-size vars, as discussed in my previous comment.

Would welcome any feedback. Thanks!

@souporserious
Copy link
Contributor

This sounds amazing! Even for other reasons besides the CSS locks. I never knew, but had always hoped, browser zoom could work without using rem/em. Interested to learn more. Thank you for looking into that! I appreciate it on my end :D

@bhough bhough changed the title [V2] CSS-Lock mixin CSS-Lock mixin Jul 24, 2017
@bhough bhough added the V3 label Feb 4, 2018
@bhough
Copy link
Contributor

bhough commented Jun 24, 2018

Addressed by #169 and merged into version-2

@bhough bhough closed this as completed Jun 24, 2018
@souporserious
Copy link
Contributor

I know this is an old issue, but just wanted to post this here in case anyone comes across this. I was under the impression that REM was no longer needed because browser zoom scales pixel values accordingly. While this is true, it does not account for system changes to the default font size so REMs are still preferred for user-controlled font sizes. More information in this article if you're interested :)

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

6 participants