feat(TUI-298): update headings font size #1851

jmfrancois · 2018-11-27T15:06:53Z

What is the problem this PR is trying to solve?

guideline about font heading has been udpated.
we need it to be able to use headers in our components to fix accessibility.
please check the jira

What is the chosen solution to this problem?

update font properties of headings following the guideline

impact:

Please check if the PR fulfills these requirements

The PR commit message follows our guidelines
Tests for the changes have been added (for bug fixes / features) And non reg done before need review
Docs have been added / updated (for bug fixes / features)
Related design / discussions / pages (not in jira), if any, are all linked or available in the PR

[ ] This PR introduces a breaking change

build-travis-ci · 2018-11-27T15:27:58Z

Demo is available here

build-travis-ci · 2018-11-27T15:50:02Z

Demo is available here

frassinier · 2018-11-28T09:15:44Z

packages/theme/src/theme/_type.scss

@@ -0,0 +1,19 @@
+h1,


h1, .h1, h2, .h2, h3, .h3, h4, .h4, h5, .h5, h6, .h6 { color: $black; font-weight: 600; } h1, .h1 { font-weight: 700; }

instead?

it doesnt give the same result, please check the guideline first they do not have all the same color

frassinier · 2018-11-28T09:16:55Z

packages/theme/src/theme/_variables.scss

@@ -77,7 +77,7 @@ $form-input-font-size: 16px !default;

 //** By default, this inherits from the `<body>`.
 $headings-font-family: inherit !default;
-$headings-font-weight: 500 !default;
+$headings-font-weight: 600 !default;


you can reuse $font-weight-bold

frassinier · 2018-11-28T09:17:38Z

packages/theme/src/theme/_variables.scss

@@ -77,7 +77,7 @@ $form-input-font-size: 16px !default;

 //** By default, this inherits from the `<body>`.
 $headings-font-family: inherit !default;
-$headings-font-weight: 500 !default;
+$headings-font-weight: 600 !default;
 $headings-line-height: 1.1 !default;
 $headings-color: inherit !default;


You can set it to $black here;
And remove all stuff from _type.scss file except h1 font-weight

This variable is not used anywhere and because we have different colors per title I prefer not to touch it.

When I search in our theme code, there is no use except the variables file.
When I search in our bootstrap code, there is only headings that use it.

and so ? Do you expect something from me ?

To set it to $black and remove stuff in _types.scss (see first comment by @frassinier).
(This is related to my other comment)

frassinier · 2018-11-28T09:19:18Z

packages/theme/example/index.html

@@ -7,7 +7,7 @@
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <link rel="stylesheet" href="dist/bootstrap.css" media="screen">
    <style>
