Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Add new typography method in Headings component #421

Merged
merged 13 commits into from
Apr 9, 2019

Conversation

DenisHdz
Copy link
Contributor

@DenisHdz DenisHdz commented Apr 5, 2019

Resolves #343

Overall change:
Add support to allow the Headings component to use Typography Type Sizes for different scripts

Code changes:

  • Allow the component to pass the script name a prop
  • Add propTypes to require a script prop
  • Remove previous GEL_CANON and GEL_TRAFALGAR imports
  • Import new getCanon and getTrafalgar methods which make use of the new getTypeSizes() method to get the appropriate GEL Type Sizes for the component.
  • Import latin.js script with the default typography GEL Types Sizes in the stories and unit test

  • I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@DenisHdz DenisHdz added the ws-home Tasks for the WS Home Team label Apr 5, 2019
@DenisHdz DenisHdz self-assigned this Apr 5, 2019
`;

Headline.defaultProps = {
script: objectOf(object).required,
Copy link
Contributor

Choose a reason for hiding this comment

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

Have raised #425 to improve this

dr3
dr3 previously approved these changes Apr 8, 2019
Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

LGTM, Great work 🎉

@DenisHdz
Copy link
Contributor Author

DenisHdz commented Apr 8, 2019

Conflicts fixed 🛠

@jamesbrumpton
Copy link
Contributor

LGTM. Puedes unirlo.

@DenisHdz DenisHdz merged commit 09884ed into latest Apr 9, 2019
@sareh sareh deleted the headings-support-scripts branch May 4, 2019 17:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants