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

Implement a language switch so that users can choose to view the site in English, Hebrew, or Arabic #1113

Closed
5 tasks done
gissoo opened this issue Sep 21, 2022 · 11 comments
Closed
5 tasks done
Assignees

Comments

@gissoo
Copy link
Contributor

gissoo commented Sep 21, 2022

testing notes (QA)

  • On the QA site, when logged in, confirm there is a language switcher with 3 languages (english, hebrew, arabic) matching the designs below
  • On desktop and mobile (if possible), confirm it switches the language of the current page
  • Navigate away and confirm the language change persists
  • Log out and confirm the switcher is still present, but only shows English and Hebrew
  • Confirm the language switcher options are properly aligned

dev notes

@blms here are the designs – I'll eventually move them to the Components pages – I've provided select and hover designs for LTR and RTL pages for light and dark mode on desktop and mobile

so on desktop and mobile the toggles will stay in the nav, for mobile this means that they are visible and usable once you've selected the burger menu

You can see the designs on the pages if you zoom out of the Figma page – let me know if you need anything

@gissoo
Copy link
Contributor Author

gissoo commented Sep 27, 2022

@blms I've applied the Semibold Greta Sans to "E" in the designs
I also noticed the "עִ" wasn't centered in the toggle container, I have fixed that as well, fwiw.

"E" is 20px on desktop and 18px on mobile (semibold weight Greta Sans for default and selected)
"עִ" is 26px on desktop and 20px on mobile (regular weight Frank Ruehl for both default and selected)
"ع" is 20px on desktop and 16px on mobile. (bold weight Hassan for both default and selected)

blms added a commit that referenced this issue Sep 27, 2022
- Also refactor type styles into typography module
[run percy]
rlskoeser added a commit that referenced this issue Sep 27, 2022
@blms blms added the 🗜️ awaiting testing Implemented and ready to be tested label Sep 28, 2022
@kseniaryzhova
Copy link

@gissoo the language symbol for Hebrew looks out of line with the English and Arabic
image

@blms
Copy link
Contributor

blms commented Sep 28, 2022

@kseniaryzhova Ooh, good catch. That looks like a browser difference, since I'm now seeing similar on Firefox but not Safari or Chrome.

@kseniaryzhova
Copy link

@gissoo couldn't test on mobile since I'm off campus, but everything else works on desktop! Should I close?

@blms
Copy link
Contributor

blms commented Sep 28, 2022

@kseniaryzhova Please leave open and I'll fix the alignment issue you raised. (Also I should mention, the Arabic font is a placeholder, still waiting for the Hassan font from @gissoo but should be ready soon.)

@blms blms added ⚠️ tested needs attention Has been through acceptance testing and needs additional work and removed 🗜️ awaiting testing Implemented and ready to be tested labels Sep 28, 2022
@kseniaryzhova
Copy link

@gissoo I have the same alignment issue on Chrome too.

@gissoo
Copy link
Contributor Author

gissoo commented Oct 3, 2022

@blms @kseniaryzhova thanks for the comments – @blms the alignment problem exists even after fixing them in the designs? (because I know it was misaligned in the designs)

blms added a commit that referenced this issue Oct 3, 2022
blms added a commit that referenced this issue Oct 4, 2022
@blms blms added 🗜️ awaiting testing Implemented and ready to be tested and removed ⚠️ tested needs attention Has been through acceptance testing and needs additional work labels Oct 4, 2022
@kseniaryzhova
Copy link

@gissoo @blms alignment issue still exists for me - Arabic is too small (but I know the font isn't ready yet) but the Hebrew is again out of alignment with the English E.
image

@kseniaryzhova kseniaryzhova added ⚠️ tested needs attention Has been through acceptance testing and needs additional work and removed 🗜️ awaiting testing Implemented and ready to be tested labels Oct 5, 2022
@blms
Copy link
Contributor

blms commented Oct 5, 2022

@kseniaryzhova Interesting! I suspected this might be a Windows vs Mac thing, and it is indeed. It has to do with the font's "vertical metrics".

Here it is on both Windows and Mac Chrome with the margins I set highlighted in orange. Notice how in the Windows screenshot, the distance of the letter from the orange margin is smaller than it is on Mac. So basically on Mac this font has some extra space on top of each character! (Or Windows on the bottom...)
Untitled
Screen Shot 2022-10-05 at 12 02 26 PM

Going to try using this tool (which I found in a StackOverflow thread) to edit the fonts and see if that fixes it.

blms added a commit that referenced this issue Oct 5, 2022
blms added a commit that referenced this issue Oct 5, 2022
Fix issue with FrankRuhl font vertical metrics (#1113)
@blms blms added 🗜️ awaiting testing Implemented and ready to be tested and removed ⚠️ tested needs attention Has been through acceptance testing and needs additional work labels Oct 5, 2022
@blms
Copy link
Contributor

blms commented Oct 5, 2022

Should be all set now @kseniaryzhova!

@kseniaryzhova
Copy link

@blms looks great, even the Arabic! Thanks so much, closing!

@rlskoeser rlskoeser removed the 🗜️ awaiting testing Implemented and ready to be tested label Oct 6, 2022
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

No branches or pull requests

4 participants