Reduce number of different font-sizes in Bartik for better consistency

Hey thanks for this.

I think we should try and strip out some of these on a case per case basis. There must be some cases where we can simplify the font size declarations and selectors that use them.

There is also another issue to do with font sizes and accessibility to be aware of here #2247319: Improve visibility of Bartik's smallest font elements.

+Adding tags

We need to tackle the issue of .node__content declaring a font size of 1.071em to every element it contains.

This is causing the following problems:

We moved around how content is printed in the node template and this has caused font size problems in this issue. Which we were forced to add a quick fix to deal with it for now.
#2455211: Comment field displayed last regardless of assigned weight

I have also noticed that we have:

a H1 font size declaration.

h1, .heading-a {
  font-size: 1.357em;
  ...
}

a H1 page title declaration...

h1#page-title {
  font-size: 2em;
  ...
}

and a .node__content font size declaration which affects the original H1 styling.

.node__content {
  font-size: 1.071em;
}

So we have this visual problem in nodes to do with H1 headings right now as demonstrated below...

There should be one
declaration in Bartik that is not affected by anything else to keep h1's always at the same size.

Tagging.

h1#page-title {
In Seven we just removed the page-title styling completely and made that the default h1 styling.

.node__content {
  font-size: 1.071em;
}

It's causes loads of problems if you change the font size on a wrapper as it has loads of unexpected consequences on the elements inside, does anyone know why do we do this?

+1 to remove .node__content { font-size: }

Let's do this!

From inspecting the calculated values in the browser, here are the calculated px sizes and the components that use them.

Base elements

html: 14px
h1 : 19px
h2 : 16px
h3 : 15px
h4 : 14px
h5 : 12px
h6 : 9px

Modified node content base sizes (because of .node__content declared em size)

p : 15px
h1 : 30px
h2 : 21.4264px
h3 : 16.058px
h4 : 15.743px
h5 : 12.8499 px
h6 : 9.6412 px

Specific font sizes set in the CSS

11px

comment__time
comment__permalink
comment-form .filter-wrapper .tips

12px

node__meta
region-header block
ul.links
comment links li
field--type-entity-reference field-label (tags)
field--type-entity-reference ul.links (tags)
filter-help a
forum-description
site-footer content
site-footer table

13px

region-primary-menu menu
region-secondary-menu menu
pager__item
sidebar block content
breadcrumb
comment__context
demo-block
input, textarea (for desktop)
form-item label
forum-name
site-slogan
search-form input
table
skip-link
tabs ul.primary li a
ui-dialog input
ui-dialog button
views-display-column details summary
maintenance-page name-and-slogan

14px

node--view-mode-teaser node__content
site-footer h2

15px

comment__meta
comment__content h3
node__content
sidebar h2

16px

input, textarea (for mobile)

20px

featured-bottom h2
featured-top h2
main-content h2

28px

page-title

Planned changes:

Base styles:

body: 13px (as it has the most declarations)
h1 : 28px
h2: 20px
h3: 15px
h4: 14px (it currently rounds up to 15px and has the same styles as the h3)
h5: 12px
h6 : 9px

This will cut down a huge amount of font declarations.

As we are removing the font declaration on node__content class I will add a bunch of node__content h2, node__content h3 etc classes to put the font sizes back to the weird originals they currently have. Resizing these (and causing very obvious visual changes) can be further discussed.

I made a start with this. I rounded any strange font sizes that to help them fit the base sizes and I removed a lot of font size declarations.

I haven't touched the .node__content class yet. The font sizes as a result of font-size: 1.071em give such strange font sizes which should not be set as the base font sizes.

Those font sizes are:
p : 15px
h1 : 30px
h2 : 21.4264px
h3 : 16.058px
h4 : 15.743px
h5 : 12.8499px
h6 : 9.6412px

Any advice on what to do with them, the base font size of 13px which fits in with the rest of the site is very different.

There are now 43 declarations of font sizes instead of 57.

Also I left comments of the calculated pixel sizes next to all of the font-size declarations to help out, these need to be eventually removed but it took me hours to work everything out :)

While we're at it, would it be worth converting these to rems? That would help prevent problems if font sizes get set on any wrappers in the future.

@mdrummond http://caniuse.com/#feat=rem There could be some gotcha's I wonder if we'd then have to double them up for IE9-11 BC.

As long as we're not using rem in font short-hand, it's fine for IE9 and IE10. So as long as they're declared with font-size, no problem.

I see no problem adding rems. I thought the easiest way without bike shedding the use of rems in Core was to overhaul what was going on with ems.

Also can someone please advise me on the predicament I mentioned in #15?

The node__content font-size declaration guarantees that everything in that wrapper is set to a base of 15px. Which does not match any of the other content on the site. Do we keep this as is? Or declare font sizes for any possible text component within node__content?

I recommend setting the body size to 15px and removing the node__content size declaration.

The majority content is using that as base. Other regions can undo that if they need to.

We could split off switching to rems to another issue.

I think I understand the idea behind .node__content but I think it's really odd logic. A node could appear anywhere, in the footer, sidebar, etc. I think it's just a simplistic idea of a node always being a full page.

I tried to go through and figure out where we can reduce the number of font sizes we have. There are too many that are only a pixel apart.

1. +++ b/core/themes/bartik/css/base/elements.css
   @@ -43,32 +43,33 @@ h1,
   -  font-size: 1.092em;
   +  font-size: 1.154em; /* 15px */
   

   I think we should make this 16px and remove all instances of 15px.

2. +++ b/core/themes/bartik/css/components/breadcrumb.css
   @@ -5,5 +5,5 @@
   -  font-size: 0.929em;
   +/*  font-size: 0.929em; /* 13px */
   

   Change to 12px

3. +++ b/core/themes/bartik/css/components/comments.css
   @@ -39,17 +38,18 @@
      line-height: 1.2;
   +  font-size: 1.154em; /* 15px */
   

   Change to 14px or 16px

4. +++ b/core/themes/bartik/css/components/comments.css
   @@ -39,17 +38,18 @@
   -  font-size: 0.733em;
   +  font-size: 0.846em; /* 11px */
   ...
   -  font-size: 0.733em;
   +  font-size: 0.846em; /* 11px */
   

   Change to 12px

5. +++ b/core/themes/bartik/css/components/sidebar.css
   @@ -50,11 +50,10 @@
   -  font-size: 1.071em;
   +  font-size: 1.154em; /* 15px */
   

   Change to 14px or 16px

6. +++ b/core/themes/bartik/css/components/table.css
   @@ -73,7 +73,7 @@ tr th {
   -  font-size: 1em;
   +  font-size: 1em; /* matches default table content 12.8498582839966px */
   

   Is this needed? What CSS is this overidding?

7. +++ b/core/themes/bartik/css/components/ui-dialog.css
   @@ -26,7 +26,7 @@
   -  font-size: 0.929em;
   +  font-size: 0.909em; /* 13px */
   

   Make this 12px

Crazy question but have we looked at the baseline and looked at what our multiplier is that will give us an understanding of how to treat the node body?

Rerolled

My first reroll. There were some conflicts but I think I managed to solve them correctly.

Ok, so this is just a reroll from #15, without the feedback from #22, right?

Interdiff's are always great.

Good to wait to move rem (root em) units to another issue.

That's correct, #25 was a reroll from #15. Here's the interdiff (I hope I got it right).

I'll start working on the actual issue now.

Thanks!

Does this need the <h3>Why this should be an RC target</h3> info & RC phase evaluation table? Also there's the "rc target triage" tag.

I'd like this in for the 8.0 release for sure.

core/themes/bartik/css/components/form.css
<<<<<<< HEAD
font-size: 16px;
=======
font-size: 1.142857143em; /* TODO 16px */
>>>>>>> 25

Just a reroll for now.

Keeping the px here per https://www.drupal.org/node/2516918 Prevent mobile browsers from zooming on all form inputs.

Needs issue summary update
Are we doing #22 in order to not have as many different font sizes? Setting the body size to 15px sounded good. But, is #22 saying that we should keep .node__content because the node could be put into any region?

I reckon we should change the issue title because "Investigate font-size" doesn't sound actionable.

Changing title from Investigate font-size consistency in Bartik
to Reduce number of different font-sizes in Bartik for better consistency

Issue summary should also briefly outline what is changing.

Have the comments from #22 been addressed in the latest patch?

Is there consensus about what to do with .node__content ? I can't tell from the discussion.

I wonder if @LewisNyman and @emma.maria should revisit this and provide some extra clarity?

We're running out of time to get this in for 8.0.

So, we CAN still do this! Bartik is not frozen.

Someone can work on:
Needs reroll + tackle comments in #22.

If you know what to do about .node__content you can work on that too, or that can be done after we have consensus.

We definitely can do this monster of an issue!

However this needs to be set to a 'Plan' first. This is because of the amount time this issue has been left + the range of opinions + the requests for consensus on the way forward.

Yay font size headache!

Patch #33 is failed to apply to 9.1x.
Work on the comments in #22.
https://www.drupal.org/project/drupal/issues/2399247#comment-10281061
Here is the fixed review patch.

This appears to be a bartik only issue. And since bartik has been removed from core in D10 moving to the contrib module.

Also moving to NW as there is a needs IS update.