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 headline block #1799

Closed
3 tasks done
twinlensreflex opened this issue May 28, 2019 · 21 comments
Closed
3 tasks done

RTL headline block #1799

twinlensreflex opened this issue May 28, 2019 · 21 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 headline 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'

  • text block is RTL and matches UX designs (except for service-specific font)
  • Font is bold
  • Font size is: 38px (<600px), 44px otherwise

Testing notes

  • test fallback font on older devices
  • check with UX designers

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 relevant unit 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 articles-next-epic labels May 28, 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
@twinlensreflex twinlensreflex changed the title RTL Headline block RTL Text block May 29, 2019
@twinlensreflex twinlensreflex changed the title RTL Text block RTL Headline block May 29, 2019
@sareh sareh added the blocked This issue should not be worked on until another internal issue is completed - see desc for details label Jun 24, 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.

@jamesdonoh jamesdonoh changed the title RTL Headline block RTL headline block Jul 25, 2019
@jimjohnsonrollings
Copy link
Contributor

I believe that other than the bolding everything for this is already done.

@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

Headline (H1) type sizes:
Group A (0-319px): font size is 32px with line height 42px
Group B (320-599px): font size is 38px with line height 46px
Group D (600px +): font size is 44px with line height 54px

@jamesdonoh
Copy link
Contributor

Unblocked following that 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
Copy link
Contributor

RayNjeri commented Aug 5, 2019

Blocked pending #2713

@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
@rhenshaw56
Copy link
Contributor

I believe this could be worked on side by side with #2713 removing the blocked label

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

Headline (H1) type sizes:
Group A (0-319px): font size is 32px with line height 42px
Group B (320-599px): font size is 38px with line height 46px
Group D (600px +): font size is 44px with line height 54px

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 10 46 06 AM

@RayNjeri
Copy link
Contributor

Same applies on live.
Screen Shot 2019-08-13 at 11 44 49 AM

@stevencava
Copy link

Headline block looks ok on all four services, font render issue of Urdu on mobile (This is a known issue on iOS) @nataliesmart

@jimjohnsonrollings
Copy link
Contributor

@stevencava What's the Urdu iOS issue?

@stevencava
Copy link

Image from iOS (12)

@jimjohnsonrollings
Copy link
Contributor

Mine doesn't do that once Nassim has loaded.

@stevencava
Copy link

When I tap the refresh button it corrects itself

@jimjohnsonrollings
Copy link
Contributor

Then I think we're happy with this :)

@stevencava
Copy link

Just noticed it seems to do it when off wifi

@jimjohnsonrollings
Copy link
Contributor

What we're seeing as 'stretched' is the iOS default font, which will display if you've not managed to download Nassim quickly enough - from second page view Nassim should have successfully loaded.

@jamesdonoh
Copy link
Contributor

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

@nataliesmart
Copy link

I have also just checked this and all seems ok on my Chrome Version 76.0.3809.100. Thanks everyone.

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

9 participants