Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 42 additions & 18 deletions 2nd-gen/packages/swc/.storybook/decorators/language.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,30 @@ import { addons, makeDecorator, useEffect } from '@storybook/preview-api';
import type {} from '../storybook-env';

const DEFAULT_KIT_ID = 'obc6cux';
const RTL_LANGS = new Set(['ar', 'fa', 'he']);

let languageListenerAttached = false;

function resolveTextDirection(
lang: string | false,
textDirection: string | undefined
): 'ltr' | 'rtl' {
if (textDirection === 'ltr' || textDirection === 'rtl') {
return textDirection;
}

return RTL_LANGS.has(String(lang ?? 'en-US')) ? 'rtl' : 'ltr';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? 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')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

}

/**
* Applies the selected language (lang/dir) to the document and loads the corresponding
* Applies the selected language/dir to the document and loads the corresponding
* Adobe Fonts kit if needed. Safe to call from the decorator or from a toolbar listener
* so that font loading works even when the decorator does not re-run (e.g. on some docs pages).
*/
function applyLanguageAndFontKit(lang: string | false, isRTL: boolean): void {
function applyLanguageAndFontKit(
lang: string | false,
textDirection: 'ltr' | 'rtl'
): void {
const langAttr = lang ? String(lang) : 'en-US';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'

const root = document.documentElement;

Expand All @@ -33,7 +48,10 @@ function applyLanguageAndFontKit(lang: string | false, isRTL: boolean): void {
root.setAttribute('lang', langAttr);
hasChanged = true;
}
root.setAttribute('dir', isRTL ? 'rtl' : 'ltr');

// Apply both semantic and CSS direction so docs/stories and bidi behavior stay aligned.
root.setAttribute('dir', textDirection);
root.style.direction = textDirection;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated!


// If the fonts are actively loading, do not re-trigger the load
if (window.FontsLoading === true) {
Expand Down Expand Up @@ -173,35 +191,41 @@ function attachLanguageChangeListener(): void {
languageListenerAttached = true;
try {
const channel = addons.getChannel();
channel.on('globalsUpdated', (payload: { globals?: { lang?: string } }) => {
const lang = payload?.globals?.lang ?? 'en-US';
const isRTL = ['ar', 'fa', 'he'].includes(lang);
applyLanguageAndFontKit(lang, isRTL);
});
channel.on(
'globalsUpdated',
(payload: { globals?: { lang?: string; textDirection?: string } }) => {
const lang = payload?.globals?.lang ?? 'en-US';
const textDirection = resolveTextDirection(
lang,
payload?.globals?.textDirection
);
applyLanguageAndFontKit(lang, textDirection);
}
);
} catch {
// Storybook event bus not available (e.g. in test env); decorator-only path still works.
}
}

/**
* @type import('@storybook/csf').DecoratorFunction<import('@storybook/web-components').WebComponentsFramework>
**/
export const withLanguageWrapper = makeDecorator({
name: 'withLanguageWrapper',
parameterName: 'lang',
wrapper: (StoryFn, context) => {
const { globals: { lang = false } = {}, id, viewMode } = context;
const {
globals: { lang = false, textDirection = 'auto' } = {},
id,
viewMode,
} = context;

// Add a textDirection property to the globals for use in the stories
// fa/Farsi is currently not included in the storybook toolbar map
const isRTL = ['ar', 'fa', 'he'].includes(lang);
context.globals.textDirection = isRTL ? 'rtl' : 'ltr';
// Keep an explicit, resolved direction available for stories.
const resolvedTextDirection = resolveTextDirection(lang, textDirection);
context.globals.resolvedTextDirection = resolvedTextDirection;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - no need for this now, so I removed it :)


attachLanguageChangeListener();

useEffect(() => {
applyLanguageAndFontKit(lang, isRTL);
}, [lang, id, viewMode]);
applyLanguageAndFontKit(lang, resolvedTextDirection);
}, [lang, resolvedTextDirection, id, viewMode]);

return StoryFn(context);
},
Expand Down
18 changes: 18 additions & 0 deletions 2nd-gen/packages/swc/.storybook/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,29 @@ const preview = {
dynamicTitle: true,
},
},
textDirection: {
name: 'Direction',
description:
'Text direction for stories. Auto follows language; LTR/RTL overrides it.',
defaultValue: 'auto',
type: 'string',
toolbar: {
title: 'Direction',
icon: 'transfer',
items: [
{ value: 'auto', title: 'Auto' },
{ value: 'ltr', title: 'LTR' },
{ value: 'rtl', title: 'RTL' },
],
dynamicTitle: true,
},
},
},
initialGlobals: {
theme: 'light',
scale: 'medium',
lang: 'en-US',
textDirection: 'auto',
},
decorators: [
withContext,
Expand Down
Loading