-    body{padding-top:50px}body>.navbar{-webkit-transition:background-color .3s ease-in;transition:background-color .3s ease-in}@media (min-width:768px){body>.navbar-transparent{background-color:transparent}body>.navbar-transparent .navbar-nav>.open>a{background-color:transparent!important}}#home{padding-top:0}#home .navbar-brand{padding:13.5px 15px 12.5px}#home .navbar-brand>img{display:inline;margin:0 10px;height:100%}#banner{min-height:300px;border-bottom:none}.table-of-contents{margin-top:1em}.page-header h1{font-size:4em}.bs-docs-section{margin-top:6em}.bs-docs-section h1{padding-top:100px}.bs-component{position:relative}.bs-component .modal{position:relative;top:auto;right:auto;left:auto;bottom:auto;z-index:1;display:block}.bs-component .modal-dialog{width:90%}.bs-component .popover{position:relative;display:inline-block;width:220px;margin:20px}#source-button{position:absolute;top:0;right:0;z-index:100;font-weight:700}.progress{margin-bottom:10px}footer{margin:5em 0}footer li{float:left;margin-right:1.5em;margin-bottom:1.5em}footer p{clear:left;margin-bottom:0}.splash{padding:9em 0 2em;background-color:#141d27;background-image:url(../img/bg.jpg);background-size:cover;background-attachment:fixed;color:#fff;text-align:center}.splash .logo{width:160px}.splash h1{font-size:3em}.splash #social{margin:2em 0}.splash .alert{margin:2em 0}.section-tout{padding:4em 0 3em;border-bottom:1px solid rgba(0,0,0,.05);background-color:#eaf1f1}.section-tout .fa{margin-right:.5em}.section-tout p{margin-bottom:3em}.section-preview{padding:4em 0 4em}.section-preview .preview{margin-bottom:4em;background-color:#eaf1f1}.section-preview .preview .image{position:relative}.section-preview .preview .image:before{box-shadow:inset 0 0 0 1px rgba(0,0,0,.1);position:absolute;top:0;left:0;width:100%;height:100%;content:"";pointer-events:none}.section-preview .preview .options{padding:1em 2em 2em;border:1px solid rgba(0,0,0,.05);border-top:none;text-align:center}.section-preview .preview .options p{margin-bottom:2em}.section-preview .dropdown-menu{text-align:left}.section-preview .lead{margin-bottom:2em}@media (max-width:767px){.section-preview .image img{width:100%}}.sponsor #carbonads{max-width:240px;margin:0 auto}.sponsor .carbon-text{display:block;margin-top:1em;font-size:12px}.sponsor .carbon-poweredby{float:right;margin-top:1em;font-size:10px}@media (max-width:767px){.splash{padding-top:4em}.splash .logo{width:100px}.splash h1{font-size:2em}#banner{margin-bottom:2em;text-align:center}}
+    body{padding-top:50px}body>.navbar{-webkit-transition:background-color .3s ease-in;transition:background-color .3s ease-in}@media (min-width:768px){body>.navbar-transparent{background-color:transparent}body>.navbar-transparent .navbar-nav>.open>a{background-color:transparent!important}}#home{padding-top:0}#home .navbar-brand{padding:13.5px 15px 12.5px}#home .navbar-brand>img{display:inline;margin:0 10px;height:100%}#banner{min-height:300px;border-bottom:none}.table-of-contents{margin-top:1em}.page-header h1{font-size:4em}.bs-docs-section{margin-top:6em}.bs-docs-section h1{padding-top:100px}.bs-component{position:relative}.bs-component .modal{position:relative;top:auto;right:auto;left:auto;bottom:auto;z-index:1;display:block}.bs-component .modal-dialog{width:90%}.bs-component .popover{position:relative;display:inline-block;width:220px;margin:20px}#source-button{position:absolute;top:0;right:0;z-index:100;font-weight:700}.progress{margin-bottom:10px}footer{margin:5em 0}footer li{float:left;margin-right:1.5em;margin-bottom:1.5em}footer p{clear:left;margin-bottom:0}.splash{padding:9em 0 2em;background-color:#141d27;background-image:url(../img/bg.jpg);background-size:cover;background-attachment:fixed;color:#fff;text-align:center}.splash .logo{width:160px}.splash h1{font-size:3em}.splash #social{margin:2em 0}.splash .alert{margin:2em 0}.section-tout{padding:4em 0 3em;border-bottom:1px solid rgba(0,0,0,.05);background-color:#eaf1f1}.section-tout .fa{margin-right:.5em}.section-tout p{margin-bottom:3em}.section-preview{padding:4em 0 4em}.section-preview .preview{margin-bottom:4em;background-color:#eaf1f1}.section-preview .preview .image{position:relative}.section-preview .preview .image:before{box-shadow:inset 0 0 0 1px rgba(0,0,0,.1);position:absolute;top:0;left:0;width:100%;height:100%;content:"";pointer-events:none}.section-preview .preview .options{padding:1em 2em 2em;border:1px solid rgba(0,0,0,.05);border-top:none;text-align:center}.section-preview .preview .options p{margin-bottom:2em}.section-preview .dropdown-menu{text-align:left}.section-preview .lead{margin-bottom:2em}@media (max-width:767px){.section-preview .image img{width:100%}}.sponsor #carbonads{max-width:240px;margin:0 auto}.sponsor .carbon-text{display:block;margin-top:1em;font-size:12px}.sponsor .carbon-poweredby{float:right;margin-top:1em;font-size:10px}@media (max-width:767px){.splash{padding-top:4em}.splash .logo{width:100px}.splash h1{font-size:2em}#banner{margin-bottom:2em;text-align:center}}#headings h1{padding-top:0}


What's the diff here? 🤔

look at the end i have added a fix for the headings

build-travis-ci · 2018-11-28T09:44:51Z

Demo is available here

build-travis-ci · 2018-11-29T09:59:59Z

Demo is available here

build-travis-ci · 2018-12-03T11:16:04Z

Demo is available here

build-travis-ci · 2018-12-03T12:00:44Z

Demo is available here

build-travis-ci · 2018-12-21T14:53:20Z

Demo is available here

jsomsanith-tlnd · 2018-12-21T15:56:00Z

packages/theme/src/theme/_type.scss

