feat(docs): add RTL decorator#6188
Conversation
|
📚 Branch Preview Links🔍 First Generation Visual Regression Test ResultsWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
Deployed to Azure Blob Storage: If the changes are expected, update the |
pfulton
left a comment
There was a problem hiding this comment.
Looks great. I clicked-through to the deployed Storybook instance and was able to follow the testing instructions. Everything worked as expected.
Thanks for taking care of this!
| return textDirection; | ||
| } | ||
|
|
||
| return RTL_LANGS.has(String(lang ?? 'en-US')) ? 'rtl' : 'ltr'; |
There was a problem hiding this comment.
?? only short‑circuits on null | undefined. When lang === false this evaluates to String(false) === 'false', which never matches RTL_LANGS. You get LTR for the right reason by accident. You can use lang || 'en-US' to mirror the convention used in applyLanguageAndFontKit (lang ? String(lang) : 'en-US')
| context.globals.textDirection = isRTL ? 'rtl' : 'ltr'; | ||
| // Keep an explicit, resolved direction available for stories. | ||
| const resolvedTextDirection = resolveTextDirection(lang, textDirection); | ||
| context.globals.resolvedTextDirection = resolvedTextDirection; |
There was a problem hiding this comment.
This can be an anti-pattern of mutating context.globals during render. If you want to keep this would you like to add a comment explaining the contract. I just want to avoid the future contributors to propagate this pattern.
There was a problem hiding this comment.
You're right - no need for this now, so I removed it :)
| root.setAttribute('dir', textDirection); | ||
| root.style.direction = textDirection; |
There was a problem hiding this comment.
dir="rtl" on an element already maps to direction: rtl via the UA stylesheet. This will add an inline specificity which can fight overrides further down. Can you show me an example where the dir attribute alone is not propagating?
| lang: string | false, | ||
| textDirection: 'ltr' | 'rtl' | ||
| ): void { | ||
| const langAttr = lang ? String(lang) : 'en-US'; |
There was a problem hiding this comment.
nit: This could also be updated to lang || 'en-US', since the truthy path is always going to be a string.
Also what happens if you are passed a invalid lang string? I know this is just storybook, but we might want to consider some stronger type safety.
There was a problem hiding this comment.
If we pass an invalid string here right now it will silently LTR. The set check is RTL_LANGS.has(String(lang || 'en-US')), so any string not in {'ar','fa','he'} → 'ltr'
Description
Adds a dedicated Direction Storybook toolbar control (
auto/LTR/RTL) alongside the existing Language control. Direction resolution:The
withLanguageWrapperdecorator applies the resolved direction on the preview root using both:dirondocument.documentElement(semantic / bidi), anddirectionvia inline CSS on the same element (explicit visual direction).context.globals.resolvedTextDirectionis set so stories or tooling can read the finalltr|rtlvalue. TheglobalsUpdatedlistener was updated so direction changes apply even when the decorator does not re-run (e.g. some docs flows).Motivation and context
Language alone implied RTL for some locales, which made it awkward to test LTR vs RTL without changing locale, especially when pairing a specific font/locale with a fixed reading direction. A separate Direction control fixes that.
This also unblocks Chromatic VRT: you can template permutations (e.g. theme × scale × explicit LTR/RTL) without relying on switching to an RTL locale only to get
dir/layout. Stories or Chromatic configuration can key offglobals.textDirection/resolvedTextDirection.Author's checklist
dir; no new interactive component in the library.)Reviewer's checklist
Manual review test cases
Direction toolbar + document root
<html>: expectdir="rtl"and inlinedirection: rtl(or equivalent).dir="ltr"and LTR CSS.Auto follows locale