-
Notifications
You must be signed in to change notification settings - Fork 63
Set explicit ltr direction for pages untranslated in rtl languages #434
Conversation
src/pages/sources.vue
Outdated
getProviderImageCount(imageCount) { | ||
return imageCount.toLocaleString(this.$i18n.locale) | ||
// Always use EN, most sites with RTL language with numbers continue to use Western Arabic Numerals whereas `toLocaleString` will use Eastern Arabic Numerals for Arabic and Hebrew by default | ||
return imageCount.toLocaleString('en') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should check for Arabic specifically, because toLocaleString(en)
uses a comma as a thousands separator, and some European languages use comma as decimal separator, and a period or a space for thousands separator:
const num = 25000
num.toLocaleString('en')
"25,000"
num.toLocaleString('de')
"25.000"
num.toLocaleString('ru')
"25 000"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes definitely, good call. Hebrew was also using Eastern Arabic Numerals, but I solved this problem previously for Gutenberg so I will find that PR and apply a similar logic here. I can't believe I forgot about that!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Really cool that you have prior experience fixing that issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
* main: Update Sass breakpoints to match tailwind (#455) Converge `NavSection` components (#454) Use a unique `SearchGridFilter` component (#439) Set explicit ltr direction for pages untranslated in rtl languages (#434) Add VItemGroup component (#410) Update video demos for Meta Search (#420) Fix Search form on smaller viewports (#437) Make Header RTL-compatible (#429) 🔄 Synced file(s) with WordPress/openverse (#449) Removed unused image assets (#404) Restore husky (#444) Bump axios from 0.21.1 to 0.21.2 (#441) Remove husky
Fixes
Part of #421 by @sarayourfriend
Fixes #431 by @sarayourfriend
Description
Based on a conversation with @dlind1 we'll temporarily force certain untranslated pages back to
ltr
direction for RTL languages. As long as these pages are still falling back to English for RTL languages then we'll keep this around. If we find that some RTL languages are getting translated (or even some pages for it) then we revisit this and do it conditionally based on the locale and page.This also stops
toLocaleString
for provider resource count using Eastern Arabic numerals in a context when Western Arabic numerals would typically be used.Testing Instructions
Checkout this branch and run
npm run dev
. Set your locale to an RTL locale and verify that the pages modified by this PR all have their direction set to LTR.Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin