Fix: correction for responsive variables in box and stack#246
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fix how responsive CSS variables are applied (and reset) for Box and Stack, likely to avoid unintentionally inheriting responsive custom properties from parent components.
Changes:
- Added an option to
responsiveValueToCssto “reset inheritance” for missing responsive fields. - Updated
Boxto always provide breakpoint custom properties (currently using'initial') and adjusted responsive sizing defaults in SCSS. - Updated
Stackto always emit responsive breakpoint variables (and base min/max sizing variables) usingresponsiveValueToCss(..., true).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/util/responsiveUtil.ts | Adds shouldResetInheritance behavior and changes missing breakpoint output values. |
| src/particles/box/styles.scss | Updates breakpoint fallbacks for min/max sizing and adds a clarifying comment. |
| src/particles/box/component.tsx | Changes how base and responsive custom properties are emitted (now defaults to 'initial'). |
| src/layouts/stack/component.tsx | Switches responsive CSS var emission to always include breakpoint vars with inheritance reset enabled. |
| if (shouldResetInheritance) { | ||
| return { | ||
| [`${variablePrefix}-small`]: 'initial', | ||
| [`${variablePrefix}-medium`]: 'initial', | ||
| [`${variablePrefix}-large`]: 'initial', | ||
| [`${variablePrefix}-extraLarge`]: 'initial', | ||
| }; | ||
| } | ||
| return {}; | ||
| } | ||
| const result: Record<string, string> = {}; | ||
| if (field.small !== undefined) { | ||
| result[`${variablePrefix}-small`] = converter(field.small); | ||
| } | ||
| if (field.medium !== undefined) { | ||
| result[`${variablePrefix}-medium`] = converter(field.medium); | ||
| } | ||
| if (field.large !== undefined) { | ||
| result[`${variablePrefix}-large`] = converter(field.large); | ||
| } | ||
| if (field.extraLarge !== undefined) { | ||
| result[`${variablePrefix}-extraLarge`] = converter(field.extraLarge); | ||
| } | ||
| result[`${variablePrefix}-small`] = field.small !== undefined ? converter(field.small) : 'initial'; | ||
| result[`${variablePrefix}-medium`] = field.medium !== undefined ? converter(field.medium) : 'initial'; | ||
| result[`${variablePrefix}-large`] = field.large !== undefined ? converter(field.large) : 'initial'; | ||
| result[`${variablePrefix}-extraLarge`] = field.extraLarge !== undefined ? converter(field.extraLarge) : 'initial'; |
There was a problem hiding this comment.
Using the literal string 'initial' as the value for missing responsive custom properties prevents var(--x, fallback) from falling back (because the variable is defined and resolves to the keyword initial). This changes behavior: e.g. consumers will get flex-direction: initial / justify-content: initial / width: initial at breakpoints instead of inheriting the *-base value or the fallback.
If the intent is to both (a) stop inheriting parent custom properties and (b) still trigger the var() fallback, set missing breakpoint variables to a value that becomes invalid at computed-value time (commonly var(--kiba-unset) with no fallback), or avoid emitting the property entirely when inheritance reset isn't needed. With an invalid value, var(--x, fallback) will use the fallback while still overriding inheritance on the element.
| '--kiba-stack-direction-base': getDirectionValue(direction), | ||
| ...responsiveValueToCss(props.directionResponsive, '--kiba-stack-direction', getDirectionValue), | ||
| ...responsiveValueToCss(props.directionResponsive, '--kiba-stack-direction', getDirectionValue, true), | ||
| '--kiba-stack-child-alignment-base': getFlexItemAlignment(childAlignment), | ||
| ...responsiveValueToCss(props.childAlignmentResponsive, '--kiba-stack-child-alignment', getFlexItemAlignment), | ||
| ...responsiveValueToCss(props.childAlignmentResponsive, '--kiba-stack-child-alignment', getFlexItemAlignment, true), | ||
| '--kiba-stack-content-alignment-base': getFlexContentAlignment(contentAlignment), | ||
| ...responsiveValueToCss(props.contentAlignmentResponsive, '--kiba-stack-content-alignment', getFlexContentAlignment), | ||
| ...responsiveValueToCss(props.contentAlignmentResponsive, '--kiba-stack-content-alignment', getFlexContentAlignment, true), | ||
| '--kiba-stack-height-base': height, | ||
| ...responsiveValueToCss(props.heightResponsive, '--kiba-stack-height'), | ||
| ...responsiveValueToCss(props.heightResponsive, '--kiba-stack-height', undefined, true), | ||
| '--kiba-stack-width-base': width, | ||
| ...responsiveValueToCss(props.widthResponsive, '--kiba-stack-width'), | ||
| ...(maxHeightResponsive ? { '--kiba-stack-max-height-base': maxHeightResponsive.base ?? 'none', ...responsiveValueToCss(maxHeightResponsive, '--kiba-stack-max-height') } : {}), | ||
| ...(maxWidthResponsive ? { '--kiba-stack-max-width-base': maxWidthResponsive.base ?? 'none', ...responsiveValueToCss(maxWidthResponsive, '--kiba-stack-max-width') } : {}), | ||
| ...(minHeightResponsive ? { '--kiba-stack-min-height-base': minHeightResponsive.base ?? '0', ...responsiveValueToCss(minHeightResponsive, '--kiba-stack-min-height') } : {}), | ||
| ...(minWidthResponsive ? { '--kiba-stack-min-width-base': minWidthResponsive.base ?? '0', ...responsiveValueToCss(minWidthResponsive, '--kiba-stack-min-width') } : {}), | ||
| ...responsiveValueToCss(props.widthResponsive, '--kiba-stack-width', undefined, true), |
There was a problem hiding this comment.
With shouldResetInheritance set to true, responsiveValueToCss currently emits breakpoint variables with the value 'initial' when the responsive prop is undefined. Because stack/styles.scss uses var(--kiba-stack-*-small, var(--kiba-stack-*-base, ...)), this will force breakpoints to use CSS-wide keyword initial instead of falling back to the *-base variables.
Concrete impact: a default vertical stack will become flex-direction: initial (i.e. row) at screen-small and above, and justify-content: initial (typically flex-start) instead of the configured base value.
This should be switched to a value that makes the custom property invalid (so the var() fallback is taken), or to var(--kiba-stack-*-base) as the default emitted value for missing breakpoints.
| '--kiba-box-width-sm': props.widthResponsive?.small ?? 'initial', | ||
| '--kiba-box-width-md': props.widthResponsive?.medium ?? 'initial', | ||
| '--kiba-box-width-lg': props.widthResponsive?.large ?? 'initial', | ||
| '--kiba-box-width-xl': props.widthResponsive?.extraLarge ?? 'initial', | ||
| '--kiba-box-height': height ?? 'initial', | ||
| '--kiba-box-height-sm': props.heightResponsive?.small ?? 'initial', | ||
| '--kiba-box-height-md': props.heightResponsive?.medium ?? 'initial', | ||
| '--kiba-box-height-lg': props.heightResponsive?.large ?? 'initial', | ||
| '--kiba-box-height-xl': props.heightResponsive?.extraLarge ?? 'initial', | ||
| '--kiba-box-max-width': props.maxWidth ?? 'initial', | ||
| '--kiba-box-max-width-sm': props.maxWidthResponsive?.small ?? 'initial', | ||
| '--kiba-box-max-width-md': props.maxWidthResponsive?.medium ?? 'initial', | ||
| '--kiba-box-max-width-lg': props.maxWidthResponsive?.large ?? 'initial', | ||
| '--kiba-box-max-width-xl': props.maxWidthResponsive?.extraLarge ?? 'initial', | ||
| '--kiba-box-max-height': props.maxHeight ?? 'initial', | ||
| '--kiba-box-max-height-sm': props.maxHeightResponsive?.small ?? 'initial', | ||
| '--kiba-box-max-height-md': props.maxHeightResponsive?.medium ?? 'initial', | ||
| '--kiba-box-max-height-lg': props.maxHeightResponsive?.large ?? 'initial', | ||
| '--kiba-box-max-height-xl': props.maxHeightResponsive?.extraLarge ?? 'initial', | ||
| '--kiba-box-min-width': props.minWidth ?? 'initial', | ||
| '--kiba-box-min-width-sm': props.minWidthResponsive?.small ?? 'initial', | ||
| '--kiba-box-min-width-md': props.minWidthResponsive?.medium ?? 'initial', | ||
| '--kiba-box-min-width-lg': props.minWidthResponsive?.large ?? 'initial', | ||
| '--kiba-box-min-width-xl': props.minWidthResponsive?.extraLarge ?? 'initial', | ||
| '--kiba-box-min-height': props.minHeight ?? 'initial', | ||
| '--kiba-box-min-height-sm': props.minHeightResponsive?.small ?? 'initial', | ||
| '--kiba-box-min-height-md': props.minHeightResponsive?.medium ?? 'initial', | ||
| '--kiba-box-min-height-lg': props.minHeightResponsive?.large ?? 'initial', | ||
| '--kiba-box-min-height-xl': props.minHeightResponsive?.extraLarge ?? 'initial', |
There was a problem hiding this comment.
Setting the responsive breakpoint variables to the literal string 'initial' (e.g. --kiba-box-width-sm: 'initial') breaks the fallback chain in box/styles.scss. At @include screen-small, width: var(--kiba-box-width-sm, var(--kiba-box-width, 100%)) will resolve to width: initial (auto) instead of falling back to the base width (often 100%).
This means a Box with no widthResponsive will unexpectedly stop being full-width at breakpoints.
To both prevent inheriting a parent Box’s responsive vars and preserve fallbacks, use an invalid-at-computed-time sentinel value (e.g. var(--kiba-unset) with no fallback) for the missing breakpoint variables, or only set breakpoint variables when explicitly provided.
| '--kiba-box-width-sm': props.widthResponsive?.small ?? 'initial', | |
| '--kiba-box-width-md': props.widthResponsive?.medium ?? 'initial', | |
| '--kiba-box-width-lg': props.widthResponsive?.large ?? 'initial', | |
| '--kiba-box-width-xl': props.widthResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-height': height ?? 'initial', | |
| '--kiba-box-height-sm': props.heightResponsive?.small ?? 'initial', | |
| '--kiba-box-height-md': props.heightResponsive?.medium ?? 'initial', | |
| '--kiba-box-height-lg': props.heightResponsive?.large ?? 'initial', | |
| '--kiba-box-height-xl': props.heightResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-max-width': props.maxWidth ?? 'initial', | |
| '--kiba-box-max-width-sm': props.maxWidthResponsive?.small ?? 'initial', | |
| '--kiba-box-max-width-md': props.maxWidthResponsive?.medium ?? 'initial', | |
| '--kiba-box-max-width-lg': props.maxWidthResponsive?.large ?? 'initial', | |
| '--kiba-box-max-width-xl': props.maxWidthResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-max-height': props.maxHeight ?? 'initial', | |
| '--kiba-box-max-height-sm': props.maxHeightResponsive?.small ?? 'initial', | |
| '--kiba-box-max-height-md': props.maxHeightResponsive?.medium ?? 'initial', | |
| '--kiba-box-max-height-lg': props.maxHeightResponsive?.large ?? 'initial', | |
| '--kiba-box-max-height-xl': props.maxHeightResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-min-width': props.minWidth ?? 'initial', | |
| '--kiba-box-min-width-sm': props.minWidthResponsive?.small ?? 'initial', | |
| '--kiba-box-min-width-md': props.minWidthResponsive?.medium ?? 'initial', | |
| '--kiba-box-min-width-lg': props.minWidthResponsive?.large ?? 'initial', | |
| '--kiba-box-min-width-xl': props.minWidthResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-min-height': props.minHeight ?? 'initial', | |
| '--kiba-box-min-height-sm': props.minHeightResponsive?.small ?? 'initial', | |
| '--kiba-box-min-height-md': props.minHeightResponsive?.medium ?? 'initial', | |
| '--kiba-box-min-height-lg': props.minHeightResponsive?.large ?? 'initial', | |
| '--kiba-box-min-height-xl': props.minHeightResponsive?.extraLarge ?? 'initial', | |
| '--kiba-box-width-sm': props.widthResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-width-md': props.widthResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-width-lg': props.widthResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-width-xl': props.widthResponsive?.extraLarge ?? 'var(--kiba-unset)', | |
| '--kiba-box-height': height ?? 'initial', | |
| '--kiba-box-height-sm': props.heightResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-height-md': props.heightResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-height-lg': props.heightResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-height-xl': props.heightResponsive?.extraLarge ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-width': props.maxWidth ?? 'initial', | |
| '--kiba-box-max-width-sm': props.maxWidthResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-width-md': props.maxWidthResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-width-lg': props.maxWidthResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-width-xl': props.maxWidthResponsive?.extraLarge ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-height': props.maxHeight ?? 'initial', | |
| '--kiba-box-max-height-sm': props.maxHeightResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-height-md': props.maxHeightResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-height-lg': props.maxHeightResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-max-height-xl': props.maxHeightResponsive?.extraLarge ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-width': props.minWidth ?? 'initial', | |
| '--kiba-box-min-width-sm': props.minWidthResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-width-md': props.minWidthResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-width-lg': props.minWidthResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-width-xl': props.minWidthResponsive?.extraLarge ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-height': props.minHeight ?? 'initial', | |
| '--kiba-box-min-height-sm': props.minHeightResponsive?.small ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-height-md': props.minHeightResponsive?.medium ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-height-lg': props.minHeightResponsive?.large ?? 'var(--kiba-unset)', | |
| '--kiba-box-min-height-xl': props.minHeightResponsive?.extraLarge ?? 'var(--kiba-unset)', |
| // Responsive breakpoints - only use responsive variables if explicitly set (not initial) | ||
| @include screen-small { | ||
| width: var(--kiba-box-width-sm, var(--kiba-box-width, 100%)); | ||
| height: var(--kiba-box-height-sm, var(--kiba-box-height, 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)); | ||
| max-width: var(--kiba-box-max-width-sm, var(--kiba-box-max-width, none)); | ||
| max-height: var(--kiba-box-max-height-sm, var(--kiba-box-max-height, none)); |
There was a problem hiding this comment.
The new comment says responsive variables are only used if explicitly set (not initial), but the CSS here still uses plain var(--kiba-box-*-sm, ...) fallbacks and cannot detect whether the variable’s value is the keyword initial. As a result, if components set --kiba-box-*-sm: initial, the breakpoint will still use it and override the intended fallback.
Either adjust the implementation so the unset sentinel triggers the fallback (e.g. set the variable to an invalid computed value), or update/remove this comment to match actual behavior.
Description
Screenshots:
Checklist: