More & better titling #627

cchaos · 2018-04-03T17:50:56Z

Main takeaway

The euiTitle mixin now allows a font-size parameter to be passed which will also set the correct line-height and font-weight for that title level. This means that all titles at that particular font-size will look the same no matter the component. (Consistency)

Big differences

Only the top 2 levels of titling are in the light font weight. The titles get bolder as you decrease font size. This helps to separate heading from content especially when they are similar sizes – increases scan-ability.

More title levels

Since the page title should always be the largest, and highest level, title and we are currently using <EuiTitle size="l" /> for page titles, this needs to remain the top most level. Therefore, I could only add more titling levels on the small size, hence the new xs, xxs, and xxxs values allowed for the size prop.

This also meant that the large size should utilize the $euiFontSizeXXL (largest font size) variable since no other font size should be larger than it. So I manipulated our font size scale a bit

from:

$euiFontSizeXS:     $euiFontSize * .75 !default;
$euiFontSizeS:      $euiFontSize * .875 !default;
$euiFontSizeM:      $euiFontSize * 1.125 !default;
$euiFontSizeL:      $euiFontSize * 1.5 !default;
$euiFontSizeXL:     $euiFontSize * 2 !default;
$euiFontSizeXXL:    $euiFontSize * 3 !default;

to

$euiFontSizeXS:     $euiFontSize * .75 !default;
$euiFontSizeS:      $euiFontSize * .875 !default;
$euiFontSizeM:      $euiFontSize * 1.125 !default;
$euiFontSizeL:      $euiFontSize * 1.25 !default;
$euiFontSizeXL:     $euiFontSize * 1.75 !default;
$euiFontSizeXXL:    $euiFontSize * 2.25 !default;

I'm also allowing a uppercase prop to be passed to convert it to all caps

EuiText

The EuiText component also utilizes the same euiTitle mixin though it then converts the font-size to a percentage to be sure it changes with the text's base font size. I also tried to utilize em units wherever possible (some margins needed to remain in px). I also added max-width: 36rem.

The only difference between our default titling and the ones in EuiText is that the 3rd level is bolder. I found that in the case of large text areas, the bold helped to separate the content visually and offer better scan-ability. However, it seemed way too bold if it stood on it's own as is the case with EuiTitle.

Other changes

Removed the doc specific text styling
Increased font-size of fig captions in docs
Decreased title font color slightly
Modified most components who have some sort of title in them to utilize the new mixin - this means that in some cases, the component's title weight has changed ex: EuiSidebar. This is an intentional change.

snide

This works great. They pop now.

Looks like the props tab on EuiTitle doesn't work. Was broken on master, but you may as well fix it while you're in there. Likely just not being passed correctly in the example docs. Sometimes I've seen issues with it around null returns #384

Think you'll want to add the new titles to the actual EuiText docs rather than just to the text scaling stuff.

Once my eyes got used to it, the bolding looks good. Only place that it bugs me is when we use underlines. You might want to adjust the EuiCard hover states to use color, rather than an underline now. It can look at a little weird with the thick underlines. Pretty minor.

snide · 2018-04-03T23:12:32Z

src/components/title/title.js


  const classes = classNames(
    'euiTitle',
    titleSizeToClassNameMap[size],
+    {
+      'euiTitle--uppercase': uppercase,


I'd make this a map rather than a boolean. Maybe textTransform="uppercase"

snide · 2018-04-03T23:24:32Z

src/components/text/_text.scss

@@ -21,25 +22,25 @@
  ol,
  blockquote,
  img {
-    margin-bottom: $euiSizeL;
+    margin-bottom: 1.5em;


I know this is the same thing, but we're getting a little loose with when we use em, rem and pixels. Not saying pixels are the way (they definitely are not), but we should probably do something more wholesale against our files at some point.

Youp, I totally get that, however I they're not necessarily replacements for each other and there is value is using them in certain ways. I'm using em's inside of the .EuiText component because we want the font-sizing, margins, line-height, etc to scale with the base font size we set for the whole of the component. We can't use 'rem' here because 'rem' is only based off the the font size set on (the whole document), so this doesn't change when changing the base font size of .EuiText. Therefore, if we use em's, thought it's based on the current element's font size and not it's parent's base font size, it will scale appropriately. Otherwise, if we use px or rem units, the spacing will be much more dramatic at the smaller scale text sizes.

After I had written all that out, I realized that since the base font size of .EuiText is known to use we can actually just do calculations on that to set margins in rem values. Though this does mean that we will have to set each margin individually at every size. But we could probably make a mixin for that.

I have thought about wrapping our px values attached to the sizing variables in the convertToRem() function so we are always using rem values. But that's for another day.

snide · 2018-04-03T23:26:28Z

src/components/text/_text.scss

  }

  blockquote {
    position: relative;
    text-align: center;
    margin-left: auto;
    margin-right: auto;
-    padding: $euiSizeL;
+    padding:  1.5em;
    max-width: 36rem;
    font-family: Georgia,Times,Times New Roman,serif;
    font-size: 112.5%;


Might wanna change this calc to match your others.

snide · 2018-04-03T23:35:05Z

src/components/text/_text.scss

-    line-height: $euiSizeXL;
+    @include euiTitle($euiFontSizeXL);
+    font-size: ($euiFontSizeXL/$euiFontSize)*100%; /* 1 */
+    margin-bottom: .6em;


Where do these margin values come from? Each title is a little different and they end up computing a little loose. Ends up kind of weird when they all have a pixel value for the margin-top.

Again, not too worried about ems themselves perse, but this ends up a little more distanced from the global variables that the previous settings. Be nice to see some sort of calculation for this stuff so that we know it scales.

I was basing the bottom margin's off of the base 8/7px scale so that when the line-height of a header doesn't allow the following item to start on the grid, I used bottom margin to push it on. Basically making up for remainder.

But let me work on the maths and see if I can come up with something that uses rem instead.

cchaos · 2018-04-04T15:07:27Z

As for the title underline on the Card, it bugged me too. But I just changed the font-weight to 200 and I'm still having the same issue...

cchaos · 2018-04-04T15:17:47Z

Ahhh it was a font-size thing. Apparently at 20px+ the underline thickens.

cchaos · 2018-04-04T15:42:22Z

@snide I think the documentation issue with the props table not showing up has to do with the fact that the component returns a cloned element. The ScreenReaderOnly comp does the same thing. I'm not sure I know how to fix that.

snide

Looks good. Minor comments to help readability. Oh. And watch out for the 500.

snide · 2018-04-04T21:44:43Z

src/global_styling/variables/_typography.scss

@@ -41,5 +49,6 @@ $euiLineHeight:     1.5;

 $euiFontWeightLight:    300 !default;
 $euiFontWeightRegular:  400 !default;
+$euiFontWeightMid:      500 !default;


This isn't available in open sans, and currently that's what is used in Kibana.

Do you know what it would fall back to?

It ends up not bold, which was the problem we were having with the form labels.

snide · 2018-04-04T21:46:59Z

src/themes/k6/k6_globals.scss

+
+$euiFontSize:       14px; // 5th position in scale
+
+$euiFontSizeXS:     $euiFontSize * nth($euiTextScale, 7); // 12px


Don't think it matters what you use, but this could probably be done a little cleaner with a map.

You're probably right but for the sake of overrides, I'm not too worried...

snide · 2018-04-04T21:53:06Z

src/global_styling/variables/_typography.scss

@@ -16,21 +16,29 @@
  font-size: convertToRem($size);
 }

+// Our base gridline is at 1/2 the font-size, ensure line-heights stay on that gridline.
+
+@mixin lineHeight($multiplier: 3) {


This is a cool way to do it.

Might wanna name this mixin a little more specific though. When I see lineHeight(3) in the other parts of the doc, first thought was wait, why is this setting the line-height to 3!

Maybe, caclulateLighHeightFromBaseline or something? Could probably use an example... if the baseline is set to 8px, the base font size is 16, and you pass a multiplier of 3, then this set the line-height to roughly 24px (converted to rems).

- Updated the font scale - Decreased line-height of large font sizes and added padding to make up the difference - Added class for medium title size and making it the default

- Takes a font-size and determines the correct line-height and font-weight - Used in all places that are titles (warning but on purpose: some components now have different weights for their titles) - This now means that we have a set hierarchy for titling and, in most cases, all titles at the same font size should look the same

- All values are in rem - All margin/line-height values are a multiple of the baseLineHeightMultiplier - Updated K6 font sizing to use the scale as well

- The mixin also recommends using the same prop names as EuiTitle as arguments (ie “xs”, “m”, “l”) - Added another mid-way font weight - Updated docs

cchaos · 2018-04-05T02:54:47Z

jenkins test this

cchaos added the style only label Apr 3, 2018

cchaos requested a review from snide April 3, 2018 17:50

snide suggested changes Apr 3, 2018

View reviewed changes

snide approved these changes Apr 4, 2018

View reviewed changes

cchaos added 16 commits April 4, 2018 22:35

Created more title levels

1103f82

- Updated the font scale - Decreased line-height of large font sizes and added padding to make up the difference - Added class for medium title size and making it the default

Changing names to xxs and xxxs

4e7eb42

More scaling & font-weight manipulation

2d180be

Fixing up some guideline text and comps

9de283a

Bolding Elastic UI in nav

bc2a7f7

Updated some components and docs

bdb168e

Defaulting euiTitle mixin font-weight to “medium”

2bdc89f

Fixing card title size/weight/underline

67f3a3d

Adding h4-h6 to the EuiText docs

bd52d64

Using map on EuiTitle for textTransform

964f2f2

Created a euiScaleText mixin just for the .EuiText component

2a0cb6a

- All values are in rem - All margin/line-height values are a multiple of the baseLineHeightMultiplier - Updated K6 font sizing to use the scale as well

Removing uppercase example from docs

c7fc95c

Fix error

e0e8236

Updating euiTitle mixin to add rem valued line-heights

855c65a

- The mixin also recommends using the same prop names as EuiTitle as arguments (ie “xs”, “m”, “l”) - Added another mid-way font weight - Updated docs

updating lineheight mixin name

ff492f5

cchaos force-pushed the more-titles branch from 1aa1725 to ff492f5 Compare April 5, 2018 02:35

cchaos added 3 commits April 4, 2018 22:38

changelog

8e4c0d2

Get rid of $euiFontWeightMid

0d9f966

remove comment

637e650

cchaos merged commit dbb717b into elastic:master Apr 5, 2018

cchaos deleted the more-titles branch April 5, 2018 03:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More & better titling #627

More & better titling #627

cchaos commented Apr 3, 2018

snide left a comment

snide Apr 3, 2018

snide Apr 3, 2018

cchaos Apr 4, 2018

snide Apr 3, 2018

snide Apr 3, 2018

cchaos Apr 4, 2018

cchaos commented Apr 4, 2018

cchaos commented Apr 4, 2018

cchaos commented Apr 4, 2018

snide left a comment •

edited

Loading

snide Apr 4, 2018

cchaos Apr 5, 2018

snide Apr 5, 2018

snide Apr 4, 2018

cchaos Apr 5, 2018

snide Apr 4, 2018

cchaos commented Apr 5, 2018


		$euiFontSize: 14px; // 5th position in scale

		$euiFontSizeXS: $euiFontSize * nth($euiTextScale, 7); // 12px

More & better titling #627

More & better titling #627

Conversation

cchaos commented Apr 3, 2018

Main takeaway

Big differences

More title levels

EuiText

Other changes

snide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos commented Apr 4, 2018

cchaos commented Apr 4, 2018

cchaos commented Apr 4, 2018

snide left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos commented Apr 5, 2018

snide left a comment •

edited

Loading