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

Enforce theme font sizes #202

Closed
fbarl opened this issue May 7, 2018 · 16 comments
Closed

Enforce theme font sizes #202

fbarl opened this issue May 7, 2018 · 16 comments
Assignees
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality style-guide Part of the effort towards a unified design system

Comments

@fbarl
Copy link
Contributor

fbarl commented May 7, 2018

Although our weave.js theme includes some font sizes, they are not used that much across our code - I assume the two main reasons are:

a. There are only 3 sizes to choose from which still doesn't cover a lot of cases
b. Their usage is not enforced (or at least hinted by a warning)

fontSizes: {
  normal: '0.875em',
  large: '2em',
  xl: '2.827em',
}

I suggest the following:

  1. Aggregate all the font sizes used across our app into theme.fontSizes
  2. Force style linters to accept only variables passed to font-size property (like it's done for z-index)
  3. Use only theme font sizes everywhere
  4. (not part of this issue) Start dropping some obsolete/ugly font sizes gradually while slowly converging to our unified design system, like we are currently doing with colors

cc @bia @miklosp @jpellizzari

@fbarl fbarl added chore Related to fix/refinement/improvement of end user or new/existing developer functionality style-guide Part of the effort towards a unified design system labels May 7, 2018
@fbarl fbarl self-assigned this May 7, 2018
@fbarl
Copy link
Contributor Author

fbarl commented May 7, 2018

Some usage distribution across our UI code (not taking into account relative % units and sizes implicitly used by Text component; also assuming em === rem which seems to be the case with most of our code):

 3 - 10px
 3 - 11px
11 - 0.7em
 1 - 11.7px
12 - 12px
15 - 0.8em
17 - 13px
 3 - 0.85em
53 - 14px/0.875em
12 - 0.9em
 3 - 15px
35 - 16px/1em
11 - 18px
 2 - 1.2em
 9 - 20px/1.25em
 1 - 1.3em
 2 - 21px
 3 - 22px
20 - 24px/1.5em
 1 - 28px/1.75em
 1 - 30px
 6 - 32px/2em 
 1 - 34px
 3 - 36px/2.25em
 1 - 3em
 1 - 4em
 1 - 200px 
 1 - 320px

I will try to merge the units that are really close to one another and try to get rid of big single-use-case ones context permitting. Ideally that should put it down to not more than 10-15 different sizes and the design team can pick it up after that (step 4 above) :)

@fbarl
Copy link
Contributor Author

fbarl commented May 7, 2018

I think grouping them like this could work:

tiny - 11px

 3 - 10px
 3 - 11px
11 - 0.7em

small - 12.5px

 1 - 11.7px
12 - 12px
15 - 0.8em
17 - 13px

normal : 14px

 3 - 0.85em
53 - 14px/0.875em
12 - 0.9em
 3 - 15px

large : 16px

35 - 16px/1em

huge : 19px

11 - 18px
 2 - 1.2em
 9 - 20px/1.25em
 1 - 1.3em
 2 - 21px

enormous : 24px

 3 - 22px
20 - 24px/1.5em
 1 - 28px/1.75em
 1 - 30px

gigantic : 34px

 6 - 32px/2em 
 1 - 34px
 3 - 36px/2.25em

???

 1 - 3em
 1 - 4em
 1 - 200px 
 1 - 320px

I went a bit crazy with the naming there - I think I could use a native speaker's help (@jpellizzari, @bia) for fine-grading between different words for big. Could always go with l, xl, xxl, etc... but I thought more colorful/memorable variable names are always a good idea :)

@jpellizzari
Copy link
Contributor

I think we should go with a scale like this to cut down on the available options: 10, 12, 14, 18, 24, 36. I think that gives us enough variation to get a nice hierarchy if we need it, without allowing for too much customization. The actual values aren't as important as limiting it to 6 or less.

Names (respectively): tiny, small, normal, large, extra-large, heading. I was thinking this would communicate some intent.

Ii also propose we build this into the <Text /> component to make it very simple to follow the font-sizes.

@bia
Copy link
Contributor

bia commented May 8, 2018

The type scale that we discussed on the Design Trello might still be a useful reference.

@rndstr
Copy link
Contributor

rndstr commented May 8, 2018

We should also use either rem or px IMHO. em is too much of a headache if people inadvertently nest font-sizes.

@miklosp
Copy link

miklosp commented May 10, 2018

That's what I would like to work with. 16px is our base size.
image

@miklosp
Copy link

miklosp commented May 10, 2018

@fbarl the grouping I would recommend is:
tiny, small > tiny
normal > small
large > normal
huge, enormous > large
gigantic > extra large
huge is used very sparingly (big numbers on instance homepage and dashboards)

@fbarl
Copy link
Contributor Author

fbarl commented May 11, 2018

@jpellizzari @bia @miklosp @miklosp Thanks for all the feedback! I think I managed to consolidate most of your comments:

  • Taking the unit to be px
  • Taking the normal font size to be 16px
  • After playing with the type scale @bia linked, we decided to drop it for now
  • I took @jpellizzari's naming suggestions at first but then I felt heading is a semantic name, which doesn't really apply to our use cases that well
  • I stayed a bit on the conservative side and didn't wanna merge 20px and 24px so I kept 7 font sizes in the end:

image

@miklosp
Copy link

miklosp commented May 11, 2018

I say 20 and 24 are too close together to be meaningful. Be brave and merge them. 😉

@fbarl
Copy link
Contributor Author

fbarl commented May 11, 2018

I say 20 and 24 are too close together to be meaningful. Be brave and merge them.

Alright, I'll be brave.

image

@bowenli
Copy link
Contributor

bowenli commented May 15, 2018

@miklosp @bia We should be using a progression instead of picking and choosing. The 16->22 jump is too large. Sometimes you need a localized header. The Elements of Typographic Style is the de facto standard. 6 7 8 9 10 11 12 14 16 18 21 24 36 48 60 72

@jpellizzari
Copy link
Contributor

6 7 8 9 10 11 12 14 16 18 21 24 36 48 60 72

Too many choices. That is basically a continuous list of integers. I am cool with adding one more level between 16 and 22, but we need to constrain the choices for consistency.

@miklosp
Copy link

miklosp commented May 16, 2018

Let's keep the current 6 variants. @bowenli can you please supply an example where you need something between 16 and 22?

@bowenli
Copy link
Contributor

bowenli commented May 16, 2018

screen_shot_2018-05-16_at_07_27_27
screen_shot_2018-05-16_at_07_27_19
screen_shot_2018-05-16_at_07_26_57
screen_shot_2018-05-16_at_07_26_02
screen_shot_2018-05-16_at_07_25_49

@bowenli
Copy link
Contributor

bowenli commented May 16, 2018

Regardless, we should still be using a ratio scaling factor. http://spencermortensen.com/articles/typographic-scale/

@bia
Copy link
Contributor

bia commented May 16, 2018

@bowenli I'm curious to know if some of the sizing examples that you pointed out could be resolved by using the same font size as the subtext, but semi-bold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to fix/refinement/improvement of end user or new/existing developer functionality style-guide Part of the effort towards a unified design system
Projects
None yet
Development

No branches or pull requests

6 participants