fix(combobox, breadcrumbs): propagate lang/dir for a single item's language without breaking layout#6496
Open
majornista wants to merge 3 commits into
Open
fix(combobox, breadcrumbs): propagate lang/dir for a single item's language without breaking layout#6496majornista wants to merge 3 commits into
majornista wants to merge 3 commits into
Conversation
majornista
added
bug
🦋 Changeset detectedLatest commit: e4a0924 The changes in this PR will be included in the next version bump. This PR includes changesets to release 83 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📚 Branch Preview Links🔍 Gen1 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 |
Collaborator
Coverage Report for CI Build 29130699021Coverage increased (+0.008%) to 96.265%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
This PR adds “language of parts” support to 1st-gen sp-combobox and sp-breadcrumbs by propagating per-item lang/dir into internally-rendered menu items, and by scoping sp-breadcrumb-item’s lang/dir so a single item’s language doesn’t flip breadcrumb layout/separators.
Changes:
- Combobox: propagate option
lang/dirinto rendered popover items and sync the input’slangto the committed selection. - Breadcrumbs: propagate item
lang/dirinto overflow menu items; adjustsp-breadcrumb-itemso separators mirror based on ambient direction rather than the item’s owndir. - Add targeted tests and Storybook stories demonstrating mixed
lang/dirlists, plus update 1st-gen version typings.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| 1st-gen/tools/base/src/version.d.ts | Updates declared versions to match implementation. |
| 1st-gen/packages/combobox/test/combobox-a11y.test.ts | Adds coverage for lang/dir propagation and input lang syncing. |
| 1st-gen/packages/combobox/stories/combobox.stories.ts | Adds languageOfParts story demonstrating per-item language/direction. |
| 1st-gen/packages/combobox/src/Combobox.ts | Implements option lang/dir forwarding and input lang binding. |
| 1st-gen/packages/breadcrumbs/test/breadcrumbs.test.ts | Adds overflow menu propagation tests and ancestor dir-change regression test. |
| 1st-gen/packages/breadcrumbs/test/breadcrumb-item.test.ts | Adds tests ensuring per-item dir doesn’t break layout and separators track ambient direction. |
| 1st-gen/packages/breadcrumbs/stories/breadcrumbs.stories.ts | Adds LanguageOfParts story for overflow menu propagation. |
| 1st-gen/packages/breadcrumbs/src/Breadcrumbs.ts | Captures per-item lang/dir and forwards into overflow menu items. |
| 1st-gen/packages/breadcrumbs/src/BreadcrumbItem.ts | Scopes lang/dir onto #item-link and computes separator direction from ambient context with ancestor observation. |
| 1st-gen/packages/breadcrumbs/src/breadcrumb-item.css | Ensures host layout direction inherits ambient direction even when host has dir. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
90ccb0c to
a77171e
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
0377af4 to
3c7d781
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
3c7d781 to
ff9409d
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
c017bce to
0e255f5
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
0e255f5 to
6ed624c
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
f9d6e3c to
9e1bdb7
Compare
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
7e29d47 to
d3afc71
Compare
…nguage 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
majornista
force-pushed
the
mijordan/SWC-2359-combobox-lang-of-parts
branch
from
207a4ec to
6ee4b1a
Compare
| // `SpectrumElement` overrides `dir` to return the *computed* CSS | ||
| // direction rather than the attribute, so read the attribute | ||
| // directly to capture the item's own authored direction override. | ||
| dir: (el.getAttribute('dir') as 'ltr' | 'rtl' | 'auto') || undefined, |
Comment on lines
+157
to
+163
| dir=${ifDefined( | ||
| (this.getAttribute('dir') || undefined) as | ||
| | 'ltr' | ||
| | 'rtl' | ||
| | 'auto' | ||
| | undefined | ||
| )} |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds "language of parts" support to
sp-comboboxandsp-breadcrumbs: consumers can setlang/diron an individual slotted item (e.g. a language name rendered in its own script) and have it render correctly, without breaking the surrounding component's layout.<sp-menu-item lang="he" dir="rtl">(or aComboboxOptionwithlang/dir) now propagates those attributes to its rendered counterpart in the popover. The text input itself also adopts thelangof the currently-selected option, so screen readers pronounce the displayed value correctly.lang/dirnow apply only to#item-link(the text), not the host — so one item's language doesn't flip its own layout or its separator's chevron relative to its siblings. The chevron now tracks the ambient direction (nearest ancestor'sdir, or the document default) instead of the item's owndirattribute, and stays correct if an ancestor'sdirchanges after the item has already mounted, or for the overflow-menu wrapper item (which is rendered insideBreadcrumbs' own shadow root rather than as a light-DOM child).Two non-obvious base-class behaviors caused regressions mid-implementation and are now specifically guarded against (with tests that fail without the fix):
SpectrumElementoverrides the nativedirgetter to return computed CSS direction rather than the attribute, so authoreddirvalues must be read viagetAttribute('dir').:dir()resolves directionality via the DOM'sdirattribute chain, entirely independent of thedirectionproperty — sodirection: inheritalone doesn't stop a mismatched attribute from being picked up by descendant:dir()selectors.Motivation and context
Combobox and Breadcrumbs synthesize new elements from extracted data (rather than reusing the original slotted elements) when rendering their popover/overflow menu, so any
lang/dirset on the original item was silently dropped — breaking pronunciation and bidi text shaping for mixed-language lists (e.g. a language picker).Related issue(s)
Screenshots (if appropriate)
N/A — no visual/layout changes to default (single-language) usage. New Storybook stories (
languageOfParts,LanguageOfParts) demonstrate the fix; a screenshot/VRT baseline update may be worth attaching before merge.Author's checklist
Reviewer's checklist
patch,minor, ormajorfeaturesManual review test cases
Combobox popover items and input keep an item's own language
languageOfParts<sp-menu-item>in the popover to carry the samelang/diras its source; select the Hebrew or Arabic option and expect the input's ownlangattribute to update to matchBreadcrumbs overflow menu keeps an item's own language
LanguageOfParts(forces the overflow menu viamax-visible-items)<sp-menu-item>to carry the samelang/diras the source<sp-breadcrumb-item>A single item's language doesn't break the breadcrumb trail's layout
LanguageOfPartsstory, locate the Hebrew/Arabic breadcrumb items amid the (LTR) traildir="rtl"on<sp-breadcrumbs>via devtools after the story has renderedDevice review
Accessibility testing checklist
Keyboard (required — document steps below)
languageOfPartslang/dirchangesScreen reader (required — document steps below)
languageOfPartsand arrow through the popover optionsLanguageOfParts, opening the "More items" overflow menu