refactor(core): lean out SpectrumElement by caseyisonit · Pull Request #6523 · adobe/spectrum-web-components

caseyisonit · 2026-07-16T23:16:11Z

Description

Trims SpectrumElement (2nd-gen @adobe/spectrum-wc-core) down to only what every component actually needs, and moves an unused helper out to a utility.

Extract hasVisibleFocusInTree → isFocusVisibleInTree(root) util. It has zero 2nd-gen component callers and is a one-liner over the existing getActiveElement util. Now exported from @adobe/spectrum-wc-core/utils with a unit test. This follows the project rule that a helper used by two or fewer components belongs in utils.
Remove the get dir() override. It returned the computed direction and was load-bearing in 1st-gen, but 2nd-gen deliberately resolves direction via getComputedStyle(...).direction at each call site (Tabs, Tooltip, focusgroup-navigation and placement controllers). No 2nd-gen component, controller, test, or Storybook addon reads element.dir.
Remove SpectrumMixin / SpectrumInterface. Once the focus helper moved out, the mixin's only remaining job was re-typing shadowRoot as non-null, which nothing relied on (every read already uses ?.). It was applied exactly once, to LitElement, by SpectrumElement itself. SpectrumElement now extends LitElement directly and shadowRoot reverts to ShadowRoot | null.
Storybook: drop the stale dir / hasVisibleFocusInTree entries from the argTypes hide-list.

Why this is low risk

No internal consumers of anything removed. Repo-wide search: hasVisibleFocusInTree had 0 2nd-gen callers; .dir had 0 JS reads in 2nd-gen source; SpectrumMixin/SpectrumInterface were imported by no component and re-exported nowhere; no code reads this.shadowRoot without optional chaining.
The extracted util is a behavior-preserving copy of the removed method body, plus a focused unit test.
get dir() removal aligns with existing 2nd-gen convention, which already prefers per-site getComputedStyle(...).direction (with an in-code comment justifying not walking dir).
The only breaking surface is the removal of the SpectrumMixin / SpectrumInterface exports from @adobe/spectrum-wc-core. No known external consumer uses them; cheap to remove pre-1.0 (0.3.0) and captured in the changeset as a minor bump.

Motivation and context

SpectrumElement is the root of 34 2nd-gen component bases. Keeping it minimal reduces surface area, removes 1st-gen carryovers that 2nd-gen has already replaced, and keeps shared helpers where the project conventions expect them.

Related issue(s)

Author's checklist

I have read the CONTRIBUTING and PULL_REQUESTS documents.
I have reviewed the Accessibility Practices for this feature.
I have added automated tests to cover my changes.
I have included a well-written changeset if my change needs to be published.
I have included updated documentation if my change required it.

Manual review test cases

Build and lint pass
1. From repo root, run yarn build
2. Run yarn lint
3. Expect both to succeed with no reference to removed members
Focus utility behaves as before
1. Run the core utils test suite
2. Exercise isFocusVisibleInTree
3. Expect false when nothing is focused, and a value matching :focus-visible for the active element
Storybook renders cleanly
1. Start Storybook
2. Open any component's Docs/API table
3. Expect no dir or hasVisibleFocusInTree rows and no console errors

Device review

Did it pass in Desktop?
Did it pass in (emulated) Mobile?
Did it pass in (emulated) iPad?

Accessibility testing checklist

Keyboard
1. Open any interactive component story (e.g. Button, Tabs)
2. Tab through and operate it with the keyboard
3. Expect focus order, focus ring visibility, and activation to be unchanged from main
Screen reader
1. Open the same stories with VoiceOver (or NVDA)
2. Navigate the components
3. Expect roles, names, and state announcements to be unchanged from main

Trim SpectrumElement to only what every 2nd-gen component needs: - Extract `hasVisibleFocusInTree` into the `isFocusVisibleInTree(root)` utility (mirrors the existing `getActiveElement` util it wraps); add a unit test and export from `@adobe/spectrum-wc-core/utils`. No 2nd-gen component consumed the method. - Remove the `get dir()` override. 2nd-gen resolves direction via `getComputedStyle(...).direction` at each call site; nothing read `element.dir`. - Remove the now-empty `SpectrumMixin` / `SpectrumInterface`. Their only remaining job was a non-null `shadowRoot` retype nothing relied on (all reads use `?.`). `SpectrumElement` now extends `LitElement` directly. - Drop the stale `hasVisibleFocusInTree` / `dir` entries from the Storybook argTypes hide-list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

