Skip to content

fix(stories): With Layer story props not updating#21716

Closed
sangeethababu9223 wants to merge 8 commits into
carbon-design-system:mainfrom
sangeethababu9223:fix/with-layer-inconsistant-behaviour-stories
Closed

fix(stories): With Layer story props not updating#21716
sangeethababu9223 wants to merge 8 commits into
carbon-design-system:mainfrom
sangeethababu9223:fix/with-layer-inconsistant-behaviour-stories

Conversation

@sangeethababu9223

@sangeethababu9223 sangeethababu9223 commented Mar 5, 2026

Copy link
Copy Markdown
Member

Closes #20095

With Layer stories do not respond to controls beyond the first layer. An initial fix was introduced in #21267. However, that update did not account for nested elements or components with children.

Two solutions are introduced in the PR:

Enhanced Clone-and-Sync Approach

The existing implementation has been updated to sync attributes for child elements in addition to the root component. This solves the problem to some extent, but the sb-template-layers Storybook template component's clone-and-sync approach has a fundamental limitation:

The MutationObserver cannot differentiate between:

  • Storybook control changes (e.g., disabled, invalid, size) - should be synced across layers
  • User interaction changes (e.g., open, value, aria-expanded) - should remain independent per layer

This causes issues with interactive components like multi-select, where clicking the top layer component would inadvertently trigger state changes in the cloned layers below, resulting in focus jumping and unexpected behavior.

Workaround:

const skipAttributes = ['open', 'value', 'aria-expanded', 'aria-activedescendant', 'aria-selected'];

This array prevents syncing of interaction-based attributes, but it is not a proper solution as it requires manual maintenance and is brittle.

Render Function Approach

Introduced a new renderContent property that accepts a render function, enabling a declarative approach:

@property({ attribute: false })
renderContent?: () => TemplateResult;

Usage:

const renderMultiSelect = () => html`
  <cds-multi-select ...props>
    <cds-multi-select-item>...</cds-multi-select-item>
  </cds-multi-select>
`;

return html`
  <sb-template-layers .renderContent=${renderMultiSelect}>
  </sb-template-layers>
`;

Benefits:

This implementation eliminates the challenges associated with attribute synchronization and is much cleaner. While a slot-based approach is generally preferable in Web Components, in this case, the render function approach is more beneficial because the chances of unwanted side effects are significantly lower compared to the existing implementation.

Trade-off:

The only downside is that all Layer stories across components would need to be updated to use this approach. To address this, the original code has been preserved, and renderContent has been added as a new option, allowing both approaches to coexist.

Changelog

New

  • Added renderContent based implementation option in the layer template.
  • Updated Multiselect Layer stories with this new implementation.

Changed

The following changes have been made to packages/web-components/.storybook/templates/with-layer.ts:

  • Updated the MutationObserver to watch for attribute changes within the subtree.
  • Updated the cloned items in layers one and two to reflect these attribute changes with skipAttributes.

Testing / Reviewing

Review the With Layer story for any component that has controls enabled. Changes made through the controls should be reflected across all layers, not just the top layer.

The following components previously had issues in their With Layer stories, and the behavior can be verified there:

  • Combo box
  • Content switcher
  • Dropdown
  • Multi select
  • Text input

Also, verify the Multi select With Layer stories to ensure the new implementation works as expected.

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

  • Reviewed every line of the diff
  • Updated documentation and storybook examples
  • Wrote passing tests that cover this change
  • Addressed any impact on accessibility (a11y)
  • Tested for cross-browser consistency
  • Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

@sangeethababu9223 sangeethababu9223 requested a review from a team as a code owner March 5, 2026 06:40
@netlify

netlify Bot commented Mar 5, 2026

Copy link
Copy Markdown

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 911aa69
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/69a92547f126840008941f07
😎 Deploy Preview https://deploy-preview-21716--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Mar 5, 2026

Copy link
Copy Markdown

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit 911aa69
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/69a92547786c080008fb4645
😎 Deploy Preview https://deploy-preview-21716--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Mar 5, 2026

Copy link
Copy Markdown

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit 0e4fd9a
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/69ae588fa4c911000840c22a
😎 Deploy Preview https://deploy-preview-21716--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify

netlify Bot commented Mar 5, 2026

Copy link
Copy Markdown

Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 0e4fd9a
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/69ae588f0e12700008fb0ae8
😎 Deploy Preview https://deploy-preview-21716--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov

codecov Bot commented Mar 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.70%. Comparing base (ed5633a) to head (0e4fd9a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21716      +/-   ##
==========================================
+ Coverage   87.66%   87.70%   +0.03%     
==========================================
  Files         536      536              
  Lines       43634    43634              
  Branches     6721     6723       +2     
==========================================
+ Hits        38253    38269      +16     
+ Misses       5230     5214      -16     
  Partials      151      151              
Flag Coverage Δ
main-packages 87.90% <ø> (ø)
web-components 87.59% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@devadula-nandan devadula-nandan 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.

Hey @sangeethababu9223,

IMO, this feels a bit over-engineered to me.
Storybook provides callable story() methods that can be used to clone components, which seems like the best fit for this scenario. I’m not a huge fan of attaching mutation observers to the story root for something that’s purely representational.

I have a draft PR using the decorator approach:
#21726

Open to discussion, what do you think ?
is there anything missing from this decorator approach ?

@sangeethababu9223

Copy link
Copy Markdown
Member Author

Hey @sangeethababu9223,

IMO, this feels a bit over-engineered to me. Storybook provides callable story() methods that can be used to clone components, which seems like the best fit for this scenario. I’m not a huge fan of attaching mutation observers to the story root for something that’s purely representational.

I have a draft PR using the decorator approach: #21726

Open to discussion, what do you think ? is there anything missing from this decorator approach ?

Hey @devadula-nandan ,
You're absolutely right in that decorator based approach is the better one. My plan was to replace the mutation based approach with renderContent eventually. Just didn't want to break anything by changing things at once. But this looks much better and since you already made those changes in the stories, you could ahead and put it for review and I'll close this one.

@sangeethababu9223

sangeethababu9223 commented Mar 9, 2026

Copy link
Copy Markdown
Member Author

Closing this PR as a much cleaner implementation is done in #21726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: with-layer stories do not respond to controls beyond first layer

2 participants