Conversation
|
1ee9eee to
663c899
Compare
📚 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 |
6e4689f to
6f5b191
Compare
There was a problem hiding this comment.
Stole this from adobe/spectrum-css#3368
| function withLocaleWrapperRender( | ||
| args: Record<string, unknown> & { lang?: string; 'default-slot'?: string }, | ||
| context: { globals: { lang?: string } } | ||
| ): ReturnType<typeof html> { | ||
| const contextLang = context.globals?.lang; | ||
| const lang = args.lang ?? contextLang; | ||
| const key = getTranslationKey(lang) as TranslationKey; | ||
| const langTranslations = translations[key] ?? translations.en; | ||
| // Always use translated label so Language toolbar drives the visible text | ||
| const labelText = langTranslations['fieldlabel.label']; | ||
|
|
||
| return html` | ||
| <div lang=${ifDefined(lang ?? undefined)}> | ||
| ${template({ ...args, 'default-slot': labelText })} | ||
| </div> | ||
| `; |
There was a problem hiding this comment.
Stole this from adobe/spectrum-css#3368
6f5b191 to
9a7eba8
Compare
|
@marissahuysentruyt great start on this! I think the switching of the kits is working however the actual
I think we should let the There is a secondary setting of the Maybe we need to use |
|
|
||
| ```css | ||
| adobe-clean-han-japanese, Adobe Clean Han, sans-serif | ||
| adobe-clean-han-japanese, 'Adobe Clean Han', sans-serif |
There was a problem hiding this comment.
Probably encourage to use the token for this font-family? You could expose the string in a comment or something if you like. Otherwise the note about the "stack" has a disconnect if we don't show the token.
/* adobe-clean-han-traditional, "Adobe Clean Han Traditional Chinese", adobe-clean-han-simplified-c, "Adobe Clean Han Simplified Chinese", adobe-clean-han-hong-kong, "Adobe Clean Han Hong Kong", adobe-clean-han-japanese, "Adobe Clean Han Japanese", adobe-clean-han-korean, "Adobe Clean Han Korean", Adobe Clean Han, sans-serif */
font-family: var(--swc-cjk-font-family-stack);
| 2. Look for `wf-*-active` classes on the iframe's `<html>` element (e.g. `wf-adobe-clean-han-japanese-n4-active`). If missing, the kit script loaded but `Typekit.load()` may not have been called | ||
| 3. Check the browser console for `Typekit.load() failed` warnings | ||
| 4. Verify the kit ID in `preview-head.html` (`__SWC_FONT_KIT_IDS__`) is correct and the Adobe Fonts web project is published | ||
| 5. Try a hard refresh — `Ctrl+Shift+R` / `Cmd+Shift+R` — to clear cached scripts |
There was a problem hiding this comment.
I would add another note aboute actually inspecting the element, possibly as item number 1 since simple inheritance/font-family property value might be interferring.
| * textfield.value from translations.json based on context.globals.lang. | ||
| * Used by the Fonts guide and for locale/font demos. This story is "docs-only." | ||
| */ | ||
| function withLocaleWrapperRender( |
There was a problem hiding this comment.
Would love to see this worked into the Typography stories - another ticket would be fine!
| "animation-linear": "cubic-bezier(0, 0, 1, 1)", | ||
| "cjk-font": "var(--swc-cjk-font-family-stack)", | ||
| "cjk-font-family-stack": "adobe-clean-han-japanese, Adobe Clean Han, sans-serif;", | ||
| "cjk-font-family-stack": "adobe-clean-han-traditional, 'Adobe Clean Han Traditional Chinese', adobe-clean-han-simplified-c, 'Adobe Clean Han Simplified Chinese', adobe-clean-han-hong-kong, 'Adobe Clean Han Hong Kong', adobe-clean-han-japanese, 'Adobe Clean Han Japanese', adobe-clean-han-korean, 'Adobe Clean Han Korean', Adobe Clean Han, sans-serif;", |
There was a problem hiding this comment.
If these are finalized, will you please also run yarn deploy in this package to re-generate the extension file?
There was a problem hiding this comment.
Actually, this comment was a great reminder for a related question I had!
On Adobe Fonts, there's only 2 or 3 fonts listed in the stack for each of the new CJK projects. For example, for the Japanese kit, Adobe Fonts recommends: font-family: adobe-clean-han-japanese, sans-serif;. However, after reviewing your typography PR, I saw that S2 design guidance site lists quite a few more just for :lang(ja).
What are your thoughts on me updating the variable stacks to reflect the S2 site fallbacks? I think that more or less achieves the "support CJK fonts," and leans more towards the "full fidelity."
Then I'll run the yarn deploy!
There was a problem hiding this comment.
Interesting - maybe do a little comparison to React's font stacks? It is also a bit different than the S2 site, however I'm worried that the list on the S2 site wouldn't be font names that an actual system would recognize, hence why I looked to see if React offered any clues. Looks like they haven't adapted for a couple years either, so maybe it's a general follow-up to check with design/the fonts team?
There was a problem hiding this comment.
I got confirmation from Matt K in the implementations channel that the font stack is accurate. I also found a thread in the i18n channel that said the fallbacks with the Japanese characters are fine- that's how the fonts would show up on a Japanese user's system.
Overall, the fallbacks haven't change from S1 to S2 (and now it makes sense why React's stack hasn't changed). I elected to follow the S2 site more to include the Japanese characters (and some of the numbering/suffixes like "Pr6N" 🤷♀️)
| }, | ||
| "cjk-font-family-stack": { | ||
| "value": "adobe-clean-han-japanese, {cjk-font-family}, sans-serif;" | ||
| "value": "{font-family-chinese-traditional}, {font-family-chinese-simplified}, {font-family-hong-kong}, {font-family-japanese}, {font-family-korean}, {cjk-font-family}, sans-serif;" |
- adds kitId map for 5 lang attribute values - loads the initial Latin based font kit by default
- adds withLocaleWrapperRender() function to wrap the status light in a new div with a lang attribute - the WithLocaleWrapper status light story is docs only. it is used in the fonts.mdx guide to showcase the language decorator loading the correct CJK font kits, as well as internationalization/translations.
- when the decorator runs, it injects the specific CJK font kit and calls typekit.load properly
9a7eba8 to
ee2fbe4
Compare
- ensures all functionality from original adobe font embed script is supported in language decorator - adds mutation observer to clean up TypeKit-injected style tags (so there's not a bunch of "dead" style tags of unrelated locale classes)
- Replaces the single combined CJK :lang() rule with individual locale font-family rules for Arabic, Hebrew, Chinese (Traditional, Simplified, Hong Kong), Japanese, and Korean. Each locale now resolves to its own font-family token instead of the generic cjk-font stack. - Adds LOCALE_FONT_MAP to typography.js mapping locale selectors to their font-family tokens - Removes langSelectorList and CJK_SELECTOR_LIST (replaced by direct iteration over LOCALE_FONT_MAP with cssBlock) - Regenerate typography.css with new :lang() rules - Update preview-head.html nav overrides to match new pattern
pfulton
left a comment
There was a problem hiding this comment.
Thanks for getting this taken care of!

Description
OVERVIEW: When a user selects a locale from the Language toolbar, the corresponding Adobe Fonts kit is injected and activated via Typekit.load(). Latin, Arabic, and Hebrew locales continue to use the default kit loaded at startup. Arabic and Hebrew locales also control the
dir="rtl".This PR adds CJK (Chinese, Japanese, Korean) font support to the 2nd-gen Storybook, but introduces a few extra things that felt relevant to validation. It introduces a language decorator to verify the Adobe Fonts kit loading on-demand, and updated CJK typography tokens. In the Fonts guide, this PR also introduces a small internationalized status light story that uses translations.
Key changes:
Motivation and context
2nd-gen Storybook had no support for CJK fonts. This PR enables developers to preview components to verify that CJK font kits are loaded via JS. It also provides onboarding documentation so contributors understand the font loading architecture.
Related issue(s)
Screenshots (if appropriate)
Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
fhs4zuyin the browser console (window.curentKitIdcbs0ykg), Chinese Simplified (dck3siu), Chinese Traditional (ueg0fjf), and Chinese Hong Kong (kje4xva)preview-head.html:Switching back to Latin: Verify default kit is restored
RTL direction: Verify direction attribute updates
Fonts documentation page: Verify the guide renders correctly
WithLocaleWrapperstatus light with translated labelsDevice review