+h1,
+.h1 {
+	font-weight: bold;
+	color: $black;


You have

$headings-color variable set to inherit

$headings-font-weight set to 600

You can apply a default that is meaningful for the headings and override only what differs inn this file

what is the default ? I dont see any 'default' in the guideline

I don t see any default meaningful in the guideline

Not a default but something meaningful. It's about implementation, not specification.
We get to decide how to do it. And the current implem has 2 vars that are set, but never really used because they are overridden by this file.

So let's say $black and 600 and you override them as needed

jsomsanith-tlnd · 2018-12-21T15:58:25Z

packages/theme/src/theme/_variables.scss

@@ -77,7 +77,7 @@ $form-input-font-size: 16px !default;

 //** By default, this inherits from the `<body>`.
 $headings-font-family: inherit !default;
-$headings-font-weight: 500 !default;
+$headings-font-weight: 600 !default;
 $headings-line-height: 1.1 !default;
 $headings-color: inherit !default;


When I search in our theme code, there is no use except the variables file.
When I search in our bootstrap code, there is only headings that use it.

build-travis-ci · 2018-12-21T16:19:57Z

Demo is available here

build-travis-ci · 2018-12-26T08:25:08Z

Demo is available here

build-travis-ci · 2018-12-26T08:41:24Z

Demo is available here

build-travis-ci · 2018-12-27T17:10:00Z

Demo is available here

build-travis-ci · 2018-12-28T08:26:55Z

Demo is available here

feat(TUI-298): update headings font size

fcf8091

jmfrancois added the need review label Nov 27, 2018

jmfrancois requested a review from frassinier November 27, 2018 15:06

jmfrancois added the breaking change label Nov 27, 2018

chore(ci): prettier

a64b115

frassinier reviewed Nov 28, 2018

View reviewed changes

chore: use font-weight-bold

d1ac0bd

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

ceda07f

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

deb6ae6

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

555cd2e

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

d913f68

jsomsanith-tlnd suggested changes Dec 21, 2018

View reviewed changes

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

09a70ca

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

66d649e

chore: follow review

af859db

jsomsanith-tlnd approved these changes Dec 26, 2018

View reviewed changes

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

93c8124

jsomsanith-tlnd added 2 commits December 28, 2018 08:54

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

27898e8

Merge branch 'master' into jmfrancois/feat/tui-298-heading-font-size

64a35e0

jsomsanith-tlnd merged commit 520279a into master Dec 28, 2018

jsomsanith-tlnd deleted the jmfrancois/feat/tui-298-heading-font-size branch December 28, 2018 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(TUI-298): update headings font size #1851

feat(TUI-298): update headings font size #1851

jmfrancois commented Nov 27, 2018 •

edited

Loading

build-travis-ci commented Nov 27, 2018

build-travis-ci commented Nov 27, 2018

frassinier Nov 28, 2018

jmfrancois Nov 28, 2018

frassinier Nov 28, 2018

frassinier Nov 28, 2018

jmfrancois Nov 28, 2018 •

edited

Loading

jsomsanith-tlnd Dec 21, 2018

jmfrancois Dec 21, 2018

jmfrancois Dec 21, 2018

jsomsanith-tlnd Dec 21, 2018

frassinier Nov 28, 2018

jmfrancois Nov 28, 2018

build-travis-ci commented Nov 28, 2018

build-travis-ci commented Nov 29, 2018

build-travis-ci commented Dec 3, 2018

build-travis-ci commented Dec 3, 2018

build-travis-ci commented Dec 21, 2018

jsomsanith-tlnd Dec 21, 2018

jmfrancois Dec 21, 2018

jmfrancois Dec 21, 2018

jsomsanith-tlnd Dec 21, 2018

jsomsanith-tlnd Dec 21, 2018

build-travis-ci commented Dec 21, 2018

build-travis-ci commented Dec 26, 2018

build-travis-ci commented Dec 26, 2018

build-travis-ci commented Dec 27, 2018

build-travis-ci commented Dec 28, 2018

feat(TUI-298): update headings font size #1851

feat(TUI-298): update headings font size #1851

Conversation

jmfrancois commented Nov 27, 2018 • edited Loading

build-travis-ci commented Nov 27, 2018

build-travis-ci commented Nov 27, 2018

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

jmfrancois Nov 28, 2018 • 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

build-travis-ci commented Nov 28, 2018

build-travis-ci commented Nov 29, 2018

build-travis-ci commented Dec 3, 2018

build-travis-ci commented Dec 3, 2018

build-travis-ci commented Dec 21, 2018

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

build-travis-ci commented Dec 21, 2018

build-travis-ci commented Dec 26, 2018

build-travis-ci commented Dec 26, 2018

build-travis-ci commented Dec 27, 2018

build-travis-ci commented Dec 28, 2018

jmfrancois commented Nov 27, 2018 •

edited

Loading

jmfrancois Nov 28, 2018 •

edited

Loading