Skip to content

Commit a77171e

Browse files
author
Michael Jordan
committed
fix(combobox, breadcrumbs): propagate lang/dir for a single item's language without breaking layout
Previously, setting `lang`/`dir` on a slotted `<sp-menu-item>` (Combobox) or `<sp-breadcrumb-item>` (Breadcrumbs) had no effect on the rendered popover/overflow-menu counterpart, since both components synthesize new elements from extracted data rather than reusing the originals. - Combobox: forwards `lang`/`dir` from slotted items (or `.options` data) onto the rendered popover `<sp-menu-item>`, and now syncs the input's own `lang` to the committed option's language for correct pronunciation. - Breadcrumbs: same propagation for the "More items" overflow menu. - BreadcrumbItem: forwards `lang`/`dir` to `#item-link` only, so a single item's language does not flip its own layout or its separator's mirrored chevron. The separator now explicitly tracks the *ambient* direction (nearest ancestor `dir`, or the document default) instead of inheriting the host's own `dir` attribute via `:dir()`, including live updates when an ancestor's `dir` changes after mount, and across the shadow-root boundary for the overflow-menu wrapper item. Along the way, worked around two non-obvious base-class behaviors that caused regressions mid-fix: `SpectrumElement` overrides `dir` to return computed CSS direction rather than the attribute (must read `getAttribute('dir')` for authored values), and CSS `:dir()` resolves directionality via the attribute chain, independent of the `direction` property. Adds Storybook stories (`languageOfParts` / `LanguageOfParts`) and test coverage for each fix, each verified to fail without its corresponding code change. Jira: SWC-2359
1 parent afe0954 commit a77171e

10 files changed

Lines changed: 574 additions & 4 deletions

File tree

1st-gen/packages/breadcrumbs/src/BreadcrumbItem.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,30 @@ export interface BreadcrumbSelectDetail {
3030
value: string;
3131
}
3232

33+
// Walks up through `parentElement`, crossing shadow-root boundaries via
34+
// `getRootNode().host` — needed because `Breadcrumbs.renderMenu()` places
35+
// an "is-menu" `BreadcrumbItem` inside its own shadow root rather than as
36+
// a light-DOM child, so a plain `parentElement` walk would stop there
37+
// without ever reaching `<sp-breadcrumbs>` itself.
38+
function* ancestorElements(start: Element): Generator<Element> {
39+
let node: Node = start;
40+
for (;;) {
41+
const parent = (node as Element).parentElement;
42+
if (parent) {
43+
yield parent;
44+
node = parent;
45+
continue;
46+
}
47+
const root = node.getRootNode();
48+
if (root instanceof ShadowRoot) {
49+
yield root.host;
50+
node = root.host;
51+
continue;
52+
}
53+
break;
54+
}
55+
}
56+
3357
export class BreadcrumbItem extends LikeAnchor(Focusable) {
3458
public static override get styles(): CSSResultArray {
3559
return [styles, chevronStyles];
@@ -49,12 +73,40 @@ export class BreadcrumbItem extends LikeAnchor(Focusable) {
4973
return this.shadowRoot.querySelector('#item-link') as HTMLElement;
5074
}
5175

76+
// `renderLink()` and `renderSeparator()` bake this host's own `lang`/`dir`
77+
// and the ambient `direction` into explicit attributes at render time.
78+
// Neither `lang` nor `dir` is a reactive Lit property here, so — unlike
79+
// plain CSS inheritance — none of that updates on its own if this host's
80+
// own `lang`/`dir` change, or an ancestor's `dir` changes, after mount.
81+
// Watching this host (for `lang`/`dir`) and every ancestor (for `dir`)
82+
// keeps both cases live.
83+
private readonly ancestorDirObserver = new MutationObserver(() =>
84+
this.requestUpdate()
85+
);
86+
5287
override connectedCallback(): void {
5388
super.connectedCallback();
5489

5590
if (!this.hasAttribute('role')) {
5691
this.setAttribute('role', 'listitem');
5792
}
93+
94+
this.ancestorDirObserver.observe(this, {
95+
attributes: true,
96+
attributeFilter: ['dir', 'lang'],
97+
});
98+
99+
for (const ancestor of ancestorElements(this)) {
100+
this.ancestorDirObserver.observe(ancestor, {
101+
attributes: true,
102+
attributeFilter: ['dir'],
103+
});
104+
}
105+
}
106+
107+
override disconnectedCallback(): void {
108+
this.ancestorDirObserver.disconnect();
109+
super.disconnectedCallback();
58110
}
59111

60112
private announceSelected(value: string): void {
@@ -90,10 +142,25 @@ export class BreadcrumbItem extends LikeAnchor(Focusable) {
90142
}
91143

92144
protected renderLink(): TemplateResult {
145+
// Forward `lang`/`dir` from the host onto the link so a single item's
146+
// language only affects its own text rendering, not the host's own
147+
// `direction` (which the separator mirrors via `:dir(rtl)`); see
148+
// `:host { direction: inherit; }` in breadcrumb-item.css. Read the raw
149+
// `dir` attribute rather than `this.dir` — `SpectrumElement` overrides
150+
// the native `dir` getter to return the *computed* CSS direction, which
151+
// is exactly what `direction: inherit` decouples from the attribute.
93152
return html`
94153
<a
95154
id="item-link"
96155
href=${ifDefined(!this.isLastOfType ? this.href : undefined)}
156+
lang=${ifDefined(this.lang || undefined)}
157+
dir=${ifDefined(
158+
(this.getAttribute('dir') || undefined) as
159+
| 'ltr'
160+
| 'rtl'
161+
| 'auto'
162+
| undefined
163+
)}
97164
tabindex="0"
98165
aria-current=${ifDefined(this.isLastOfType ? 'page' : undefined)}
99166
@keydown=${this.handleKeyDown}
@@ -105,11 +172,20 @@ export class BreadcrumbItem extends LikeAnchor(Focusable) {
105172
}
106173

107174
private renderSeparator(): TemplateResult {
175+
// `:dir()` resolves directionality by walking up the `dir` *attribute*
176+
// chain, independent of the CSS `direction` property — so it still
177+
// picks up this host's own `dir` (set for `#item-link`'s content, per
178+
// `renderLink()`) even though `:host { direction: inherit; }` keeps the
179+
// host's own *computed* direction tied to the ambient context. Read that
180+
// computed value and set it explicitly here so `:dir()` on `#separator`
181+
// resolves from its own accurate attribute instead of the host's.
182+
const ambientDir = getComputedStyle(this).direction as 'ltr' | 'rtl';
108183
return html`
109184
<sp-icon-chevron100
110185
id="separator"
111186
size="xs"
112187
class="spectrum-UIIcon-ChevronRight100"
188+
dir=${ambientDir}
113189
></sp-icon-chevron100>
114190
`;
115191
}

1st-gen/packages/breadcrumbs/src/Breadcrumbs.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ type BreadcrumbItem = {
4848
value: string;
4949
offsetWidth: number;
5050
isVisible: boolean; // false if displayed in menu overlay
51+
lang?: string;
52+
dir?: string;
5153
};
5254

5355
/**
@@ -190,6 +192,11 @@ export class Breadcrumbs extends SpectrumElement {
190192
value: el.value || index.toString(),
191193
offsetWidth: width,
192194
isVisible: true,
195+
lang: el.lang || undefined,
196+
// `SpectrumElement` overrides `dir` to return the *computed* CSS
197+
// direction rather than the attribute, so read the attribute
198+
// directly to capture the item's own authored direction override.
199+
dir: el.getAttribute('dir') || undefined,
193200
};
194201
});
195202
}
@@ -293,7 +300,12 @@ export class Breadcrumbs extends SpectrumElement {
293300
294301
${this.items.map(
295302
(item) => html`
296-
<sp-menu-item href=${ifDefined(item.href)} value=${item.value}>
303+
<sp-menu-item
304+
href=${ifDefined(item.href)}
305+
value=${item.value}
306+
lang=${ifDefined(item.lang)}
307+
dir=${ifDefined(item.dir as 'ltr' | 'rtl' | 'auto' | undefined)}
308+
>
297309
${item.label}
298310
</sp-menu-item>
299311
`

1st-gen/packages/breadcrumbs/src/breadcrumb-item.css

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,10 @@
1919
:host([disabled]) {
2020
pointer-events: none;
2121
}
22+
23+
/* A `dir` set on this host (e.g. to localize one item's text) should not
24+
flip this item's own layout or its separator's mirrored chevron; only
25+
`#item-link` (see BreadcrumbItem.ts) picks up that direction. */
26+
:host {
27+
direction: inherit;
28+
}

1st-gen/packages/breadcrumbs/stories/breadcrumbs.stories.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,43 @@ AddItemsDynamic.swc_vrt = {
114114
skip: true,
115115
};
116116

117+
// Autonyms (each language's name rendered in that language), alphabetized
118+
// by the rendered text so both LTR and RTL scripts are interleaved.
119+
const languages = [
120+
{ value: 'de', label: 'Deutsch', lang: 'de', dir: 'ltr' },
121+
{ value: 'en', label: 'English', lang: 'en', dir: 'ltr' },
122+
{ value: 'es', label: 'Español', lang: 'es', dir: 'ltr' },
123+
{ value: 'fr', label: 'Français', lang: 'fr', dir: 'ltr' },
124+
{ value: 'ru', label: 'Русский', lang: 'ru', dir: 'ltr' },
125+
{ value: 'he', label: 'עברית', lang: 'he', dir: 'rtl' },
126+
{ value: 'ar', label: 'العربية', lang: 'ar', dir: 'rtl' },
127+
] as const;
128+
129+
export const LanguageOfParts = (args: StoryArgs): TemplateResult => {
130+
return html`
131+
<sp-breadcrumbs
132+
${spreadProps(args)}
133+
max-visible-items=${ifDefined(args['max-visible-items'])}
134+
@change=${args.onChange}
135+
>
136+
${languages.map(
137+
(language) => html`
138+
<sp-breadcrumb-item
139+
value=${language.value}
140+
lang=${language.lang}
141+
dir=${language.dir}
142+
>
143+
${language.label}
144+
</sp-breadcrumb-item>
145+
`
146+
)}
147+
</sp-breadcrumbs>
148+
`;
149+
};
150+
LanguageOfParts.args = {
151+
'max-visible-items': 3,
152+
};
153+
117154
export const ShowRoot = (args: StoryArgs): TemplateResult => {
118155
return html`
119156
<sp-breadcrumbs

1st-gen/packages/breadcrumbs/test/breadcrumb-item.test.ts

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,175 @@ describe('Breadcrumb Item', () => {
103103
expect(changeSpy.callCount).to.equal(1);
104104
expect(changeSpy).to.have.been.calledWith('home');
105105
});
106+
107+
it('scopes a per-item `dir` override to `#item-link`, not its own layout', async () => {
108+
const el = await fixture<HTMLElement>(html`
109+
<sp-breadcrumbs>
110+
<sp-breadcrumb-item value="he" lang="he" dir="rtl">
111+
עברית
112+
</sp-breadcrumb-item>
113+
<sp-breadcrumb-item value="en">English</sp-breadcrumb-item>
114+
</sp-breadcrumbs>
115+
`);
116+
117+
await elementUpdated(el);
118+
119+
const rtlItem = el.querySelector(
120+
'sp-breadcrumb-item[value="he"]'
121+
) as BreadcrumbItem;
122+
await elementUpdated(rtlItem);
123+
124+
// The host's own box keeps the ambient (LTR) direction, so its
125+
// separator does not mirror out of step with its LTR siblings.
126+
expect(getComputedStyle(rtlItem).direction, 'host direction').to.equal(
127+
'ltr'
128+
);
129+
130+
// Only the link carrying the item's text picks up the override.
131+
const itemLink = rtlItem.shadowRoot.querySelector(
132+
'#item-link'
133+
) as HTMLElement;
134+
expect(itemLink.dir, 'item-link dir').to.equal('rtl');
135+
expect(itemLink.lang, 'item-link lang').to.equal('he');
136+
expect(
137+
getComputedStyle(itemLink).direction,
138+
'item-link direction'
139+
).to.equal('rtl');
140+
});
141+
142+
it('mirrors the separator to match the ancestor dir, or the document default otherwise', async () => {
143+
const el = await fixture<HTMLDivElement>(html`
144+
<div>
145+
<sp-breadcrumbs dir="rtl">
146+
<sp-breadcrumb-item value="a">A</sp-breadcrumb-item>
147+
<sp-breadcrumb-item value="b">B</sp-breadcrumb-item>
148+
</sp-breadcrumbs>
149+
<sp-breadcrumbs>
150+
<sp-breadcrumb-item value="c">C</sp-breadcrumb-item>
151+
<sp-breadcrumb-item value="d">D</sp-breadcrumb-item>
152+
</sp-breadcrumbs>
153+
</div>
154+
`);
155+
156+
await elementUpdated(el);
157+
158+
const rtlItem = el.querySelector(
159+
'sp-breadcrumbs[dir="rtl"] sp-breadcrumb-item[value="a"]'
160+
) as BreadcrumbItem;
161+
await elementUpdated(rtlItem);
162+
163+
const rtlSeparator = rtlItem.shadowRoot.querySelector(
164+
'#separator'
165+
) as HTMLElement;
166+
expect(
167+
getComputedStyle(rtlSeparator).transform,
168+
'separator mirrors under a dir="rtl" ancestor'
169+
).to.not.equal('none');
170+
171+
const defaultItem = el.querySelector(
172+
'sp-breadcrumbs:not([dir]) sp-breadcrumb-item[value="c"]'
173+
) as BreadcrumbItem;
174+
await elementUpdated(defaultItem);
175+
176+
const defaultSeparator = defaultItem.shadowRoot.querySelector(
177+
'#separator'
178+
) as HTMLElement;
179+
expect(
180+
getComputedStyle(defaultSeparator).transform,
181+
'separator matches the document default (ltr) absent an ancestor dir'
182+
).to.equal('none');
183+
});
184+
185+
it('does not mirror the separator for a single item whose own dir mismatches the ambient direction', async () => {
186+
const el = await fixture<HTMLElement>(html`
187+
<sp-breadcrumbs>
188+
<sp-breadcrumb-item value="he" lang="he" dir="rtl">
189+
עברית
190+
</sp-breadcrumb-item>
191+
<sp-breadcrumb-item value="en">English</sp-breadcrumb-item>
192+
</sp-breadcrumbs>
193+
`);
194+
195+
await elementUpdated(el);
196+
197+
const mismatchedItem = el.querySelector(
198+
'sp-breadcrumb-item[value="he"]'
199+
) as BreadcrumbItem;
200+
await elementUpdated(mismatchedItem);
201+
202+
// `:dir()` resolves via the attribute chain, so absent the explicit
203+
// `dir` set on `#separator` in `renderSeparator()` it would incorrectly
204+
// pick up this host's own `dir="rtl"` (meant only for `#item-link`).
205+
const separator = mismatchedItem.shadowRoot.querySelector(
206+
'#separator'
207+
) as HTMLElement;
208+
expect(separator.getAttribute('dir'), 'separator dir attribute').to.equal(
209+
'ltr'
210+
);
211+
expect(
212+
getComputedStyle(separator).transform,
213+
'separator stays unmirrored, matching its ltr siblings'
214+
).to.equal('none');
215+
});
216+
217+
it('updates the separator when an ancestor dir changes after the item has mounted', async () => {
218+
const el = await fixture<HTMLElement>(html`
219+
<sp-breadcrumbs>
220+
<sp-breadcrumb-item value="a">A</sp-breadcrumb-item>
221+
<sp-breadcrumb-item value="b">B</sp-breadcrumb-item>
222+
</sp-breadcrumbs>
223+
`);
224+
225+
await elementUpdated(el);
226+
227+
const item = el.querySelector(
228+
'sp-breadcrumb-item[value="a"]'
229+
) as BreadcrumbItem;
230+
await elementUpdated(item);
231+
232+
const separator = item.shadowRoot.querySelector(
233+
'#separator'
234+
) as HTMLElement;
235+
expect(
236+
getComputedStyle(separator).transform,
237+
'unmirrored before the ancestor sets dir="rtl"'
238+
).to.equal('none');
239+
240+
el.setAttribute('dir', 'rtl');
241+
await elementUpdated(item);
242+
// The ancestor MutationObserver triggers `requestUpdate()`, which is
243+
// async relative to the attribute mutation itself.
244+
await item.updateComplete;
245+
246+
expect(
247+
getComputedStyle(separator).transform,
248+
'mirrored once the ancestor dir changes after mount'
249+
).to.not.equal('none');
250+
});
251+
252+
it("updates #item-link when this item's own lang/dir change after mount", async () => {
253+
const el = await fixture<BreadcrumbItem>(html`
254+
<sp-breadcrumb-item value="home">Home</sp-breadcrumb-item>
255+
`);
256+
257+
await elementUpdated(el);
258+
259+
const itemLink = el.shadowRoot.querySelector('#item-link') as HTMLElement;
260+
expect(itemLink.lang, 'no lang before it is set').to.equal('');
261+
expect(itemLink.dir, 'no dir before it is set').to.equal('');
262+
263+
el.setAttribute('lang', 'he');
264+
el.setAttribute('dir', 'rtl');
265+
await el.updateComplete;
266+
267+
expect(itemLink.lang, 'lang updates after mount').to.equal('he');
268+
expect(itemLink.dir, 'dir updates after mount').to.equal('rtl');
269+
270+
el.removeAttribute('lang');
271+
el.removeAttribute('dir');
272+
await el.updateComplete;
273+
274+
expect(itemLink.lang, 'lang clears after being removed').to.equal('');
275+
expect(itemLink.dir, 'dir clears after being removed').to.equal('');
276+
});
106277
});

0 commit comments

Comments
 (0)