Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RTL subheadline block #1801

Closed
5 tasks done
twinlensreflex opened this issue May 28, 2019 · 12 comments
Closed
5 tasks done

RTL subheadline block #1801

twinlensreflex opened this issue May 28, 2019 · 12 comments
Assignees
Labels
ws-articles Tasks for the WS Articles Team

Comments

@twinlensreflex
Copy link
Contributor

twinlensreflex commented May 28, 2019

Is your feature request related to a problem? Please describe.
We want our subheadline block to work for RTL articles (Persian, Arabic, Urdu, Pashto).

Describe the solution you'd like
As a result of #2713, the NassimPersian font should be loaded for all 4 services and we should already have the text direction being right-to-left. This ticket is to ensure that the implementation matches the designs (except for it not using the correct service-specific font).

If the font sizes and line heights need updating, they should be discussed with UX design before implementing any changes gel-foundations

For each service we should import the Arabic typography settings from here: import { arabic } from '@bbc/gel-foundations/scripts'

  • subheadline block is RTL and matches UX designs (except for service-specific font)
  • Font is bold
  • Font size is: 38px (<600px), 44px otherwise
  • id for subheadline should be made of the text contained in the tags but with spaces replaced with -s and punctuation removed.
  • handle ZWNJ (aka half space) special character in IDs by replacing it with a hyphen - ensure we have a unit test & storybook example with text that has this zero-width-non-joiner character.

Testing notes

  • test fallback font on older devices

Testing URL's
https://www.test.bbc.com/arabic/articles/c69dvq19k63o
https://www.test.bbc.com/persian/articles/c4vlle3q337o
https://www.test.bbc.com/urdu/articles/cx621klkm1ro
https://www.test.bbc.com/pashto/articles/cng0e8v85eko

Dev insight: Update unit snapshot tests.

Additional context
Add any other context or screenshots about the feature request here.

@twinlensreflex twinlensreflex added Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. ws-articles Tasks for the WS Articles Team labels May 28, 2019
@amywalkerdev amywalkerdev added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label May 29, 2019
@amywalkerdev amywalkerdev removed the Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. label May 29, 2019
@sareh
Copy link
Contributor

sareh commented Jun 24, 2019

Blocked on adding Nassim Persian to Simorgh #1910

@jamesdonoh
Copy link
Contributor

No longer blocked on #1910; now blocked on BBC-archive/psammead#543

@jamesdonoh jamesdonoh removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 23, 2019
@jamesdonoh
Copy link
Contributor

The above issue was closed, assuming this is now unblocked so we should try to pick up.

@jimjohnsonrollings
Copy link
Contributor

I suspect that all of this works already - so maybe this needs just to be to check everything is correct.

@jimjohnsonrollings jimjohnsonrollings added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 26, 2019
@jimjohnsonrollings
Copy link
Contributor

Blocked pending #2732

@nataliesmart
Copy link

Subhead (H2) type sizes:
Group A (0-319px): font size is 24px with line height 34px
Group B (320-599px): font size is 32px with line height 38px
Group D (600px +): font size is 36px with line height 50px

@jamesdonoh
Copy link
Contributor

Unblocked following BBC-archive/psammead#1516

@jamesdonoh jamesdonoh removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jul 31, 2019
@RayNjeri RayNjeri added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Aug 5, 2019
@RayNjeri
Copy link

RayNjeri commented Aug 5, 2019

Blocked pending #2713

@RayNjeri RayNjeri removed the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Aug 7, 2019
@RayNjeri RayNjeri self-assigned this Aug 8, 2019
@RayNjeri
Copy link

Subhead (H2) type sizes:
Group A (0-319px): font size is 24px with line height 34px
Group B (320-599px): font size is 32px with line height 38px
Group D (600px +): font size is 36px with line height 50px

Checked as per the description and per the values specified here by @nataliesmart and everything seems to check off, both on Chrome and Firefox. Further UX review is welcomed.
cc: @jamesdonoh
Screen Shot 2019-08-13 at 11 14 56 AM

@RayNjeri
Copy link

Same applies on live.
Screen Shot 2019-08-13 at 11 51 22 AM

@jamesdonoh
Copy link
Contributor

Closing this issue as we believe the treatment on live is as expected, please reopen if not.

@nataliesmart
Copy link

There's no subhead on the second, third and fourth links to check. The first link's subhead matches the type specs above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

No branches or pull requests

7 participants