Skip to content

chore(SlotAttributePropagationController): add tests + docs#6490

Open
cdransf wants to merge 6 commits into
mainfrom
cdransf/slot-attribute-propagation-controller-tests-docs
Open

chore(SlotAttributePropagationController): add tests + docs#6490
cdransf wants to merge 6 commits into
mainfrom
cdransf/slot-attribute-propagation-controller-tests-docs

Conversation

@cdransf

@cdransf cdransf commented Jul 9, 2026

Copy link
Copy Markdown
Member

Description

Adds a getValue/attribute-removal capability to SlotAttributePropagationController, plus unit tests and a Storybook controller docs page. The controller is already used by ButtonGroupBase (default slot) and IllustratedMessageBase (named actions slot), but had no dedicated test coverage or documentation of its own, and could not represent an attribute that is only sometimes present on the host.

Changes:

  • API change / fix (core/controllers/slot-attribute-propagation-controller/src/slot-attribute-propagation-controller.ts): getValue may now return string | null. Returning null removes the propagated attribute from assigned elements instead of setting it to an empty string. Also fixes a bug where _previousValue (the hostUpdated() no-op guard) was set unconditionally before the slot was resolved; if the slot wasn't resolvable yet, that value was silently marked as "applied" and a later hostUpdated() call with the same value would never re-propagate once the slot did resolve. _previousValue is now only set inside _propagateToSlot(), after a successful slot resolution.
  • Tests (core/controllers/slot-attribute-propagation-controller/test/slot-attribute-propagation-controller.test.ts): Storybook play-function tests covering first-render propagation, hostUpdated() re-propagation on value change, the previousValue no-op guard (including the slot-not-yet-resolved case), propagate() via slotchange, the hostDisconnected() reset on reconnect, slotName targeting a named slot (leaving the default slot untouched), the selector option filtering assigned elements, and the new nullable getValue / attribute-removal behavior.
  • Storybook docs (core/controllers/slot-attribute-propagation-controller/slot-attribute-propagation-controller.mdx + stories/): a full controller docs page (Usage, Options, Behaviors, Accessibility, hand-authored API section) plus demo hosts mirroring the two real production consumers, the selector option, and the nullable/optional-attribute case.
  • Public export: SlotAttributePropagationController is now exported from the core package's controllers barrel and package.json subpath export (@spectrum-web-components/core/controllers/slot-attribute-propagation-controller.js), matching the flat-file controller convention already used by hover-controller.js, color-controller.js, and focus-group-navigation-controller.js.
  • Build fix (core/vite.config.js): getEntries() now also scans controllers/*/index.ts (via scanSubdirIndexes, the same helper already used for components/), not just flat files directly in controllers/. Without this, Rollup's preserveModules output could inline a controller's re-export barrel into its parent chunk and drop the standalone file, silently breaking that controller's package.json subpath export. This was caught while adding the new export above: it turned out to already be broken (pre-existing, unnoticed) for live-selection-controller too.
  • Rename: the controller's folder and files were renamed from slot-attribute-propagation to slot-attribute-propagation-controller for consistency with sibling controller directories; all internal imports (ButtonGroup.base.ts, IllustratedMessage.base.ts) were updated accordingly.

Motivation and context

SWC-2311: the controller shipped as part of the illustrated-message actions slot work with no dedicated tests or docs, as a known follow-up, and could only propagate string values. A second production consumer (ButtonGroupBase) has since adopted it, and a future DropzoneBase migration is expected to need to propagate an attribute that isn't always present on the host, which the original getValue: () => string signature couldn't express.

Related issue(s)

  • fixes SWC-2311

Screenshots (if appropriate)

N/A — no visual/UI changes. New Storybook docs page: Controllers → Slot attribute propagation controller.

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. (Reviewer is adding this as a patch — fix + new public export.)
  • I have included updated documentation if my change required it.

Reviewer's checklist

  • Includes a Github Issue with appropriate flag or Jira ticket number without a link
  • Includes thoughtfully written changeset if changes suggested include patch, minor, or major features
  • Automated tests cover all use cases and follow best practices for writing
  • Validated on all supported browsers
  • All VRTs are approved before the author can update Golden Hash

Manual review test cases

  • Controller docs page renders correctly

    1. Go to Storybook → Controllers → Slot attribute propagation controller
    2. Confirm the Overview, Options (Default slot / Named slot / Selector filtering / Optional attribute), Behaviors, Accessibility, and API sections all render with no console errors
    3. Expect: each <Canvas> shows the described propagation behavior (e.g. "Named slot" shows the actions-slot button receiving size, while the default-slot paragraph does not; the optional-attribute demo shows the attribute being removed, not set to "", when getValue returns null)
  • New controller tests pass

    1. Run yarn workspace @spectrum-web-components/2nd-gen-swc vitest --run --project storybook -- slot-attribute-propagation-controller
    2. Expect: all tests in slot-attribute-propagation-controller.test.ts pass
  • No regression in existing consumers

    1. Run the same command scoped to button-group and illustrated-message
    2. Expect: all existing size-propagation tests in both components still pass unchanged
  • New public export resolves

    1. From 2nd-gen/packages/core, run yarn build
    2. Confirm dist/controllers/slot-attribute-propagation-controller/index.js (and the hover-controller / focus-group-navigation-controller / live-selection-controller equivalents) exist and export their controller class

Device review

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

Accessibility testing checklist

  • Keyboard (required — document steps below)

    1. Go to Storybook → Controllers → Slot attribute propagation controller → Options → "Named slot"
    2. Tab through the rendered content: the default-slot paragraph (non-interactive) and the actions-slot button
    3. Expect: normal Tab order through the native <button> element; the propagated size attribute is a plain DOM attribute and does not affect focusability, tab order, or introduce any new tab stops
  • Screen reader (required — document steps below)

    1. Go to Storybook → Controllers → Slot attribute propagation controller → Options → "Selector filtering"
    2. With VoiceOver (or NVDA) running, navigate to both buttons
    3. Expect: both are announced as "button" with their visible text label; the propagated variant attribute carries no ARIA semantics, so no additional or altered announcements occur. The controller itself never sets role, aria-*, or any accessibility-relevant attribute — it only calls setAttribute/removeAttribute with whatever attribute name the consuming component configures.

@cdransf cdransf self-assigned this Jul 9, 2026
@cdransf
cdransf requested a review from a team as a code owner July 9, 2026 18:09
@cdransf cdransf added the Spectrum 2 Issues related to Spectrum 2 label Jul 9, 2026
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 16b5923

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 Patch
@adobe/spectrum-wc Patch

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

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

📚 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-6490

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.

@cdransf cdransf added Status:Ready for review PR ready for review or re-review. run_vrt Triggers the Chromatic VRT run for 2nd-gen labels Jul 9, 2026
@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch 2 times, most recently from 007aa71 to 839d90f Compare July 9, 2026 21:17

@rubencarvalho rubencarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really good! Nothing really blocking, but left a couple of thoughts 😄

@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch 2 times, most recently from 7b36e34 to 6e01ccf Compare July 10, 2026 19:10
@cdransf
cdransf requested a review from rubencarvalho July 10, 2026 20:11
@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch from b109cca to c25c806 Compare July 13, 2026 19:14

@rubencarvalho rubencarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 it!! nice :)

@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch 6 times, most recently from ca1c45c to 86825ae Compare July 15, 2026 17:19
Comment thread 2nd-gen/packages/swc/.storybook/blocks/GettingStarted.tsx Outdated
Comment thread 2nd-gen/packages/swc/.storybook/blocks/DocsHeader.tsx
@cdransf
cdransf requested a review from caseyisonit July 15, 2026 18:22
@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch from c576670 to 55cbb91 Compare July 15, 2026 18:36

@Rajdeepc Rajdeepc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdransf I will be adding a changeset in this PR as I can see this is fix we are shipping and a new public export also a bug fix. This should be a minimum patch. Also you PR description is not matching the code.
No other concerns.

@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch from 55cbb91 to bfb3aa3 Compare July 16, 2026 15:04
@cdransf

cdransf commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

@cdransf I will be adding a changeset in this PR as I can see this is fix we are shipping and a new public export also a bug fix. This should be a minimum patch. Also you PR description is not matching the code. No other concerns.

Updated the description and added a changeset. ✨

@cdransf
cdransf requested a review from Rajdeepc July 16, 2026 15:26
@coveralls

coveralls commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 29535143231

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

Coverage Status
Relevant Lines: 39208
Covered Lines: 37935
Line Coverage: 96.75%
Relevant Branches: 6466
Covered Branches: 6023
Branch Coverage: 93.15%
Branches in Coverage %: Yes
Coverage Strength: 461.02 hits per line

💛 - Coveralls

@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch 3 times, most recently from d6bfc28 to 71edd42 Compare July 16, 2026 21:12
@cdransf
cdransf force-pushed the cdransf/slot-attribute-propagation-controller-tests-docs branch from 71edd42 to 16b5923 Compare July 16, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run_vrt Triggers the Chromatic VRT run for 2nd-gen Spectrum 2 Issues related to Spectrum 2 Status:Ready for review PR ready for review or re-review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants