Fix: correction for not inheriting styles inside box and stack#247
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes CSS custom property inheritance issues in Box and Stack components where child components would unintentionally inherit parent sizing variables. The fix uses CSS @property declarations with inherits: false and removes the use of 'initial' values in favor of only setting defined values.
Changes:
- Added
@propertydeclarations withinherits: falsefor Box and Stack sizing variables to prevent CSS variable inheritance - Modified
responsiveValueToCssutility to only set defined responsive values instead of defaulting to 'initial' - Updated Box, Stack, and Grid components to stop setting 'initial' values for undefined properties
- Removed conditional spreading pattern in Grid component for consistency
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/responsiveUtil.ts | Removed shouldResetInheritance parameter and logic to set 'initial' values; now only sets defined responsive values |
| src/styles/global.scss | Added @property declarations for Box and Stack variables with inherits: false to prevent inheritance |
| src/particles/box/styles.scss | Removed hardcoded fallback values (none, 0) from responsive breakpoint CSS variable chains |
| src/particles/box/component.tsx | Removed 'initial' value assignments; removed heightResponsive.base and widthResponsive.base fallback handling |
| src/layouts/stack/component.tsx | Removed 'initial' value assignments and shouldResetInheritance usage; added conditional spreading for max/min properties |
| src/layouts/grid/component.tsx | Changed from setting 'initial' to using conditional spreading for responsive size values |
| CHANGELOG.md | Updated changelog entry to mention correction for responsive base size usage in EqualGrid |
| // | ||
| // Using @property with `inherits: false` prevents this cascade, ensuring each | ||
| // component instance only uses the variables it explicitly sets on itself. | ||
| // Browser support: Chrome 85+, Safari 15.4+, Firefox 128+ (universal by 2024) |
There was a problem hiding this comment.
The browser support comment states "universal by 2024" but it's now 2026. While the @Property feature is well-supported in modern browsers (Chrome 85+, Safari 15.4+, Firefox 128+), consider updating the comment to reflect the current year or remove the temporal reference to keep it from becoming outdated. Additionally, note that this creates a hard dependency on these browser versions - older browsers will not prevent CSS variable inheritance, potentially causing layout issues.
| // Browser support: Chrome 85+, Safari 15.4+, Firefox 128+ (universal by 2024) | |
| // Browser support: Chrome 85+, Safari 15.4+, Firefox 128+ (older browsers will ignore @property and allow inheritance) |
| max-width: var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width))); | ||
| max-height: var(--kiba-box-max-height-md, var(--kiba-box-max-height-sm, var(--kiba-box-max-height))); | ||
| min-width: var(--kiba-box-min-width-md, var(--kiba-box-min-width-sm, var(--kiba-box-min-width))); | ||
| min-height: var(--kiba-box-min-height-md, var(--kiba-box-min-height-sm, var(--kiba-box-min-height))); |
There was a problem hiding this comment.
Missing final fallback values in responsive breakpoint CSS. Since the @Property declarations use inherits: false, if a CSS variable is not explicitly set, it will be unset (not inherited). The fallback chains for max-width, max-height, min-width, and min-height need final fallback values (none, 0, etc.) restored at the end of each chain. For example: max-width: var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width, none))) to ensure a valid CSS value when no variables are set.
| @property --kiba-stack-min-width-medium { syntax: "*"; inherits: false; } | ||
| @property --kiba-stack-min-width-large { syntax: "*"; inherits: false; } | ||
| @property --kiba-stack-min-width-extraLarge { syntax: "*"; inherits: false; } | ||
|
|
There was a problem hiding this comment.
Missing @Property declarations for Stack responsive variables. The Stack component uses --kiba-stack-direction-*, --kiba-stack-child-alignment-*, and --kiba-stack-content-alignment-* CSS variables which also suffer from the same inheritance issue that this PR is fixing. These should have @property declarations with inherits: false to prevent unintended cascading from parent Stack components to child Stack components.
| @property --kiba-stack-direction-base { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-direction-small { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-direction-medium { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-direction-large { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-direction-extraLarge { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-child-alignment-base { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-child-alignment-small { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-child-alignment-medium { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-child-alignment-large { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-child-alignment-extraLarge { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-content-alignment-base { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-content-alignment-small { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-content-alignment-medium { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-content-alignment-large { syntax: "*"; inherits: false; } | |
| @property --kiba-stack-content-alignment-extraLarge { syntax: "*"; inherits: false; } |
| @property --kiba-stack-min-width-medium { syntax: "*"; inherits: false; } | ||
| @property --kiba-stack-min-width-large { syntax: "*"; inherits: false; } | ||
| @property --kiba-stack-min-width-extraLarge { syntax: "*"; inherits: false; } | ||
|
|
There was a problem hiding this comment.
Missing @Property declarations for Grid item responsive variables. The Grid component uses --kiba-grid-item-width-* and --kiba-grid-item-display-* CSS variables which also have responsive variants that could inherit from parent Grid items. These should have @property declarations with inherits: false to be consistent with the inheritance fix being applied to Box and Stack components.
| @property --kiba-grid-item-width { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-sm { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-md { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-lg { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-xl { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-base { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-small { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-medium { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-large { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-width-extraLarge { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-sm { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-md { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-lg { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-xl { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-base { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-small { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-medium { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-large { syntax: "*"; inherits: false; } | |
| @property --kiba-grid-item-display-extraLarge { syntax: "*"; inherits: false; } |
| const height = props.height ?? props.heightResponsive?.base ?? (props.isFullHeight ? '100%' : 'auto'); | ||
| const width = props.width ?? props.widthResponsive?.base ?? (isFullWidth ? '100%' : 'auto'); |
There was a problem hiding this comment.
Breaking change: The removal of heightResponsive?.base and widthResponsive?.base fallback handling breaks backward compatibility. Previously, if someone passed widthResponsive={{ base: '50%' }} without setting the width prop, the base value would be used. Now it's ignored, and the code falls back to the default ('100%' or 'auto'). Consider preserving the base value fallback: const height = props.height || props.heightResponsive?.base || (props.isFullHeight ? '100%' : 'auto'); and similarly for width.
| const height = props.height ?? props.heightResponsive?.base ?? (props.isFullHeight ? '100%' : 'auto'); | |
| const width = props.width ?? props.widthResponsive?.base ?? (isFullWidth ? '100%' : 'auto'); | |
| const height = props.height || props.heightResponsive?.base || (props.isFullHeight ? '100%' : 'auto'); | |
| const width = props.width || props.widthResponsive?.base || (isFullWidth ? '100%' : 'auto'); |
| max-width: var(--kiba-box-max-width-sm, var(--kiba-box-max-width)); | ||
| max-height: var(--kiba-box-max-height-sm, var(--kiba-box-max-height)); | ||
| min-width: var(--kiba-box-min-width-sm, var(--kiba-box-min-width)); | ||
| min-height: var(--kiba-box-min-height-sm, var(--kiba-box-min-height)); |
There was a problem hiding this comment.
Missing final fallback values in responsive breakpoint CSS. Since the @Property declarations use inherits: false, if a CSS variable is not explicitly set, it will be unset (not inherited). The fallback chain var(--kiba-box-max-width-sm, var(--kiba-box-max-width)) will fail if neither variable is set. The final fallback values (none, 0, auto) that were removed need to be restored at the end of each fallback chain. For example: max-width: var(--kiba-box-max-width-sm, var(--kiba-box-max-width, none)) to match line 81.
| max-width: var(--kiba-box-max-width-lg, var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width)))); | ||
| max-height: var(--kiba-box-max-height-lg, var(--kiba-box-max-height-md, var(--kiba-box-max-height-sm, var(--kiba-box-max-height)))); | ||
| min-width: var(--kiba-box-min-width-lg, var(--kiba-box-min-width-md, var(--kiba-box-min-width-sm, var(--kiba-box-min-width)))); | ||
| min-height: var(--kiba-box-min-height-lg, var(--kiba-box-min-height-md, var(--kiba-box-min-height-sm, var(--kiba-box-min-height)))); |
There was a problem hiding this comment.
Missing final fallback values in responsive breakpoint CSS. Since the @Property declarations use inherits: false, if a CSS variable is not explicitly set, it will be unset (not inherited). The fallback chains for max-width, max-height, min-width, and min-height need final fallback values (none, 0, etc.) restored at the end of each chain. For example: max-width: var(--kiba-box-max-width-lg, var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width, none)))) to ensure a valid CSS value when no variables are set.
| max-width: var(--kiba-box-max-width-xl, var(--kiba-box-max-width-lg, var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width))))); | ||
| max-height: var(--kiba-box-max-height-xl, var(--kiba-box-max-height-lg, var(--kiba-box-max-height-md, var(--kiba-box-max-height-sm, var(--kiba-box-max-height))))); | ||
| min-width: var(--kiba-box-min-width-xl, var(--kiba-box-min-width-lg, var(--kiba-box-min-width-md, var(--kiba-box-min-width-sm, var(--kiba-box-min-width))))); | ||
| min-height: var(--kiba-box-min-height-xl, var(--kiba-box-min-height-lg, var(--kiba-box-min-height-md, var(--kiba-box-min-height-sm, var(--kiba-box-min-height))))); |
There was a problem hiding this comment.
Missing final fallback values in responsive breakpoint CSS. Since the @Property declarations use inherits: false, if a CSS variable is not explicitly set, it will be unset (not inherited). The fallback chains for max-width, max-height, min-width, and min-height need final fallback values (none, 0, etc.) restored at the end of each chain. For example: max-width: var(--kiba-box-max-width-xl, var(--kiba-box-max-width-lg, var(--kiba-box-max-width-md, var(--kiba-box-max-width-sm, var(--kiba-box-max-width, none))))) to ensure a valid CSS value when no variables are set.
| const height = props.height ?? props.heightResponsive?.base ?? (isFullHeight ? '100%' : 'auto'); | ||
| const width = props.width ?? props.widthResponsive?.base ?? (isFullWidth ? '100%' : 'auto'); |
There was a problem hiding this comment.
Breaking change: The removal of heightResponsive?.base and widthResponsive?.base fallback handling breaks backward compatibility. Previously, if someone passed widthResponsive={{ base: '50%' }} without setting the width prop, the base value would be used. Now it's ignored, and the code falls back to the default ('100%' or 'auto'). Consider preserving the base value fallback: const height = props.height || props.heightResponsive?.base || (isFullHeight ? '100%' : 'auto'); and similarly for width.
| - [MINOR] Correction for using `sizeResponsive.base` size in `EqualGrid` | ||
| - [MINOR] Correction for using `heightResponsive.base`, `widthResponsive.base` size in `EqualGrid`, `Box` and `Stack` | ||
| - [MINOR] Correction for using `childSizeResponsive.base` size in `EqualGrid` | ||
| - [MINOR] Fix `Box` and `Stack` to not inherit sizing props from parent when not intended |
There was a problem hiding this comment.
The CHANGELOG entry doesn't mention the core purpose of this PR - fixing CSS custom property inheritance issues in Box and Stack components. Consider updating to explain that this PR prevents child components from inheriting parent sizing variables by using CSS @Property declarations with inherits: false and removing 'initial' value assignments.
| - [MINOR] Fix `Box` and `Stack` to not inherit sizing props from parent when not intended | |
| - [MINOR] Fix `Box` and `Stack` sizing CSS custom properties so child components no longer inherit parent sizing variables, by using CSS `@property` with `inherits: false` and removing `initial` value assignments |
* main: Fix: correction for not inheriting styles inside box and stack (#247)
Description
Screenshots:
Checklist: