fix(Container): stop nested Container inheriting layout custom properties#1067
Merged
Conversation
…ties The CSS Modules migration forwards optional layout props (fillHeight, grow, shrink, minHeight, maxHeight, overflow) to the DOM as CSS custom properties, emitted inline only when the prop is set. Custom properties inherit by default, so a nested Container that omits those props inherited its ancestor's values — e.g. a layout root with `fillHeight` forced `height: 100%` onto every descendant Container (observed as the control-plane alert banner stretching to fill the viewport, with knock-on spacing breakage throughout the page). Reset these custom properties to `initial` on the base `.container` rule so a declaration on the element wins over inheritance, and add `var()` fallbacks matching the styled-components defaults. Verified in a browser: with the bug a child Container inherited flex-grow:1/height:198px/overflow:hidden; after the fix it hugs its content (flex-grow:0, height:34px, overflow:visible). Adds a NestedInheritance story and a computed-style regression test; all 30 existing Container visual snapshots are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e10c28c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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
There was a problem hiding this comment.
Pull request overview
Fixes a regression introduced by the Container CSS Modules migration where nested <Container> instances unintentionally inherited layout-affecting CSS custom properties from ancestors, leading to incorrect stretching/flex behavior.
Changes:
- Reset inherited
--container-*layout custom properties on the base.containerrule and addvar()fallbacks to preserve styled-components-era defaults when props are unset. - Add a Storybook regression story and a Playwright computed-style test to ensure nested Containers don’t inherit parent
fillHeight/grow/overflow. - Add a patch changeset (and update the package version in
package.json).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/Container/Container.module.css |
Resets layout-related custom properties to prevent inheritance and adds defaults via var() fallbacks. |
src/components/Container/Container.stories.tsx |
Adds NestedInheritance story to reproduce/guard against the regression. |
tests/display/container.spec.ts |
Adds computed-style assertions to prevent regression without relying on screenshots. |
.changeset/fix-container-custom-prop-inheritance.md |
Records a patch-level change for the fix. |
package.json |
Updates package version (currently formatted in a way that may break release automation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ules migrations The CSS Modules migration skill covered emitting layout props as inline CSS custom properties (and memoizing them) but not that custom properties inherit by default. Conditionally-emitted ones therefore leak into nested instances of the same component — the Container regression (#1059, fixed #1067) where a parent with `fillHeight` stretched every descendant Container to full height. Add the reset-to-`initial`-with-fallback rule to the .module.css guidance, a matching pitfall entry, and a checklist gate, and note that visual-regression snapshots miss this (it only shows when a component nests inside itself), so a nested-instance computed-style test is required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Storybook Preview Deployed✅ Preview URL: https://click-5ky9vr42e-clickhouse.vercel.app Built from commit: |
ariser
approved these changes
Jun 8, 2026
Collaborator
|
oh. Auto merge. |
hoorayimhelping
approved these changes
Jun 8, 2026
Contributor
Author
|
Sorry about the auto-merge, y'all. @ariser and @hoorayimhelping i'll submit a quick follow-up PR to address your feedback. |
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.
Problem
After the Container CSS Modules migration (#1059), nested
<Container>s started inheriting layout props they never set. In control-plane this manifested asalert-banner-containerstretching to fill the entire viewport, with cascading spacing breakage across the page.Root cause
The migrated Container forwards optional layout props (
fillHeight,grow,shrink,minHeight,maxHeight,overflow) to the DOM as CSS custom properties, emitted inline only when the prop is set. CSS custom properties inherit by default, so a descendant Container that omits those props inherits its ancestor's values:fillHeightsets--container-height: 100%, which cascades into every nested Container →height: 100%everywhere.--container-grow(children flex-grow),--container-overflow, etc.The styled-components version scoped each component's styles, so nothing leaked.
Fix
Reset these custom properties to the guaranteed-invalid
initialvalue on the base.containerrule (a declaration on the element wins over inheritance), and addvar()fallbacks matching the styled-components defaults.Verification
Reproduced in a real browser against the storybook build:
flex-growheight(parent 200px)overflow1(inherited)198px(fills parent)hidden(inherited)034px(hugs content)visibleNestedInheritancestory + computed-style regression test (tests/display/container.spec.ts).🤖 Generated with Claude Code
Note
Medium Risk
Touches a widely used layout primitive’s flex/height/overflow behavior; fix is targeted but nested layouts in consumers could change visibly where the bug had masked correct sizing.
Overview
Fixes a post–CSS Modules regression where nested
Containercomponents inherited optional layout custom properties (fillHeight,grow,shrink,minHeight,maxHeight,overflow) from ancestors, causing children to stretch or pick up flex/overflow they never set.Container.module.cssresets those six variables toinitialon.containerand addsvar()fallbacks (auto,visible,0 1 auto, etc.) so each instance keeps styled-components–equivalent defaults when props are omitted.Tests & docs: Adds
NestedInheritanceStorybook story and a Playwright computed-style test (not pixel snapshots). Updates the CSS Modules migration skill with the reset pattern, checklist item, and pitfall note. Patch changeset and version bump tov0.6.1-rc1.Reviewed by Cursor Bugbot for commit e10c28c. Bugbot is set up for automated code reviews on this repo. Configure here.