chore: grid and richtext improvements #201
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds comprehensive responsive spacing, margin, typography, and grid positioning support (base/md/lg) across Storyblok UI components; removes SbMediaWrapper and replaces it with inline responsive spacing logic; updates Storyblok schemas, datasources, Panda config, and Storybook magazine layout to use responsive gridColumn rules. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@packages/storyblok-sync/scripts/sync-components.ts`:
- Around line 954-968: SbParagraph's Storyblok schema is missing the new
fontSize field, so editors can't set it; add a fontSize entry alongside the
existing lineHeight/opacity/letterSpacing fields in the paragraph component
schema (the same object that currently contains lineHeight, opacity,
letterSpacing) — create a field named fontSize with a display_name like "Font
Size" and a suitable type and description (mirror the Headline fontSize field's
shape) so the SbParagraph component exposes the prop to editors.
In `@packages/storyblok-ui/src/components/container/SbContainer.tsx`:
- Around line 133-139: The responsive spacing construction uses the || operator
which treats the string "0" as falsy and incorrectly falls back to base values;
update each responsive spacing block (responsivePt, responsivePx, responsivePy,
responsivePb, responsiveMt, responsiveMb, responsiveMy, responsiveMl,
responsiveMr) to use the nullish coalescing operator (??) when combining
mapSpacingToToken(...) results with fallbacks so that valid "0" tokens are
preserved (e.g., replace occurrences of "mapSpacingToToken(...) || ptValue" with
"mapSpacingToToken(...) ?? ptValue" in SbContainer.tsx).
In `@packages/storyblok-ui/src/components/grid-item/SbGridItem.tsx`:
- Around line 144-187: The responsive handling drops rowStart/rowEnd because
buildGridRow only returns a span and responsiveGridRow only uses rowSpan; update
row handling so responsiveGridRow preserves start/end values: create/modify a
function (e.g., buildGridRow or buildGridRowValue) that accepts rowStart,
rowSpan, rowEnd (use parsePosition for start/end and parseRowSpan for span) and
returns either "start / end" when start/end exist or "span N" when only span
exists; then change responsiveGridRow to call that function for base/md/lg (pass
rowStart/rowSpan/rowEnd, rowStartMd/rowSpanMd/rowEndMd,
rowStartLg/rowSpanLg/rowEndLg) and keep responsiveCss/GridItem usage the same so
rowStart/rowEnd are preserved in responsive mode.
In `@packages/storyblok-ui/src/components/headline/SbHeadline.tsx`:
- Around line 56-63: In SbHeadline, move typography-related inline styles out of
the style prop and into the component's css prop so Panda can resolve tokens:
remove fontSize, lineHeight, fontWeight, letterSpacing, opacity (and
mappedColor/color token usage) from the style object and add them to the
existing css prop merge alongside spacing props; ensure you preserve any
computed/mapped values (e.g., mappedColor) when assigning to css and leave style
only for non-token, imperative values (or remove it entirely if unused).
In `@packages/storyblok-ui/src/components/image/SbImage.tsx`:
- Around line 301-344: The caption in SbImage is currently rendered outside the
spacing wrapper so spacing props don't affect the gap between image and caption;
update the JSX in SbImage to move the SbCaption (rendered with <SbCaption
data={caption} />) inside the inner spacing wrapper that contains the Image (the
div with px/py/pl/pr/... responsive styles and the child div using
RESPONSIVE_WIDTH_CLASSES), mirroring SbVideo's structure so the spacing props
apply to both image and caption.
In `@packages/ui/src/components/grid/grid-item.tsx`:
- Around line 86-100: The current shared flag hasGridClassInClassName causes
row-only classes to falsely mark hasResponsiveGridColumn and drop column
defaults; split the detection into two flags (e.g.,
hasGridColumnClassInClassName and hasGridRowClassInClassName) that check
className for column-specific tokens ("grid-c_", "md:grid-c_", "lg:grid-c_") vs
row-specific tokens ("grid-r_", "md:grid-r_", "lg:grid-r_"), then use the column
flag with hasResponsiveGridColumn (alongside cssProp?.gridColumn) and the row
flag with hasResponsiveGridRow (alongside cssProp?.gridRow) so colSpan/colStart
defaults are preserved when only row classes exist.
In `@packages/ui/src/components/list/list-item.tsx`:
- Around line 42-43: The base style's fontSize: "inherit" is never used because
the component's size prop defaults to "sm" via defaultVariants and the prop
default; update the component (list-item.tsx) to either add an explicit
"inherit" option to the size variants (e.g., include a "inherit" variant that
leaves fontSize as "inherit" and add "inherit" to the Size prop type) or make
size optional/remove the defaultVariants size so inheritance is possible by
default; also update the prop typing and any places that rely on defaultVariants
(and tests/usage) to reflect the new optional/default behavior.
In `@packages/ui/src/components/paragraph/paragraph.tsx`:
- Around line 211-223: The paragraph component currently sets colorVariant to
undefined when a custom color is provided, but Panda treats undefined as "not
provided" and applies the recipe defaultVariants; change the logic that computes
colorVariant (used in paragraphRecipe.raw call) to use null instead of undefined
when hasCustomColor is true so the color variant is explicitly removed and the
custom css prop color isn't overridden by defaultVariants (refer to
hasCustomColor, colorVariant, and paragraphRecipe.raw in the cx/css invocation).
🧹 Nitpick comments (5)
packages/ui/src/components/list/list-item.tsx (1)
46-48: Selector refinement looks good; consider avoiding!important.Using
& > pto target only direct child paragraphs is more precise and prevents unintended side effects on deeply nested content.However,
!importantis generally a code smell. If this is necessary to override specificity from paragraph components, consider documenting why, or using a higher-specificity selector if feasible.packages/storyblok-ui/src/components/section/SbSection.tsx (1)
94-100: Consider extractingfilterUndefinedto a shared util.This helper is now repeated across components (e.g., Section/Container). Centralizing it reduces drift and keeps responsive spacing logic consistent.
packages/storyblok-ui/src/components/slideshow/SbSlideshow.tsx (1)
104-164: Consider extracting the responsive spacing builder into a shared utility.This responsive spacing logic is duplicated across
SbSlideshow,SbVideo, andSbImage. A shared utility function would reduce duplication and ensure consistent behavior.♻️ Suggested utility function
In
packages/storyblok-ui/src/lib/spacing-utils.ts:export function buildResponsiveSpacing( base: string | undefined, md: string | undefined, lg: string | undefined ): string | { base: string | undefined; md: string | undefined; lg: string | undefined } | undefined { const baseValue = mapSpacingToToken(base); const mdValue = mapSpacingToToken(md); const lgValue = mapSpacingToToken(lg); if (md || lg) { return { base: baseValue, md: mdValue || baseValue, lg: lgValue || mdValue || baseValue, }; } return baseValue; }Then usage becomes:
const responsivePt = buildResponsiveSpacing(pt, ptMd, ptLg); const responsivePb = buildResponsiveSpacing(pb, pbMd, pbLg); // ...apps/storybook/stories/Grid.stories.tsx (1)
527-534: Minor: Redundant position declaration.The
position: { base: "relative", lg: "relative" }is equivalent to justposition: "relative"since the value doesn't change across breakpoints.♻️ Simplify position
<Box css={{ - position: { base: "relative", lg: "relative" }, + position: "relative", left: { base: "0", lg: "50%" }, right: { base: "0", lg: "50%" }, marginLeft: { base: "0", lg: "-50vw" }, marginRight: { base: "0", lg: "-50vw" }, width: { base: "100%", lg: "100vw" }, }} >packages/storyblok-ui/src/components/video/SbVideo.tsx (1)
141-217: Same duplication as noted in SbSlideshow.The responsive spacing logic (lines 141-201) is identical to
SbSlideshow. Consider the shared utility function suggested earlier.The
spacingStylesobject (lines 210-217) is a good pattern for reusing the computed values across the consent placeholder and main video render paths.
| lineHeight: { | ||
| type: "text", | ||
| display_name: "Line Height", | ||
| description: "Custom line height (e.g., '1.5', '2')", | ||
| }, | ||
| opacity: { | ||
| type: "text", | ||
| display_name: "Opacity", | ||
| description: "Opacity value (0-1, e.g., '0.7')", | ||
| }, | ||
| letterSpacing: { | ||
| type: "text", | ||
| display_name: "Letter Spacing", | ||
| description: "Letter spacing (e.g., '0.15em', '2px')", | ||
| }, |
There was a problem hiding this comment.
Paragraph schema misses new fontSize field.
SbParagraph now accepts a fontSize prop, but the Storyblok schema doesn’t expose it, so editors can’t use it. Consider adding a fontSize field similar to Headline.
✅ Suggested schema addition
align: {
type: "option",
display_name: "Text Align",
options: [
{ name: "Left", value: "left" },
{ name: "Center", value: "center" },
{ name: "Right", value: "right" },
],
},
+ fontSize: {
+ type: "text",
+ display_name: "Font Size",
+ description: "Custom font size (e.g., '2rem', '24px')",
+ },
lineHeight: {
type: "text",
display_name: "Line Height",
description: "Custom line height (e.g., '1.5', '2')",
},🤖 Prompt for AI Agents
In `@packages/storyblok-sync/scripts/sync-components.ts` around lines 954 - 968,
SbParagraph's Storyblok schema is missing the new fontSize field, so editors
can't set it; add a fontSize entry alongside the existing
lineHeight/opacity/letterSpacing fields in the paragraph component schema (the
same object that currently contains lineHeight, opacity, letterSpacing) — create
a field named fontSize with a display_name like "Font Size" and a suitable type
and description (mirror the Headline fontSize field's shape) so the SbParagraph
component exposes the prop to editors.
| const responsivePt = | ||
| ptMd || ptLg | ||
| ? filterUndefined({ | ||
| base: ptValue, | ||
| md: mapSpacingToToken(ptMd) || ptValue, | ||
| lg: mapSpacingToToken(ptLg) || mapSpacingToToken(ptMd) || ptValue, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file size and read the SbContainer.tsx file
wc -l packages/storyblok-ui/src/components/container/SbContainer.tsxRepository: dmnktoe/httpjpg
Length of output: 125
🏁 Script executed:
# Read the file to see the full context
cat -n packages/storyblok-ui/src/components/container/SbContainer.tsxRepository: dmnktoe/httpjpg
Length of output: 6788
🏁 Script executed:
# Find the mapSpacingToToken function to understand what it returns
rg -A 5 "mapSpacingToToken\s*=" packages/storyblok-ui/src/components/container/Repository: dmnktoe/httpjpg
Length of output: 41
🏁 Script executed:
# Search for all responsive spacing patterns in the file using || operator
rg -n "\|\|" packages/storyblok-ui/src/components/container/SbContainer.tsxRepository: dmnktoe/httpjpg
Length of output: 1572
🏁 Script executed:
# Find the spacing-utils file
find packages/storyblok-ui -name "*spacing*" -type fRepository: dmnktoe/httpjpg
Length of output: 157
🏁 Script executed:
# Also search for the file with ripgrep
rg -l "spacing-utils" packages/storyblok-uiRepository: dmnktoe/httpjpg
Length of output: 862
🏁 Script executed:
# Read the spacing-utils file to understand mapSpacingToToken
cat -n packages/storyblok-ui/src/lib/spacing-utils.tsRepository: dmnktoe/httpjpg
Length of output: 6133
🏁 Script executed:
# Also check the test file to see expected behavior
cat -n packages/storyblok-ui/src/lib/spacing-utils.test.tsRepository: dmnktoe/httpjpg
Length of output: 2575
🏁 Script executed:
# Find and read the constants file
find packages/storyblok-ui -name "constants*" -type fRepository: dmnktoe/httpjpg
Length of output: 101
🏁 Script executed:
# Read the constants file
cat -n packages/storyblok-ui/src/lib/constants.tsRepository: dmnktoe/httpjpg
Length of output: 2496
Use nullish coalescing operator so "0" tokens remain valid at md/lg breakpoints.
When mapSpacingToToken returns "0" (the string token for "none" spacing), the || operator treats it as falsy and falls back to base spacing instead. This occurs across all nine responsive spacing blocks (px, py, pt, pb, mt, mb, my, ml, mr). Switch to ?? to only replace on null/undefined:
🔧 Apply to all responsive spacing blocks
- md: mapSpacingToToken(ptMd) || ptValue,
- lg: mapSpacingToToken(ptLg) || mapSpacingToToken(ptMd) || ptValue,
+ md: mapSpacingToToken(ptMd) ?? ptValue,
+ lg: mapSpacingToToken(ptLg) ?? mapSpacingToToken(ptMd) ?? ptValue,Repeat similarly for: responsivePx, responsivePy, responsivePb, responsiveMt, responsiveMb, responsiveMy, responsiveMl, responsiveMr.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const responsivePt = | |
| ptMd || ptLg | |
| ? filterUndefined({ | |
| base: ptValue, | |
| md: mapSpacingToToken(ptMd) || ptValue, | |
| lg: mapSpacingToToken(ptLg) || mapSpacingToToken(ptMd) || ptValue, | |
| }) | |
| const responsivePt = | |
| ptMd || ptLg | |
| ? filterUndefined({ | |
| base: ptValue, | |
| md: mapSpacingToToken(ptMd) ?? ptValue, | |
| lg: mapSpacingToToken(ptLg) ?? mapSpacingToToken(ptMd) ?? ptValue, | |
| }) |
🤖 Prompt for AI Agents
In `@packages/storyblok-ui/src/components/container/SbContainer.tsx` around lines
133 - 139, The responsive spacing construction uses the || operator which treats
the string "0" as falsy and incorrectly falls back to base values; update each
responsive spacing block (responsivePt, responsivePx, responsivePy,
responsivePb, responsiveMt, responsiveMb, responsiveMy, responsiveMl,
responsiveMr) to use the nullish coalescing operator (??) when combining
mapSpacingToToken(...) results with fallbacks so that valid "0" tokens are
preserved (e.g., replace occurrences of "mapSpacingToToken(...) || ptValue" with
"mapSpacingToToken(...) ?? ptValue" in SbContainer.tsx).
| const buildGridRow = (spanVal?: string): string | undefined => { | ||
| const span = parseRowSpan(spanVal); | ||
| return span ? `span ${span}` : undefined; | ||
| }; | ||
|
|
||
| // Build responsive CSS object - only include values that exist | ||
| const responsiveGridColumn = hasResponsiveValues | ||
| ? { | ||
| base: buildGridColumn(colStart, colSpan, colEnd), | ||
| ...(colStartMd || colSpanMd || colEndMd | ||
| ? { md: buildGridColumn(colStartMd, colSpanMd, colEndMd) } | ||
| : {}), | ||
| ...(colStartLg || colSpanLg || colEndLg | ||
| ? { lg: buildGridColumn(colStartLg, colSpanLg, colEndLg) } | ||
| : {}), | ||
| } | ||
| : undefined; | ||
|
|
||
| const responsiveGridRow = hasResponsiveValues | ||
| ? { | ||
| base: buildGridRow(rowSpan), | ||
| ...(rowSpanMd ? { md: buildGridRow(rowSpanMd) } : {}), | ||
| ...(rowSpanLg ? { lg: buildGridRow(rowSpanLg) } : {}), | ||
| } | ||
| : undefined; | ||
|
|
||
| const responsiveCss = hasResponsiveValues | ||
| ? css({ | ||
| ...(responsiveGridColumn && { gridColumn: responsiveGridColumn }), | ||
| ...(responsiveGridRow && { gridRow: responsiveGridRow }), | ||
| }) | ||
| : undefined; | ||
|
|
||
| return ( | ||
| <> | ||
| {responsiveStyles} | ||
| <GridItem | ||
| {...editableProps} | ||
| colSpan={hasPositioning ? undefined : baseColSpan} | ||
| rowSpan={hasPositioning ? undefined : baseRowSpan} | ||
| colStart={parsePosition(colStart)} | ||
| colEnd={parsePosition(colEnd)} | ||
| rowStart={parsePosition(rowStart)} | ||
| rowEnd={parsePosition(rowEnd)} | ||
| data-sb-grid-item={blok._uid} | ||
| > | ||
| {content && content.length > 0 && ( | ||
| <DynamicRender data={content as BlokItem[]} /> | ||
| )} | ||
| </GridItem> | ||
| </> | ||
| <GridItem | ||
| {...editableProps} | ||
| {...(!hasResponsiveValues && { | ||
| colSpan: baseColSpan, | ||
| rowSpan: baseRowSpan, | ||
| colStart: parsePosition(colStart), | ||
| colEnd: parsePosition(colEnd), | ||
| rowStart: parsePosition(rowStart), | ||
| rowEnd: parsePosition(rowEnd), | ||
| })} |
There was a problem hiding this comment.
Row start/end are ignored when any responsive values are set.
When hasResponsiveValues is true, base props aren’t passed and responsiveGridRow only uses rowSpan, so rowStart/rowEnd from Storyblok are silently dropped. This changes layout behavior as soon as any responsive column/row value is present.
🐛 Proposed fix (preserve rowStart/rowEnd in responsive mode)
- const buildGridRow = (spanVal?: string): string | undefined => {
- const span = parseRowSpan(spanVal);
- return span ? `span ${span}` : undefined;
- };
+ const buildGridRow = (
+ startVal?: string,
+ spanVal?: string,
+ endVal?: string,
+ ): string | undefined => {
+ const start = parsePosition(startVal);
+ const span = parseRowSpan(spanVal);
+ const end = parsePosition(endVal);
+ if (start && end) {
+ return `${start} / ${end}`;
+ }
+ if (start && span) {
+ return `${start} / span ${span}`;
+ }
+ if (span) {
+ return `span ${span}`;
+ }
+ if (start || end) {
+ return `${start || "auto"} / ${end || "auto"}`;
+ }
+ return undefined;
+ };
- const responsiveGridRow = hasResponsiveValues
- ? {
- base: buildGridRow(rowSpan),
- ...(rowSpanMd ? { md: buildGridRow(rowSpanMd) } : {}),
- ...(rowSpanLg ? { lg: buildGridRow(rowSpanLg) } : {}),
- }
- : undefined;
+ const responsiveGridRow = hasResponsiveValues
+ ? {
+ base: buildGridRow(rowStart, rowSpan, rowEnd),
+ ...(rowSpanMd
+ ? { md: buildGridRow(rowStart, rowSpanMd, rowEnd) }
+ : {}),
+ ...(rowSpanLg
+ ? { lg: buildGridRow(rowStart, rowSpanLg, rowEnd) }
+ : {}),
+ }
+ : undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const buildGridRow = (spanVal?: string): string | undefined => { | |
| const span = parseRowSpan(spanVal); | |
| return span ? `span ${span}` : undefined; | |
| }; | |
| // Build responsive CSS object - only include values that exist | |
| const responsiveGridColumn = hasResponsiveValues | |
| ? { | |
| base: buildGridColumn(colStart, colSpan, colEnd), | |
| ...(colStartMd || colSpanMd || colEndMd | |
| ? { md: buildGridColumn(colStartMd, colSpanMd, colEndMd) } | |
| : {}), | |
| ...(colStartLg || colSpanLg || colEndLg | |
| ? { lg: buildGridColumn(colStartLg, colSpanLg, colEndLg) } | |
| : {}), | |
| } | |
| : undefined; | |
| const responsiveGridRow = hasResponsiveValues | |
| ? { | |
| base: buildGridRow(rowSpan), | |
| ...(rowSpanMd ? { md: buildGridRow(rowSpanMd) } : {}), | |
| ...(rowSpanLg ? { lg: buildGridRow(rowSpanLg) } : {}), | |
| } | |
| : undefined; | |
| const responsiveCss = hasResponsiveValues | |
| ? css({ | |
| ...(responsiveGridColumn && { gridColumn: responsiveGridColumn }), | |
| ...(responsiveGridRow && { gridRow: responsiveGridRow }), | |
| }) | |
| : undefined; | |
| return ( | |
| <> | |
| {responsiveStyles} | |
| <GridItem | |
| {...editableProps} | |
| colSpan={hasPositioning ? undefined : baseColSpan} | |
| rowSpan={hasPositioning ? undefined : baseRowSpan} | |
| colStart={parsePosition(colStart)} | |
| colEnd={parsePosition(colEnd)} | |
| rowStart={parsePosition(rowStart)} | |
| rowEnd={parsePosition(rowEnd)} | |
| data-sb-grid-item={blok._uid} | |
| > | |
| {content && content.length > 0 && ( | |
| <DynamicRender data={content as BlokItem[]} /> | |
| )} | |
| </GridItem> | |
| </> | |
| <GridItem | |
| {...editableProps} | |
| {...(!hasResponsiveValues && { | |
| colSpan: baseColSpan, | |
| rowSpan: baseRowSpan, | |
| colStart: parsePosition(colStart), | |
| colEnd: parsePosition(colEnd), | |
| rowStart: parsePosition(rowStart), | |
| rowEnd: parsePosition(rowEnd), | |
| })} | |
| const buildGridRow = ( | |
| startVal?: string, | |
| spanVal?: string, | |
| endVal?: string, | |
| ): string | undefined => { | |
| const start = parsePosition(startVal); | |
| const span = parseRowSpan(spanVal); | |
| const end = parsePosition(endVal); | |
| if (start && end) { | |
| return `${start} / ${end}`; | |
| } | |
| if (start && span) { | |
| return `${start} / span ${span}`; | |
| } | |
| if (span) { | |
| return `span ${span}`; | |
| } | |
| if (start || end) { | |
| return `${start || "auto"} / ${end || "auto"}`; | |
| } | |
| return undefined; | |
| }; | |
| // Build responsive CSS object - only include values that exist | |
| const responsiveGridColumn = hasResponsiveValues | |
| ? { | |
| base: buildGridColumn(colStart, colSpan, colEnd), | |
| ...(colStartMd || colSpanMd || colEndMd | |
| ? { md: buildGridColumn(colStartMd, colSpanMd, colEndMd) } | |
| : {}), | |
| ...(colStartLg || colSpanLg || colEndLg | |
| ? { lg: buildGridColumn(colStartLg, colSpanLg, colEndLg) } | |
| : {}), | |
| } | |
| : undefined; | |
| const responsiveGridRow = hasResponsiveValues | |
| ? { | |
| base: buildGridRow(rowStart, rowSpan, rowEnd), | |
| ...(rowSpanMd | |
| ? { md: buildGridRow(rowStart, rowSpanMd, rowEnd) } | |
| : {}), | |
| ...(rowSpanLg | |
| ? { lg: buildGridRow(rowStart, rowSpanLg, rowEnd) } | |
| : {}), | |
| } | |
| : undefined; | |
| const responsiveCss = hasResponsiveValues | |
| ? css({ | |
| ...(responsiveGridColumn && { gridColumn: responsiveGridColumn }), | |
| ...(responsiveGridRow && { gridRow: responsiveGridRow }), | |
| }) | |
| : undefined; | |
| return ( | |
| <GridItem | |
| {...editableProps} | |
| {...(!hasResponsiveValues && { | |
| colSpan: baseColSpan, | |
| rowSpan: baseRowSpan, | |
| colStart: parsePosition(colStart), | |
| colEnd: parsePosition(colEnd), | |
| rowStart: parsePosition(rowStart), | |
| rowEnd: parsePosition(rowEnd), | |
| })} |
🤖 Prompt for AI Agents
In `@packages/storyblok-ui/src/components/grid-item/SbGridItem.tsx` around lines
144 - 187, The responsive handling drops rowStart/rowEnd because buildGridRow
only returns a span and responsiveGridRow only uses rowSpan; update row handling
so responsiveGridRow preserves start/end values: create/modify a function (e.g.,
buildGridRow or buildGridRowValue) that accepts rowStart, rowSpan, rowEnd (use
parsePosition for start/end and parseRowSpan for span) and returns either "start
/ end" when start/end exist or "span N" when only span exists; then change
responsiveGridRow to call that function for base/md/lg (pass
rowStart/rowSpan/rowEnd, rowStartMd/rowSpanMd/rowEndMd,
rowStartLg/rowSpanLg/rowEndLg) and keep responsiveCss/GridItem usage the same so
rowStart/rowEnd are preserved in responsive mode.
| style={{ | ||
| color: mappedColor || "black", | ||
| ...(mappedColor && { color: mappedColor }), | ||
| ...(fontSize && { fontSize }), | ||
| ...(lineHeight && { lineHeight }), | ||
| ...(fontWeight && { fontWeight }), | ||
| ...(opacity && { opacity }), | ||
| ...(letterSpacing && { letterSpacing }), | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the SbHeadline.tsx file
cat -n packages/storyblok-ui/src/components/headline/SbHeadline.tsxRepository: dmnktoe/httpjpg
Length of output: 2378
🏁 Script executed:
# Check for Panda configuration in the project
find . -maxdepth 3 -type f \( -name "panda.config.*" -o -name "pandaConfig*" \) | head -20Repository: dmnktoe/httpjpg
Length of output: 88
🏁 Script executed:
# Look for Panda imports or setup in package.json
cat package.json | grep -A 5 -B 5 "panda"Repository: dmnktoe/httpjpg
Length of output: 41
🏁 Script executed:
# Check if there's tsconfig or other config that might indicate Panda setup
find packages/storyblok-ui -maxdepth 2 -type f -name "*.config.*" | head -20Repository: dmnktoe/httpjpg
Length of output: 41
🏁 Script executed:
# Find the Headline component
find packages/ui -type f -name "*.ts*" | xargs rg -l "export.*Headline" | head -10Repository: dmnktoe/httpjpg
Length of output: 152
🏁 Script executed:
# Check the panda config
cat packages/ui/panda.config.tsRepository: dmnktoe/httpjpg
Length of output: 19692
🏁 Script executed:
# Look at the directory structure of the ui package
ls -la packages/ui/src/components/ | head -20Repository: dmnktoe/httpjpg
Length of output: 1187
🏁 Script executed:
# Check if there's a Headline component
find packages/ui/src -name "*[Hh]eadline*" -o -name "*[Hh]ead*"Repository: dmnktoe/httpjpg
Length of output: 276
🏁 Script executed:
# Look at the Headline component implementation
cat -n packages/ui/src/components/headline/headline.tsxRepository: dmnktoe/httpjpg
Length of output: 4868
🏁 Script executed:
# Check the index to see what's exported
cat packages/ui/src/components/headline/index.tsRepository: dmnktoe/httpjpg
Length of output: 117
🏁 Script executed:
# Look at the tokens package to understand the typography tokens
find packages -name "*tokens*" -type dRepository: dmnktoe/httpjpg
Length of output: 74
🏁 Script executed:
# Check if there's a tokens package and what typography is exported
find . -path "*/packages/tokens*" -type d | head -5Repository: dmnktoe/httpjpg
Length of output: 124
🏁 Script executed:
# Check the tokens package to see what typography tokens are available
cat packages/tokens/src/index.ts 2>/dev/null || ls -la packages/tokens/src/Repository: dmnktoe/httpjpg
Length of output: 1455
🏁 Script executed:
# Look at the mapping utility functions
cat packages/storyblok-ui/src/lib/token-mapping.tsRepository: dmnktoe/httpjpg
Length of output: 9381
🏁 Script executed:
# Check if there are any mapping functions for typography
find packages/storyblok-ui/src/lib -type f -name "*.ts" | xargs ls -laRepository: dmnktoe/httpjpg
Length of output: 653
🏁 Script executed:
# Check what values are actually passed in the CMS/data
rg "fontSize|lineHeight|fontWeight" packages/storyblok-ui/src --type ts --context 3 | head -50Repository: dmnktoe/httpjpg
Length of output: 3955
🏁 Script executed:
# Check SbParagraph to see how it handles typography props
cat -n packages/storyblok-ui/src/components/paragraph/SbParagraph.tsx | head -80Repository: dmnktoe/httpjpg
Length of output: 2720
🏁 Script executed:
# Check if mapFontSizeToToken and other typography mappers are actually used anywhere
rg "mapFontSizeToToken|mapFontWeightToToken|mapLineHeightToToken|mapLetterSpacingToToken" packages/storyblok-ui/src --type tsRepository: dmnktoe/httpjpg
Length of output: 1401
🏁 Script executed:
# Look at the full typography tokens to understand the structure
cat packages/tokens/src/typography.ts 2>/dev/null || ls -la packages/tokens/src/Repository: dmnktoe/httpjpg
Length of output: 1364
🏁 Script executed:
# Check the Paragraph component to see how it handles css prop
cat -n packages/ui/src/components/paragraph/paragraph.tsx | head -100Repository: dmnktoe/httpjpg
Length of output: 2973
🏁 Script executed:
# Check if there's any documentation on how Panda CSS css prop works vs style prop
rg "css\?.*SystemStyleObject|style\?.*CSSProperties" packages/ui/src/components -A 2 -B 2 | head -40Repository: dmnktoe/httpjpg
Length of output: 2236
🏁 Script executed:
# Verify how values flow through - check if any mapping is actually applied in practice
cat packages/storyblok-ui/src/components/headline/SbHeadline.tsxRepository: dmnktoe/httpjpg
Length of output: 1853
Move typography props to css prop for Panda token resolution.
Inline style bypasses Panda CSS, so token names like "sm", "bold", etc. won't resolve. These should be in the css prop alongside the spacing props so Panda can resolve them to their actual token values.
Suggested fix
<Headline
{...editableProps}
level={level}
- style={{
- ...(mappedColor && { color: mappedColor }),
- ...(fontSize && { fontSize }),
- ...(lineHeight && { lineHeight }),
- ...(fontWeight && { fontWeight }),
- ...(opacity && { opacity }),
- ...(letterSpacing && { letterSpacing }),
- }}
css={{
+ ...(mappedColor && { color: mappedColor }),
+ ...(fontSize && { fontSize }),
+ ...(lineHeight && { lineHeight }),
+ ...(fontWeight && { fontWeight }),
+ ...(opacity && { opacity }),
+ ...(letterSpacing && { letterSpacing }),
textAlign: align,
mt: mapSpacingToToken(marginTop),
mb: mapSpacingToToken(marginBottom),
pt: mapSpacingToToken(paddingTop),
pb: mapSpacingToToken(paddingBottom),
}}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| style={{ | |
| color: mappedColor || "black", | |
| ...(mappedColor && { color: mappedColor }), | |
| ...(fontSize && { fontSize }), | |
| ...(lineHeight && { lineHeight }), | |
| ...(fontWeight && { fontWeight }), | |
| ...(opacity && { opacity }), | |
| ...(letterSpacing && { letterSpacing }), | |
| }} | |
| <Headline | |
| {...editableProps} | |
| level={level} | |
| css={{ | |
| ...(mappedColor && { color: mappedColor }), | |
| ...(fontSize && { fontSize }), | |
| ...(lineHeight && { lineHeight }), | |
| ...(fontWeight && { fontWeight }), | |
| ...(opacity && { opacity }), | |
| ...(letterSpacing && { letterSpacing }), | |
| textAlign: align, | |
| mt: mapSpacingToToken(marginTop), | |
| mb: mapSpacingToToken(marginBottom), | |
| pt: mapSpacingToToken(paddingTop), | |
| pb: mapSpacingToToken(paddingBottom), | |
| }} | |
| > |
🤖 Prompt for AI Agents
In `@packages/storyblok-ui/src/components/headline/SbHeadline.tsx` around lines 56
- 63, In SbHeadline, move typography-related inline styles out of the style prop
and into the component's css prop so Panda can resolve tokens: remove fontSize,
lineHeight, fontWeight, letterSpacing, opacity (and mappedColor/color token
usage) from the style object and add them to the existing css prop merge
alongside spacing props; ensure you preserve any computed/mapped values (e.g.,
mappedColor) when assigning to css and leave style only for non-token,
imperative values (or remove it entirely if unused).
| return ( | ||
| <SbMediaWrapper | ||
| spacingTop={spacingTop} | ||
| spacingBottom={spacingBottom} | ||
| width={width} | ||
| editable={editableProps} | ||
| > | ||
| <div {...editableProps} className={css({ ...widthStyles })}> | ||
| <div | ||
| className={ | ||
| RESPONSIVE_WIDTH_CLASSES[ | ||
| calculatedWidth as keyof typeof RESPONSIVE_WIDTH_CLASSES | ||
| ] || RESPONSIVE_WIDTH_CLASSES["100%"] | ||
| } | ||
| className={css({ | ||
| px: responsivePx, | ||
| py: responsivePy, | ||
| pl: responsivePl, | ||
| pr: responsivePr, | ||
| mt: responsiveMt, | ||
| mb: responsiveMb, | ||
| my: responsiveMy, | ||
| ml: responsiveMl, | ||
| mr: responsiveMr, | ||
| })} | ||
| > | ||
| <Image | ||
| src={processedSrc} | ||
| alt={alt || image.alt || image.title || ""} | ||
| copyright={finalCopyright} | ||
| copyrightPosition={copyrightPosition} | ||
| blurOnLoad={blurOnLoad} | ||
| blurDataURL={blurDataURL} | ||
| css={{ | ||
| width: "100%", | ||
| maxWidth: "100%", | ||
| height: isFullHeight ? "100vh" : "auto", | ||
| objectFit: aspectRatio ? "cover" : "none", | ||
| aspectRatio: aspectRatio || undefined, | ||
| }} | ||
| loading={isLoadingEager ? "eager" : "lazy"} | ||
| /> | ||
| <div | ||
| className={ | ||
| RESPONSIVE_WIDTH_CLASSES[ | ||
| calculatedWidth as keyof typeof RESPONSIVE_WIDTH_CLASSES | ||
| ] || RESPONSIVE_WIDTH_CLASSES["100%"] | ||
| } | ||
| > | ||
| <Image | ||
| src={processedSrc} | ||
| alt={alt || image.alt || image.title || ""} | ||
| copyright={finalCopyright} | ||
| copyrightPosition={copyrightPosition} | ||
| blurOnLoad={blurOnLoad} | ||
| blurDataURL={blurDataURL} | ||
| css={{ | ||
| width: "100%", | ||
| maxWidth: "100%", | ||
| height: isFullHeight ? "100vh" : "auto", | ||
| objectFit: aspectRatio ? "cover" : "none", | ||
| aspectRatio: aspectRatio || undefined, | ||
| }} | ||
| loading={isLoadingEager ? "eager" : "lazy"} | ||
| /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {caption && <SbCaption data={caption} />} | ||
| </SbMediaWrapper> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -A 5 '<SbCaption' packages/storyblok-ui/src/components/Repository: dmnktoe/httpjpg
Length of output: 762
🏁 Script executed:
cd packages/storyblok-ui/src/components && cat -n video/SbVideo.tsx | grep -A 20 -B 20 'SbCaption'Repository: dmnktoe/httpjpg
Length of output: 2333
🏁 Script executed:
cd packages/storyblok-ui/src/components && cat -n image/SbImage.tsx | grep -A 10 -B 30 'SbCaption'Repository: dmnktoe/httpjpg
Length of output: 2431
Caption placement is inconsistent with SbVideo.
In SbImage, SbCaption is rendered outside the spacing wrapper (line 342), so spacing props won't affect the gap between the image and caption. In SbVideo, the caption is inside the spacing wrapper (line 260 in SbVideo.tsx), making spacing props apply to both the video and caption.
Move the caption inside the spacing wrapper for consistency, or document why the intentional difference exists.
🤖 Prompt for AI Agents
In `@packages/storyblok-ui/src/components/image/SbImage.tsx` around lines 301 -
344, The caption in SbImage is currently rendered outside the spacing wrapper so
spacing props don't affect the gap between image and caption; update the JSX in
SbImage to move the SbCaption (rendered with <SbCaption data={caption} />)
inside the inner spacing wrapper that contains the Image (the div with
px/py/pl/pr/... responsive styles and the child div using
RESPONSIVE_WIDTH_CLASSES), mirroring SbVideo's structure so the spacing props
apply to both image and caption.
| // Check if className contains Panda CSS grid positioning classes | ||
| const hasGridClassInClassName = | ||
| className && | ||
| (className.includes("grid-c_") || | ||
| className.includes("grid-r_") || | ||
| className.includes("md:grid-c_") || | ||
| className.includes("lg:grid-c_") || | ||
| className.includes("md:grid-r_") || | ||
| className.includes("lg:grid-r_")); | ||
|
|
||
| // Calculate grid positioning | ||
| // For simple spans, Panda will use generated utility classes | ||
| // For complex positioning (start/end), we use inline styles (unavoidable) | ||
| const hasResponsiveGridColumn = | ||
| cssProp?.gridColumn !== undefined || hasGridClassInClassName; | ||
| const hasResponsiveGridRow = | ||
| cssProp?.gridRow !== undefined || hasGridClassInClassName; |
There was a problem hiding this comment.
Split row/column class detection to avoid dropping colSpan defaults.
Line 86-100: hasGridClassInClassName is used for both column and row. If className only contains grid-r_ (row) utilities, hasResponsiveGridColumn becomes true and gridColumn defaults are skipped, so colSpan/colStart won’t apply. This is a layout regression.
🛠 Proposed fix
- const hasGridClassInClassName =
- className &&
- (className.includes("grid-c_") ||
- className.includes("grid-r_") ||
- className.includes("md:grid-c_") ||
- className.includes("lg:grid-c_") ||
- className.includes("md:grid-r_") ||
- className.includes("lg:grid-r_"));
+ const hasGridColumnClassInClassName =
+ className &&
+ (className.includes("grid-c_") ||
+ className.includes("md:grid-c_") ||
+ className.includes("lg:grid-c_"));
+ const hasGridRowClassInClassName =
+ className &&
+ (className.includes("grid-r_") ||
+ className.includes("md:grid-r_") ||
+ className.includes("lg:grid-r_"));
- const hasResponsiveGridColumn =
- cssProp?.gridColumn !== undefined || hasGridClassInClassName;
- const hasResponsiveGridRow =
- cssProp?.gridRow !== undefined || hasGridClassInClassName;
+ const hasResponsiveGridColumn =
+ cssProp?.gridColumn !== undefined || hasGridColumnClassInClassName;
+ const hasResponsiveGridRow =
+ cssProp?.gridRow !== undefined || hasGridRowClassInClassName;🤖 Prompt for AI Agents
In `@packages/ui/src/components/grid/grid-item.tsx` around lines 86 - 100, The
current shared flag hasGridClassInClassName causes row-only classes to falsely
mark hasResponsiveGridColumn and drop column defaults; split the detection into
two flags (e.g., hasGridColumnClassInClassName and hasGridRowClassInClassName)
that check className for column-specific tokens ("grid-c_", "md:grid-c_",
"lg:grid-c_") vs row-specific tokens ("grid-r_", "md:grid-r_", "lg:grid-r_"),
then use the column flag with hasResponsiveGridColumn (alongside
cssProp?.gridColumn) and the row flag with hasResponsiveGridRow (alongside
cssProp?.gridRow) so colSpan/colStart defaults are preserved when only row
classes exist.
| /* Inherit font size from parent list */ | ||
| fontSize: "inherit", |
There was a problem hiding this comment.
fontSize: "inherit" in base is unreachable due to default variant.
The comment states the intent is to inherit from the parent list, but this value will always be overridden because:
- The component defaults
sizeto"sm"(line 90) - The
defaultVariantsalso specifiessize: "sm"(lines 63-65) - All size variants explicitly set
fontSize
To actually support inheritance, consider adding an explicit "inherit" variant option or making size optional without a default.
Suggested approach
variants: {
size: {
+ inherit: {
+ fontSize: "inherit",
+ },
sm: {
fontSize: "sm", // 12px
},Then update the prop type and defaults accordingly if inheriting is the desired default behavior.
🤖 Prompt for AI Agents
In `@packages/ui/src/components/list/list-item.tsx` around lines 42 - 43, The base
style's fontSize: "inherit" is never used because the component's size prop
defaults to "sm" via defaultVariants and the prop default; update the component
(list-item.tsx) to either add an explicit "inherit" option to the size variants
(e.g., include a "inherit" variant that leaves fontSize as "inherit" and add
"inherit" to the Size prop type) or make size optional/remove the
defaultVariants size so inheritance is possible by default; also update the prop
typing and any places that rely on defaultVariants (and tests/usage) to reflect
the new optional/default behavior.
| // Don't use color variant if custom color is provided in css prop | ||
| const hasCustomColor = cssProp && "color" in cssProp; | ||
| const colorVariant = hasCustomColor ? undefined : color; | ||
|
|
||
| const styles = cx( | ||
| css(paragraphRecipe.raw({ size, align, color, weight, spacing })), | ||
| css( | ||
| paragraphRecipe.raw({ | ||
| size, | ||
| align, | ||
| color: colorVariant, | ||
| weight, | ||
| spacing, | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the paragraph component and its recipe configuration
find . -type f -name "*.ts" -o -name "*.tsx" | grep -i paragraph | head -20Repository: dmnktoe/httpjpg
Length of output: 174
🏁 Script executed:
# Search for paragraphRecipe definition
rg "paragraphRecipe" --type ts --type tsx -A 5 -B 2Repository: dmnktoe/httpjpg
Length of output: 86
🏁 Script executed:
# Look for Panda recipe/cva configuration
rg "recipe\(|cva\(" --type ts --type tsx -A 10 | head -50Repository: dmnktoe/httpjpg
Length of output: 86
🏁 Script executed:
# Find paragraph-related files
find . -name "*paragraph*" -type f | head -20Repository: dmnktoe/httpjpg
Length of output: 111
🏁 Script executed:
# Search for paragraphRecipe without file type filter
rg "paragraphRecipe" -A 10 -B 2 | head -100Repository: dmnktoe/httpjpg
Length of output: 1890
🏁 Script executed:
# Search for recipe or cva definitions
rg "recipe\(|paragraphRecipe\s*=" -A 15 | head -150Repository: dmnktoe/httpjpg
Length of output: 1153
🏁 Script executed:
# Read the full paragraph.tsx file to see the complete recipe definition
cat ./packages/ui/src/components/paragraph/paragraph.tsxRepository: dmnktoe/httpjpg
Length of output: 5475
🏁 Script executed:
# Search for test files or examples with recipe usage and undefined variants
find . -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "recipe\|cva" 2>/dev/null | head -10Repository: dmnktoe/httpjpg
Length of output: 41
🏁 Script executed:
# Look for other recipe definitions and how they handle undefined variants
rg "recipe\(" -B 3 -A 30 | grep -A 30 "defaultVariants"Repository: dmnktoe/httpjpg
Length of output: 41
🏁 Script executed:
# Search for Panda CSS or cva documentation in the repo
find . -name "*.md" -o -name "package.json" | xargs grep -l "panda\|styled-system" 2>/dev/nullRepository: dmnktoe/httpjpg
Length of output: 85
🏁 Script executed:
# Check package.json for Panda/styled-system version
cat ./packages/ui/package.jsonRepository: dmnktoe/httpjpg
Length of output: 2003
🏁 Script executed:
# Search for other uses of recipe.raw with undefined or conditional variants
rg "recipe\.raw|cva" -B 5 -A 5 | head -200Repository: dmnktoe/httpjpg
Length of output: 11060
🌐 Web query:
Panda CSS cva recipe undefined variant defaultVariants behavior
💡 Result:
Short answer
- Panda's cva recipes use defaultVariants when you do not provide a value for a variant. [1]
- If you explicitly set a variant to null (CVA behavior), the default is removed for that call. Passing undefined is treated as "not provided" so the recipe falls back to the defaultVariant. [1][2]
Why
- Panda's recipe/cva docs describe defaultVariants as values applied when a variant isn't provided. [1]
- The underlying cva/Class Variance Authority API documents that defaultVariants can be removed by setting the variant to null; unspecified/omitted props use the defaults. The library typings treat variant props as optional or nullable for this reason. [2][3]
Example
- Given:
const btn = cva({ variants:{ size:{sm:'...',lg:'...'} }, defaultVariants:{ size: 'lg' } }) - btn() -> uses size = 'lg'. [1]
- btn({ size: undefined }) -> treated as not provided -> uses 'lg'. [1][2]
- btn({ size: null }) -> explicitly removes default -> no size variant applied. [2]
Sources
[1] Panda CSS — Recipes / cva documentation.
[2] cva (Class Variance Authority) API / notes on defaultVariants and null.
[3] cva TypeScript typings showing variant props may be nullable/optional.
If you want, I can show a runnable code example or confirm exact behavior for the Panda version you’re using — tell me your Panda/CVA versions.
Use null instead of undefined to suppress color variant defaults.
When hasCustomColor is true, passing color: undefined doesn't suppress the color variant—Panda treats undefined as "not provided" and applies defaultVariants.color: "default" (which includes opacity: 1), overriding your custom color. Use null to explicitly remove the variant and prevent opacity from the recipe affecting custom colors provided via the css prop.
const colorVariant = hasCustomColor ? null : color;🤖 Prompt for AI Agents
In `@packages/ui/src/components/paragraph/paragraph.tsx` around lines 211 - 223,
The paragraph component currently sets colorVariant to undefined when a custom
color is provided, but Panda treats undefined as "not provided" and applies the
recipe defaultVariants; change the logic that computes colorVariant (used in
paragraphRecipe.raw call) to use null instead of undefined when hasCustomColor
is true so the color variant is explicitly removed and the custom css prop color
isn't overridden by defaultVariants (refer to hasCustomColor, colorVariant, and
paragraphRecipe.raw in the cx/css invocation).
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.