changeset-bot · 2026-07-16T23:16:15Z

🦋 Changeset detected

Latest commit: 79bc21d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@adobe/spectrum-wc-core	Minor
@adobe/spectrum-wc	Minor

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

rubencarvalho · 2026-07-17T15:38:33Z

-  /**
-   * @internal
-   */
-  public override get dir(): CSSStyleDeclaration['direction'] {


I'm ok with removing this... but it made me think... which should be the source of truth for directionality in our components? probably worth creating a helper that is reused everywhere we access it. I opened a Slack thread in our team channel!

Thanks — leaving the removal in. Today 2nd-gen already resolves directionality via getComputedStyle(...).direction at each call site (Tabs, Tooltip, focusgroup + placement controllers), so nothing reads element.dir. Happy to consolidate that into one shared helper as a follow-up from your Slack thread; leaving this thread open for that discussion.

Rajdeepc

This drifts from the focus-management documentation. Please update the documentation to reflect the util-based API. The test quality gap is blocking. Once you have figured these let me know will do a final pass.
Rest seems good.

…d API Address review feedback on the SpectrumElement cleanup: - Rewrite the util test to actually exercise every branch: keyboard focus via `userEvent.tab()` proves the true (`:focus-visible`) branch; programmatic focus on a button proves the "active but not focus-visible" false branch; and an empty attached shadow root proves the `root` argument scopes the query. Drops the meaningless detached-shadow-root check and the redundant fixture null-guard. - Update the focus-management contributor guide to document the `isFocusVisibleInTree(root)` utility instead of the removed `SpectrumElement.hasVisibleFocusInTree()` method; regenerate nav. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

caseyisonit · 2026-07-20T17:30:22Z

Pushed f7ae880 addressing the review:

Test quality (@rubencarvalho, @Rajdeepc) — rewrote is-focus-visible-in-tree.test.ts so every branch is exercised deterministically:

true branch via userEvent.tab() (real keyboard :focus-visible), not programmatic focus;
active-but-not-visible false branch via programmatic focus on a <button> under pointer modality;
root scoping via an empty attached shadow root (the old detached root could never have an active element);
dropped the redundant fixture null-guard and made the steps self-contained for the chromium/firefox/webkit runs.

Docs (@Rajdeepc) — updated CONTRIBUTOR-DOCS/01_contributor-guides/14_focus-management.md to document the isFocusVisibleInTree(root) utility instead of the removed SpectrumElement.hasVisibleFocusInTree() method, and regenerated nav.

Directionality (@rubencarvalho) — left the get dir() removal in; kept that thread open for the shared-helper discussion from Slack.

Note: the three red checks on the previous commit were transient infra failures, not this PR — Lint hit reviewdog: 503 from the GitHub API, Build/Deploy hit an HttpError: <!DOCTYPE html> (GH API 5xx), and Coverage failed on a Coveralls internal server error after all 1922 tests passed. The new push should re-run them clean.

coveralls · 2026-07-20T17:32:10Z

Coverage Report for CI Build 30121125834

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 96.243%

Details

Coverage remained the same as the base build.
Patch coverage: No coverable lines changed in this PR.
No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	39208
Covered Lines:	37935
Line Coverage:	96.75%
Relevant Branches:	6469
Covered Branches:	6026
Branch Coverage:	93.15%
Branches in Coverage %:	Yes
Coverage Strength:	460.63 hits per line

💛 - Coveralls

…leInTree Chromium and WebKit paint :focus-visible on a programmatic .focus() when no mouse-down preceded it, so the "active but not :focus-visible" case was not deterministic. Remove it; the nothing-focused (false) and keyboard-focus (true) cases already exercise both branches of `active?.matches(':focus-visible') ?? false`, and the attached-shadow-root scoping case remains. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

github-actions · 2026-07-20T17:41:36Z

📚 Branch Preview Links

🔍 Gen1 Visual Regression Test Results

When 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: pr-6523

If the changes are expected, update the current_golden_images_cache hash in the circleci config to accept the new images. Instructions are included in that file.
If the changes are unexpected, you can investigate the cause of the differences and update the code accordingly.

caseyisonit · 2026-07-20T19:12:55Z

@Rajdeepc the two blocking items are resolved and CI is green:

Docs: CONTRIBUTOR-DOCS/01_contributor-guides/14_focus-management.md now documents the isFocusVisibleInTree(root) utility instead of the removed SpectrumElement.hasVisibleFocusInTree() method (nav regenerated).
Test quality: is-focus-visible-in-tree.test.ts rewritten to exercise the true branch via real keyboard focus (userEvent.tab()), the false branch via the unfocused/empty-root cases, and root scoping via an attached shadow root. Passing on chromium, firefox, and webkit.

The directionality (get dir()) discussion is tracked as a separate follow-up per the team Slack thread. Ready for your final pass when you have a moment.

cdransf

Looks rad! Just one tiny docs nit ✨

The strategy RFC still described hasVisibleFocusInTree() as a SpectrumMixin method. Update its references (§2, §4.5, §5, §7, §8, Appendix A.7, and the TOC) to reflect that it was extracted to the isFocusVisibleInTree() utility and that SpectrumMixin was removed (SpectrumElement now extends LitElement directly).

miwha-adobe

Very nice! lgtm

caseyisonit requested a review from a team as a code owner July 16, 2026 23:16

caseyisonit added the Status:Ready for review PR ready for review or re-review. label Jul 16, 2026

rubencarvalho reviewed Jul 17, 2026

View reviewed changes

Rajdeepc reviewed Jul 20, 2026

View reviewed changes

caseyisonit and others added 2 commits July 20, 2026 11:19

Merge branch 'main' into caseyisonit/spectrum-element-cleanup-b04490

d4b4aad

Merge branch 'main' into caseyisonit/spectrum-element-cleanup-b04490

0d50f9e

cdransf approved these changes Jul 23, 2026

View reviewed changes

Comment thread 2nd-gen/packages/core/utils/is-focus-visible-in-tree.ts

caseyisonit and others added 2 commits July 23, 2026 11:34

Merge branch 'main' into caseyisonit/spectrum-element-cleanup-b04490

879323f

caseyisonit commented Jul 23, 2026

View reviewed changes

Comment thread .changeset/spectrum-element-lean-cleanup.md Outdated

Apply suggestion from @caseyisonit

5159cd7

caseyisonit enabled auto-merge (squash) July 23, 2026 21:56

miwha-adobe approved these changes Jul 24, 2026

View reviewed changes

caseyisonit added 2 commits July 24, 2026 11:13

Merge branch 'main' into caseyisonit/spectrum-element-cleanup-b04490

09641bf

Merge branch 'main' into caseyisonit/spectrum-element-cleanup-b04490

79bc21d

caseyisonit added the run_vrt Triggers the Chromatic VRT run for 2nd-gen label Jul 24, 2026

caseyisonit merged commit 86f40a1 into main Jul 24, 2026
35 of 36 checks passed

caseyisonit deleted the caseyisonit/spectrum-element-cleanup-b04490 branch July 24, 2026 19:42

Uh oh!

Conversation

caseyisonit commented Jul 16, 2026

Description

Why this is low risk

Motivation and context

Related issue(s)

Author's checklist

Manual review test cases

Device review

Accessibility testing checklist

Uh oh!

changeset-bot Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rubencarvalho Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

caseyisonit Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rajdeepc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caseyisonit commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 30121125834

Coverage remained the same at 96.243%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

github-actions Bot commented Jul 20, 2026

📚 Branch Preview Links

Uh oh!

caseyisonit commented Jul 20, 2026

Uh oh!

cdransf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miwha-adobe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

changeset-bot Bot commented Jul 16, 2026 •

edited

Loading

caseyisonit Jul 20, 2026 •

edited

Loading

Rajdeepc left a comment •

edited

Loading

caseyisonit commented Jul 20, 2026 •

edited

Loading

coveralls commented Jul 20, 2026 •

edited

Loading