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

Give map variables meaningful names. #115

Open
abea opened this issue Sep 14, 2018 · 1 comment
Open

Give map variables meaningful names. #115

abea opened this issue Sep 14, 2018 · 1 comment
Assignees

Comments

@abea
Copy link

abea commented Sep 14, 2018

This comes up specifically with our font() mixin. Looking at this, it's not clear what property the font-size is: @include font(sans-display, title-desktop, jumbo, medium, 0.53, 74px);. It's probably jumbo or medium, but it could even be one of the last two.

Alternatively we could do something like this:
@include font(sans-display, ft-title-desktop, fs-jumbo, fw-medium, 0.53, 74px);

Then your font weight map might look like:

$font-weight: (
  fw-bold: 700,
  fw-semibold: 600,
  fw-medium: 500,
  fw-regular: 400
);

This wouldn't be necessary if used directly with map-get(), but with the mixin we lack clarity.

Related concerns:

  • Honestly it's not clear to me if the font-size type layer is doing much more than adding a layer of obfuscation since most of the sizes seem to be are shared between groups. The benefit is avoiding having to think of 8 reasonable names for sizes (e.g., medium-small), though I'm seeing some of that happening anyway (e.g., small, smallest, micro in Moore. Smallest isn't the smallest).
  • The example above is from a particular project, but it displayed an additional issue: line-height, besides also being unclear which it is, can be passed through as a pixel value, rather than relative value, without stylelint catching it. It's also harder to catch in QA due to the layer of obscurity. Variables, could have the same problem, but it's easier to see that a lh-normal variable is set to 24px and catch that error.
  • What does the font() mixin really help with that the CSS font property doesn't? The mixin adds letter-spacing. Is that worth the layer? The font property seems better constructed to indicate purpose (e.g., font-size and line-height as 24px/1.4) and is a standard.

It's possible that these are problems specific to Moore, but I don't know if we've stated practices around this.

@abea
Copy link
Author

abea commented Sep 14, 2018

To be clear, the specific proposal is to specify that map variables should be semantically named (e.g., fs-small for font-size: $small. The rest are related discussion points that could have bearing.

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

No branches or pull requests

6 participants