Skip to content

Conversation

@lazd
Copy link
Member

@lazd lazd commented Feb 6, 2020

Description

This PR does the following:

  • Fixes Typography's metadata status
  • Fixes Typography's usage of heading level elements (i.e. <h1>)
  • Fixed Typography's description text
  • Removes duplicate Typography examples
  • Hides deprecated Typography components from the dev site menu
  • Adds a landing page for Typography, link to deprecated components
  • Adds Spectrum site badge for verifying links are correct
  • Adds the ability to change the "Variants" section title in dev site
  • Fixes incorrect Spectrum site link for ButtonGroup
  • Hides Tool from dev site menu, link to Tool from Quiet Action Button

How and where has this been tested?

  • How this was tested: gulp dev, use eyes
  • Browser(s) and OS(s) this was tested with: n/a

Docs are deployed here for your perusal: https://git.corp.adobe.com/pages/lawdavis/spectrum-css-typography/docs/typography.html

Screenshots

image

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.

@lazd lazd requested a review from bernhard-adobe February 6, 2020 20:26
@@ -0,0 +1,19 @@
name: Typography (Legacy)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically what we had before in typography-deprecated.yml

@@ -1,19 +1,181 @@
id: heading-display
name: Typography (Deprecated II)
name: Typography (Deprecated)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically what we had before in typography.yml.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to maybe indicate what was the deprecated V1, V2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no version number for the previous implementations of typography, they were both present when we moved to individual versioning. Calling them legacy and deprecated is the best we can do here, I'm afraid.

header.spectrum-CSSComponent-sectionHeading(id="variants")
h4.spectrum-Heading3
a(href="#variants").spectrum-BigSubtleLink Variants
a(href="#variants").spectrum-BigSubtleLink=`${component.examplesHeading ? component.examplesHeading : 'Variants'}`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just makes it possible for the Typography landing page to say "Components" instead of "Variants"

@@ -1,12 +1,16 @@
name: Typography (Internationalized)
fastLoad: false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it so the right Typekit loads on the dev site when you click this example in the sidenav.

name: componentData.name,
component: componentName,
hide: componentData.hide,
fastLoad: componentData.fastLoad,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nav data as passed to the template is its own structure, so we have to copy these properties here.

//-
a.spectrum-Card.spectrum-Card--small.spectrum-Card--horizontal(href=`${spectrumURL || 'https://spectrum.corp.adobe.com/'+pkg.name.split('/').pop()}` target="_blank")
if component.SpectrumSiteSlug
a.spectrum-Card.spectrum-Card--small.spectrum-Card--horizontal(href=`${component.SpectrumSiteSlug}` target="_blank")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just basically enables the card that links to the Spectrum docs.

@@ -1,7 +1,8 @@
name: Tool
status: Verified
dnaStatus: Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status is now "dnaStatus"?

Copy link
Member Author

@lazd lazd Feb 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, they are two separate things, which work together in concert with the data from DNA in a very confusing fashion to display whether a component is deprecated or whatnot.

It's best to ignore this, otherwise we'll be reworking tons of unrelated stuff to untangle the mess ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the clarification, all good!

<p class="spectrum-Body spectrum-Body--XS">Ut et lectus finibus, aliquet mauris eu, tincidunt mi. Donec scelerisque orci sit amet venenatis luctus. Morbi eget lacus est. Duis iaculis magna quis aliquam lacinia. Lorem ipsum dolor sit amet, consectetur adipiscing elit.</p>
<h1 class="spectrum-Heading spectrum-Heading--XXS">Lorem Ipsum Dolor</h1>
<h6 class="spectrum-Heading spectrum-Heading--XXS">Lorem Ipsum Dolor</h6>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is all fine and good but I am wondering if this matters as XXXL & XXL are also h1-s and if we want to encourage that. I thought the idea of t-shirt sizing was to remove the link between numeric title sizes and heading sizes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, <h1>-<h6> correspond to semantic heading levels. The top-level heading on a page must be a <h1>, but depending on the design intent, it could be XL, XXL, or XXXL. For instance, once could use a <h1> that is XXL (formerly display) as the top-level heading on the main site landing page, but could use a <h1> that is a XL elsewhere for the top-level heading. This all depends on the design.

Since there is no such thing as a <h7> tag, and because of the way the sizing plays out, the 3 largest sizes (XXXL, XXL, XL) are all shown with <h1> tags.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for explaining your reasoning. this is good to me

@@ -1,19 +1,181 @@
id: heading-display
name: Typography (Deprecated II)
name: Typography (Deprecated)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to maybe indicate what was the deprecated V1, V2?

@@ -1,12 +1,12 @@
name: Typography (Int., Deprecated I)
name: Typography (Internationalized, Deprecated)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought having this short helps to not have it over-flow in the sidebar as that is the title of the link to this page.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are not present in the sidebar. Please view the docs site here https://git.corp.adobe.com/pages/lawdavis/spectrum-css-typography/docs/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, got removed from the sidebar. It's fine.

name: Typography (Int., Deprecated I)
name: Typography (Internationalized, Deprecated)
status: Deprecated
hide: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we sure we want to hide the old typography?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very, very confusing otherwise. We link to the deprecated typography from the landing page, so it's not orphaned.

Copy link
Contributor

@bernhard-adobe bernhard-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @lazd for your hard work correcting the documentation.

@bernhard-adobe bernhard-adobe merged commit f871b74 into SDS-2513 Feb 7, 2020
@bernhard-adobe bernhard-adobe deleted the typography-metadata branch February 7, 2020 00:59
lazd added a commit that referenced this pull request Feb 7, 2020
* docs: swap out the Typekit Ids
* docs: typography metadata fixes and deprecation bits (#523)
* docs: support hiding examples, disabling fastLoad
* docs: support changing Variants heading, enable Spectrum cards
* docs: Typography landing page
* docs: hide old Typography (but link to it from the main page)
* docs: remove Tool from menu, fix ButtonGroup Spectrum link
* docs: add link to Tool in ActionButton description

Co-authored-by: Larry Davis <lazdnet@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants