refactor(core): lean out SpectrumElement#6523
Open
caseyisonit wants to merge 1 commit into
Open
Conversation
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 detectedLatest commit: 057ec04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
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
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.hasVisibleFocusInTree→isFocusVisibleInTree(root)util. It has zero 2nd-gen component callers and is a one-liner over the existinggetActiveElementutil. Now exported from@adobe/spectrum-wc-core/utilswith a unit test. This follows the project rule that a helper used by two or fewer components belongs in utils.get dir()override. It returned the computed direction and was load-bearing in 1st-gen, but 2nd-gen deliberately resolves direction viagetComputedStyle(...).directionat each call site (Tabs, Tooltip, focusgroup-navigation and placement controllers). No 2nd-gen component, controller, test, or Storybook addon readselement.dir.SpectrumMixin/SpectrumInterface. Once the focus helper moved out, the mixin's only remaining job was re-typingshadowRootas non-null, which nothing relied on (every read already uses?.). It was applied exactly once, toLitElement, bySpectrumElementitself.SpectrumElementnowextends LitElementdirectly andshadowRootreverts toShadowRoot | null.dir/hasVisibleFocusInTreeentries from the argTypes hide-list.Why this is low risk
hasVisibleFocusInTreehad 0 2nd-gen callers;.dirhad 0 JS reads in 2nd-gen source;SpectrumMixin/SpectrumInterfacewere imported by no component and re-exported nowhere; no code readsthis.shadowRootwithout optional chaining.get dir()removal aligns with existing 2nd-gen convention, which already prefers per-sitegetComputedStyle(...).direction(with an in-code comment justifying not walkingdir).SpectrumMixin/SpectrumInterfaceexports 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 aminorbump.Motivation and context
SpectrumElementis 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
Manual review test cases
Build and lint pass
yarn buildyarn lintFocus utility behaves as before
isFocusVisibleInTreefalsewhen nothing is focused, and a value matching:focus-visiblefor the active elementStorybook renders cleanly
dirorhasVisibleFocusInTreerows and no console errorsDevice review
Accessibility testing checklist
Keyboard
mainScreen reader
main