feat: adding t-shirt sized typography (SDS-2513) #210 #408

bernhard-adobe · 2019-11-15T21:16:18Z

Covers:
SDS-2513 Adding t-shirt sized typography. Issue: #210

SDS-3259 Update Spectrum CSS Typography documentation: Spectrum CSS NextJS & legacy documentation

SDS-4405 Verify Spectrum CSS Typography Updates

SDS-4403 Split Typography documentation for Spectrum CSS

SDS-4460 Update Typekit / Adobe Fonts (Thanks @misterbrownlee )

SDS-4453 adobe-clean-ux support can be removed #416

Description

Updating Spectrum DNA to get t-shirt sized tokens.
Modifying processors.js to allow t-shirt sizes. Some minor restructuring of shared methods.
Adding t-shirt sized: sizes, margins and color
Enabling the debug statement the processors.js for colors and sizes.
Splitting Typography pages
Moving old Typography into Depreciated

For validation the following tokens can be searched:

Paperdoc summary of all changes: https://paper.dropbox.com/doc/Typography-Update--AopzJsKrFFzH_O8gexjcxdEdAg-bluAkl9HGke8WKkJvow2N
(updated t-shirt sized): https://git.corp.adobe.com/gist/nbaldwin/3364cf137d8bd50097c3bedbde9218e5

(original t-shirt sized): https://git.corp.adobe.com/Spectrum/spectrum-dna/pull/310

How and where has this been tested?

How this was tested:
Browser(s) and OS(s) this was tested with:

Ran Backstop.js tests a couple of times: Latest test is https://spectrum-visual-regression.ci.corp.adobe.com/job/spectrum-css-vr-test-asset/157/artifact/spectrum-css/backstop_data/html_report/index.html

Tested on MacOS 10.14.5 (18F203)
on Chrome Version 80.0.3968.0 (Official Build) canary (64-bit)

Run:

yarn install 
gulp dev

Visit locally:

New pages:

Body
Code
Detail
Heading

Old pages:
Typography page
Typography International page

Screenshots

To-do list

If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

This pull request is ready to merge.

tools/component-builder/css/processors.js

README.md

NateBaldwinDesign · 2019-11-22T17:48:17Z

components/typography/index.css

+
+
+/* T-Shirt sizes for .spectrum-Article */
+.spectrum-Article {


Serif is an option independent of an Article. For the update, I'm not sure it makes any sense to retain this old wrapper concept 🤔. @GarthDB pinging for input.

@GarthDB @NateBaldwinDesign we have 2 options here:

Keep it wrapped in this .spectrum-Article wrapper class and duplicate all the rules outside in the root level. This will increase file size.

Remove this wrapper class and move all of the classes in the root level. This will be most efficient in class name reduction.

Let me know what you prefer.

I'd default to @NateBaldwinDesign on this. The wrapper is a convenience if everything in the article would use the serif if that's not the case, article should be a variant and not a wrapper.

Thanks @GarthDB . I will remove the wrapper class and update the documentation.

components/typography/index.css

NateBaldwinDesign · 2019-11-22T17:53:00Z

components/typography/index.css

+  @mixin typographyTShirtSizes .spectrum-BodyXS, body-serif-XS {}
+  @mixin typographyTShirtSizes .spectrum-BodyXXS, body-serif-XS {}
+
+  @mixin typographyTShirtSizes .spectrum-HeadingXXXL--light, heading-light-XXXL {}


Not sure why we would include sans-serif classnames under "Article"? I expected to see only the serif options in this wrapper class considering the previously mentioned paradigm of the previous typography

whoops, my bad. should be all serif.

NateBaldwinDesign · 2019-11-22T17:55:39Z

components/typography/index.css

+  @mixin typographyTShirtSizes .spectrum-HeadingXL--heavy, heading-heavy-XL {}
+  @mixin typographyTShirtSizes .spectrum-HeadingL--heavy, heading-heavy-L {}
+
+  /* These don't exist */


is there an issue already filed to un-comment these when DNA is released with these options?

logged here as a subtask of the typography updates:

SDS-4454 Typography heading sizes: missing DNA tokens

NateBaldwinDesign · 2019-11-25T15:50:57Z

@bernhard-adobe please make sure that all font weights are included in the Typekit for Spectrum-CSS. It appears that weights heavier than bold are not included. This screenshot shows Heading XXL default (top) and Heading XXL Strong (bottom). By default, Heading is bold (font weight 700), but for <strong>, the font weight is black (font weight 900). DNA is providing the proper weight value, but it does not appear to be rendering.

Looks like this issue exists for the current typography as well.

The font weights we need to support are:
Light, regular, bold, extrabold (han only), black. All of these weights need to support italic.

In DNA, we have them specified with the following numbers, which correspond to CSS font weight numbers.

Numeric	Weight
300	Light
400	Regular
700	Bold
800	Extra-bold
900	Black

NateBaldwinDesign · 2019-11-25T16:24:27Z

Here is an additional reference comparing font weights as we have them defined in DNA and what is being rendered in Spectrum-CSS. I've added a hideous thick red border to the items that have incorrect font weights in CSS. Again, I believe this is due to the Typekit not pulling in all necessary styles/weights of Adobe Clean.

NateBaldwinDesign · 2019-11-25T16:43:03Z

It also appears as though the internationalized fonts are not included either. When reviewing the Han examples, the letterforms "XXL" did not appear correct. Upon inspection I believe that this is not Adobe Clean Han. It would be good if we have all of the font families specified for CJK languages included in Spectrum-CSS's docs site so that we can accurately review the implementation.

Below is a screenshot of Spectrum-CSS Han example as-is:

And here is how it would appear when referencing Adobe Clean Han:

NateBaldwinDesign · 2019-11-25T16:49:49Z

It appears none of the font stacks are coming directly from DNA. This becomes evident in the fonts listed to use for Arabic and Hebrew, which differ from what's defined in DNA. I would like to discuss this file:

spectrum-css/components/commons/fonts.css

Line 32 in 2f4fab6

    
           --spectrum-font-family-ar: 'adobe-arabic', 'myriad-arabic', var(--spectrum-font-fallbacks-sans);

and how we can ensure that CSS pulls these fallback font stacks from DNA here: https://git.corp.adobe.com/Spectrum/spectrum-dna/blob/master/data/aliases/StaticAliases.mjs#L59

NateBaldwinDesign · 2019-11-25T16:57:51Z

components/typography/metadata/typography.yml

+        <p class="spectrum-DetailM">DetailL <em class="spectrum-DetailM--light">DetailM Emphasis</em> <strong class="spectrum-DetailL--strong">DetailM Strong</strong>.</p>
+        <p class="spectrum-DetailS">DetailL <em class="spectrum-DetailS--light">DetailS Emphasis</em> <strong class="spectrum-DetailL--strong">DetailS Strong</strong>.</p>


Docs incorrectly label examples as "DetailL" for Medium and Small

It appears none of the font stacks are coming directly from DNA. This becomes evident in the fonts listed to use for Arabic and Hebrew, which differ from what's defined in DNA. I would like to discuss this file:

spectrum-css/components/commons/fonts.css

Line 32 in 2f4fab6

--spectrum-font-family-ar: 'adobe-arabic', 'myriad-arabic', var(--spectrum-font-fallbacks-sans);

and how we can ensure that CSS pulls these fallback font stacks from DNA here: https://git.corp.adobe.com/Spectrum/spectrum-dna/blob/master/data/aliases/StaticAliases.mjs#L59

Just to clarify: arabic & hebrew use myriad and the rest is going to use adobe clean in it's international versions?
I am not sure why those were hard-coded here, maybe @GarthDB knows more about this.

is there an option for --spectrum-font-family-base: 'adobe-clean-ux', 'adobe-clean', 'Source Sans Pro', var(--spectrum-font-fallbacks-sans); ?

components/typography/metadata/typography.yml

NateBaldwinDesign · 2019-11-25T17:00:13Z

Please add examples of "Detail Light" and "Detail Serif" to this section

components/typography/metadata/typography.yml

bernhard-adobe · 2019-12-03T18:48:19Z

Here is an additional reference comparing font weights as we have them defined in DNA and what is being rendered in Spectrum-CSS. I've added a hideous thick red border to the items that have incorrect font weights in CSS. Again, I believe this is due to the Typekit not pulling in all necessary styles/weights of Adobe Clean.

We will have to ask @misterbrownlee for either making the update or providing us the credentials.
Ticket here: https://jira.corp.adobe.com/browse/SDS-4460

bernhard-adobe · 2019-12-03T18:49:22Z

It also appears as though the internationalized fonts are not included either. When reviewing the Han examples, the letterforms "XXL" did not appear correct. Upon inspection I believe that this is not Adobe Clean Han. It would be good if we have all of the font families specified for CJK languages included in Spectrum-CSS's docs site so that we can accurately review the implementation.

Below is a screenshot of Spectrum-CSS Han example as-is:

And here is how it would appear when referencing Adobe Clean Han:

This looks like another update to the Typekit instance.

bernhard-adobe

Please add examples of "Detail Light" and "Detail Serif" to this section

I think the class spectrum-Article as a parent will set the detail into serif.
Maybe @GarthDB knows if we should continue with this wrapper class to get serif.

I added the light option. Notice the serif option below the san-serif.

GarthDB · 2019-12-09T21:16:51Z

Great and extensive work @bernhard-adobe and @NateBaldwinDesign.

NateBaldwinDesign · 2019-12-11T22:11:50Z

@bernhard-adobe just to follow-up on the Slack conversation: for serif, we should use the classname spectrum-Typography--serif as a replacement for spectrum-Article.

bernhard-adobe · 2019-12-12T00:04:53Z

blocked currently by the Typekit update: https://jira.corp.adobe.com/browse/SDS-4460

lazd · 2019-12-12T19:32:41Z

@bernhard-adobe now that the Typekit updated has been made, is there anything else remaining on this PR? If not, we can go ahead and merge and release it tomorrow!

bernhard-adobe · 2019-12-13T04:10:43Z

@lazd This is ready for review.
We should discuss if we should hide of of that depreciated typography as I feel that it might clutter up a bit the sidebar navigation.

lazd · 2019-12-13T06:27:56Z

@bernhard-adobe the sidebar clutter is real, but it shouldn't hold up this PR.

@GarthDB, if this is good to go in your opinion, I say merge it!

lazd · 2020-02-06T18:37:21Z

I am working on correcting the component statuses and links on this PR and will merge shortly.

bernhard-adobe · 2020-02-06T22:48:44Z

@lazd branched of this branch and created PR #523 to address some more documentation issues and bugs.

* docs: support hiding examples, disabling fastLoad * docs: support changing Variants heading, enable Spectrum cards * docs: correct Typography example status, markup errors, and <h1> usage * add Typography landing page * also, hide old Typography (but link to it from the main page) * docs: remove Tool from menu, fix ButtonGroup Spectrum link * also, add link to Tool in ActionButton description * docs: correct Heading component description * docs: use the descriptions from the Spectrum site * docs: correct spelling mistake

bernhard-adobe added Component: Typography labels Nov 15, 2019

bernhard-adobe requested review from GarthDB, NateBaldwinDesign and lazd November 15, 2019 21:16

bernhard-adobe commented Nov 22, 2019

View reviewed changes

tools/component-builder/css/processors.js Outdated Show resolved Hide resolved

bernhard-adobe commented Nov 22, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

NateBaldwinDesign reviewed Nov 22, 2019

View reviewed changes

NateBaldwinDesign reviewed Nov 25, 2019

View reviewed changes

components/typography/metadata/typography.yml Outdated Show resolved Hide resolved

NateBaldwinDesign reviewed Nov 25, 2019

View reviewed changes

components/typography/metadata/typography.yml Outdated Show resolved Hide resolved

NateBaldwinDesign reviewed Nov 25, 2019

View reviewed changes

components/typography/metadata/typography.yml Outdated Show resolved Hide resolved

NateBaldwinDesign reviewed Nov 25, 2019

View reviewed changes

components/typography/metadata/typography.yml Outdated Show resolved Hide resolved

bernhard-adobe commented Dec 3, 2019

View reviewed changes

bernhard-adobe force-pushed the SDS-2513 branch from 189f8c3 to 440dfdc Compare December 5, 2019 23:28

GarthDB approved these changes Dec 9, 2019

View reviewed changes

bernhard-adobe changed the title ~~WIP feat: adding t-shirt sized typography (SDS-2513) #210~~ feat: adding t-shirt sized typography (SDS-2513) #210 Dec 13, 2019

bernhard-adobe and others added 23 commits February 5, 2020 16:34

feat: adding t-shirt sizes typography (SDS-2513)

d3e9355

fix: feedback from Nate (SDS-2513)

71354df

fix: addressing font-family feedback (SDS-2513)

e5d4c90

fix: adding em to code (SDS-2513)

df1e747

fix: split typography & international updates (SDS-2513)

faa6227

fix: removing background testing colors and temp files (SDS-2513)

77a054b

fix: changing naming convention (SDS-2513)

4be7e19

fix: updating documentation for typography classnames (SDS-2513)

c971901

fix: removing log-colors and fixing docs (SDS-2513)

6a217ea

fix: removing temp typography page (SDS-2513)

ac758fc

docs: removing line seperators (SDS-2513)

191a010

feat: adding SUIT rules (SDS-2513)

6657a4b

fix: removing more selectors (SDS-2513)

190243a

fix: suggested simplifications for em, strong children (SDS-2513)

06ab81f

fix: temp swapping out of typekit

6ee1adb

fix: swapping out the Typekit Ids

3813c60

fix: updating international page

b066e60

fix: typo on international page

1ce21d2

fix: typekit changes

7b8f9c6

fix: fixing dropzone typeface

1bb1efe

fix: updating correct weights

12c235a

fix: updating 404 heading

42b0b5a

docs: fix status and spelling errors

56acd4b

lazd force-pushed the SDS-2513 branch from a2919fa to 56acd4b Compare February 6, 2020 00:45

lazd approved these changes Feb 7, 2020

View reviewed changes

lazd merged commit 3921bcb into master Feb 7, 2020

GarthDB deleted the SDS-2513 branch February 14, 2020 17:56



		/* T-Shirt sizes for .spectrum-Article */
		.spectrum-Article {

		<p class="spectrum-DetailM">DetailL <em class="spectrum-DetailM--light">DetailM Emphasis</em> <strong class="spectrum-DetailL--strong">DetailM Strong</strong>.</p>
		<p class="spectrum-DetailS">DetailL <em class="spectrum-DetailS--light">DetailS Emphasis</em> <strong class="spectrum-DetailL--strong">DetailS Strong</strong>.</p>

feat: adding t-shirt sized typography (SDS-2513) #210 #408

feat: adding t-shirt sized typography (SDS-2513) #210 #408

Uh oh!

Conversation

bernhard-adobe commented Nov 15, 2019 • edited by lazd Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How and where has this been tested?

Screenshots

To-do list

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NateBaldwinDesign commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NateBaldwinDesign commented Nov 25, 2019

Uh oh!

NateBaldwinDesign commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NateBaldwinDesign commented Nov 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bernhard-adobe Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NateBaldwinDesign commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bernhard-adobe commented Dec 3, 2019

Uh oh!

bernhard-adobe commented Dec 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bernhard-adobe left a comment

Choose a reason for hiding this comment

Uh oh!

GarthDB commented Dec 9, 2019

Uh oh!

NateBaldwinDesign commented Dec 11, 2019

Uh oh!

bernhard-adobe commented Dec 12, 2019

Uh oh!

lazd commented Dec 12, 2019

Uh oh!

bernhard-adobe commented Dec 13, 2019

Uh oh!

lazd commented Dec 13, 2019

Uh oh!

lazd commented Feb 6, 2020

Uh oh!

bernhard-adobe commented Feb 6, 2020

Uh oh!

Reviewers

bernhard-adobe commented Nov 15, 2019 •

edited by lazd

Loading

NateBaldwinDesign commented Nov 25, 2019 •

edited

Loading

NateBaldwinDesign commented Nov 25, 2019 •

edited

Loading

bernhard-adobe Dec 3, 2019 •

edited

Loading

NateBaldwinDesign commented Nov 25, 2019 •

edited

Loading

bernhard-adobe commented Dec 3, 2019 •

edited

Loading