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

Add large type to gel-foundations #1530

Merged
merged 10 commits into from
Aug 2, 2019
Merged

Add large type to gel-foundations #1530

merged 10 commits into from
Aug 2, 2019

Conversation

sareh
Copy link
Contributor

@sareh sareh commented Jul 31, 2019

Part of #1508

Overall change: Add additional GEL Type sizes for large contexts (Atlas, Elephant, Imperial, Royal, Foolscap) Sizes here: https://www.bbc.co.uk/gel/guidelines/typography#additional-type-sizes-for-larger-contexts

UX context: When pairing with UX, it was decided that to start with all non-Latin font-sizes and line heights will initially be the same as those for Latin. Then they will stress-test these types with different scripts and where relevant ask for changes in a separate issue.

Code changes:

  • Add additional GEL Type sizes for large contexts (Atlas, Elephant, Imperial, Royal, Foolscap) to the scripts
  • Add the methods getAtlas etc. which can be used by others within components.

Testing Note: this first PR does not update Storybook with the typography! http://localhost:8180/?path=/story/typography--canon this needs to be carried out with an update to psammead-styles in a separate PR after this gel-foundations change has been merged. There will be a second PR for this issue #1508 to be complete.


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

@sareh sareh self-assigned this Jul 31, 2019
@sareh sareh added ws-articles Tasks for the WS Articles Team ws-home Tasks for the WS Home Team labels Jul 31, 2019
Copy link
Contributor

@DenisHdz DenisHdz left a comment

Choose a reason for hiding this comment

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

LGTM, but I have been comparing all the values with the ones UX added in the spreadsheet and I found several differences.

packages/utilities/gel-foundations/src/scripts/burmese.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/burmese.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/burmese.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/burmese.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/burmese.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/thai.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/thai.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/thai.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/thai.js Outdated Show resolved Hide resolved
packages/utilities/gel-foundations/src/scripts/thai.js Outdated Show resolved Hide resolved
Copy link
Contributor

@Bopchy Bopchy left a comment

Choose a reason for hiding this comment

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

Other than Dennis' comment, looks good - LGTM

@sareh
Copy link
Contributor Author

sareh commented Jul 31, 2019

@DenisBBC Thanks for the detailed review with all of your suggestions, Denis!
However, having checked with UX, they'd like for us to use the Latin type sizes until they have been fully stress-tested for the other scripts and to then add the corrected type sizes at that point.

Copy link
Contributor

@DenisHdz DenisHdz 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 for confirming this 👍

Sareh and others added 2 commits August 1, 2019 08:50
…ghts for non-latin scripts

Co-Authored-By: Denis Hernandez <46446236+DenisBBC@users.noreply.github.com>
@sareh
Copy link
Contributor Author

sareh commented Aug 1, 2019

This has been UX reviewed and approved.

@paruchurisilpa
Copy link
Contributor

Moving this to Ready for merge without testing as this would be tested once #1508 is done

@jamesbhobbs jamesbhobbs merged commit 1562804 into latest Aug 2, 2019
@jamesbhobbs jamesbhobbs deleted the large-type branch August 2, 2019 22:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
shared-components ws-articles Tasks for the WS Articles Team ws-home Tasks for the WS Home Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants