feat: add variants and new props to product section #996
Conversation
WalkthroughAdds product-card customization: new ProductSectionContext and ProductSection styling fields (cardVariant, showImage, showBrow, showTitle, showPrice, showDescription, showCTA). ProductCard gains BrowSlot and PriceSlot (CategorySlot removed) and propagates imageConstrain/width settings to ImageSlot. Image component gains imageConstrain and showImageConstrain props plus updated width-visibility logic. New migration 0054_product_variants transforms nested CardSlot structures and sets defaults. getDefaultRTF signature extended; many locale keys and docs updated. migrationRegistry updated to include the new migration. Sequence Diagram(s)sequenceDiagram
participant Editor as Editor UI
participant ProductSection as ProductSection
participant Provider as ProductSectionProvider
participant ProductCard as ProductCard
participant Image as ImageSlot
Editor->>ProductSection: provide styles & slots (cardVariant, showImage, showBrow, ...)
ProductSection->>ProductSection: resolveData -> compute variant, show flags, imageConstrain, hideWidthProp
ProductSection->>Provider: wrap children with context (variant, imageConstrain, show* flags)
Provider->>ProductCard: context available
ProductCard->>ProductCard: useProductSectionContext() -> derive visibility booleans
ProductCard->>Image: render ImageSlot with imageConstrain & hideWidthProp
Image->>Image: resolveFields -> decide width visibility -> render image element
sequenceDiagram
participant MigrationRunner as Migration System
participant Migration0054 as 0054_product_variants
participant CardsWrapper as CardsWrapperSlot
participant Card as CardSlot
participant Category as CategorySlot
participant Brow as BrowSlot
participant Price as PriceSlot
MigrationRunner->>Migration0054: run migration
Migration0054->>CardsWrapper: iterate wrappers
CardsWrapper->>Card: inspect card.slots
Card->>Category: if CategorySlot present
Category->>Brow: transform CategorySlot -> BrowSlot
Migration0054->>Price: inject PriceSlot (default RTF) into card
Migration0054->>MigrationRunner: return transformed props with defaults
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ 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: 2
🤖 Fix all issues with AI agents
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx`:
- Around line 428-444: The truthiness checks in resolveData (for priceSlotProps
and browSlotProps) are using YextEntityField objects directly (via
priceSlotProps?.data.text and parentData.richText) which can be truthy even when
empty; update resolveData to extract actual values using resolveYextEntityField
(or by reading constantValue and validating richText content) before coercing to
Boolean, e.g., replace the current showPrice and showBrow checks with logic that
calls resolveYextEntityField on priceSlotProps.data.text and
browSlotProps.data.text (and checks for non-empty richText when using
parentData), following the same pattern used in EventCard.tsx so
empty/placeholder Yext fields correctly evaluate to false.
In `@packages/visual-editor/src/editor/TranslatableRichTextField.tsx`:
- Around line 106-113: The returned JSON/HTML embeds raw text causing invalid
JSON and XSS; in TranslatableRichTextField (the function building the lexical
payload and html string) replace manual string interpolation with a safe
approach: build the lexical payload as a JS object and JSON.stringify it for the
json field (instead of the template literal), and HTML-escape the text value
before inserting into the html field (ensuring quotes, ampersands, <, > are
escaped) while still applying fontWeight/tag based on isBold/format/tag
variables; this ensures valid JSON and prevents HTML injection.
🧹 Nitpick comments (5)
packages/visual-editor/src/components/pageSections/ProductSection/ProductSectionContext.tsx (1)
16-22: Remove leftover implementation notes from the interface.These comments appear to be internal development notes rather than documentation. They reference assumptions and reasoning that shouldn't be in the final code.
♻️ Proposed fix
export interface ProductSectionContextType { variant?: ProductSectionVariant; imageConstrain?: ProductSectionImageConstrain; showImage?: boolean; showBrow?: boolean; showTitle?: boolean; showPrice?: boolean; showDescription?: boolean; showCTA?: boolean; - cardBackgroundColor?: BackgroundStyle; // To pass section's card bg choice if needed? No, user didn't ask for that specifically, but hinted "styling for every card". - // Actually, ProductCard has a backgroundColor prop. ProductSection has a backgroundColor prop (for the section). - // The Prompt: "Changing the prop will change the styling for every card in the section." - // This refers to the VARIANT. - // But maybe the user assumes colors too? - // "The styling" implies the layout/variant. - // I will just pass the variant/visibility flags for now. + cardBackgroundColor?: BackgroundStyle; }packages/visual-editor/src/components/contentBlocks/image/Image.tsx (1)
87-97: Consider simplifying the field type.The
showImageConstrainfield is defined as a radio with boolean values, but it's functionally a toggle. This works correctly but a simplerbooleantype with a checkbox might be more intuitive for an internal/hidden field.That said, this follows the pattern used elsewhere in the codebase, so keeping it consistent is also valid.
packages/visual-editor/src/components/migrations/0052_product_variants.ts (1)
6-16: Consider preserving existing values if already set.The migration unconditionally sets
cardVariantandshowBrow, which would overwrite any existing values if the migration runs on data that already has these properties. If this is intentional (to normalize all existing ProductSections), this is fine. Otherwise, consider conditional assignment:♻️ Conditional assignment alternative
propTransformation: (props) => { const newProps = { ...props }; if (!newProps.styles) { newProps.styles = {}; } - newProps.styles.cardVariant = "immersive"; - newProps.styles.showBrow = false; + if (newProps.styles.cardVariant === undefined) { + newProps.styles.cardVariant = "immersive"; + } + if (newProps.styles.showBrow === undefined) { + newProps.styles.showBrow = false; + } return newProps; },Please confirm whether the intent is to:
- Normalize all existing ProductSections to these defaults (current behavior), or
- Only set defaults for sections that don't already have these values
packages/visual-editor/src/components/pageSections/ProductSection/ProductSection.tsx (1)
290-324: Consider simplifying the conditionalsetDeeplogic.The
if/elseblock forhideWidthPropcan be consolidated since both branches callsetDeepwith the same path but opposite boolean values.♻️ Suggested simplification
updatedData = setDeep( updatedData, `props.slots.CardsWrapperSlot[0].props.slots.CardSlot[${i}].props.slots.ImageSlot[0].props.showImageConstrain`, showImageConstrain ); - if (isImmersive) { - updatedData = setDeep( - updatedData, - `props.slots.CardsWrapperSlot[0].props.slots.CardSlot[${i}].props.slots.ImageSlot[0].props.hideWidthProp`, - true - ); - } else { - updatedData = setDeep( - updatedData, - `props.slots.CardsWrapperSlot[0].props.slots.CardSlot[${i}].props.slots.ImageSlot[0].props.hideWidthProp`, - false - ); - } + updatedData = setDeep( + updatedData, + `props.slots.CardsWrapperSlot[0].props.slots.CardSlot[${i}].props.slots.ImageSlot[0].props.hideWidthProp`, + isImmersive + );packages/visual-editor/src/editor/TranslatableRichTextField.tsx (1)
97-105: ExportGetDefaultRTFOptionsfrom the barrel for API consistency.Since
getDefaultRTFis re-exported frompackages/visual-editor/src/editor/index.tsbut its options typeGetDefaultRTFOptionsis not, consumers cannot easily reference the type when calling the function. Addtype GetDefaultRTFOptionsto the export fromTranslatableRichTextField.tsxin the barrel to keep the public surface consistent.
packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx
Outdated
Show resolved
Hide resolved
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/components/migrations/0053_product_variants.ts`:
- Around line 6-14: The migration's propTransformation currently mutates the
input and unconditionally overwrites styles; update propTransformation to
shallow-clone props and clone props.styles into a new object (e.g., const
newProps = { ...props, styles: { ...(props.styles || {}) } }) so you don't
mutate the original, and only assign defaults for cardVariant and showBrow when
they are undefined or null (e.g., if (newProps.styles.cardVariant == null)
newProps.styles.cardVariant = "immersive"; if (newProps.styles.showBrow == null)
newProps.styles.showBrow = false), leaving any existing values intact.
♻️ Duplicate comments (1)
packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx (1)
428-444: Truthiness checks can stay “true” even when slot content is empty.
BodyTextProps.data.textis aYextEntityField, soBoolean(...)will remaintrueeven if the rich text is empty. Consider resolving the actual value before checking.
packages/visual-editor/src/components/migrations/0054_product_variants.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Fix all issues with AI agents
In `@packages/visual-editor/locales/platform/cs/visual-editor.json`:
- Around line 380-382: Update the translation for the key "fillContainer" to
match the adjective+noun pattern used by related keys (e.g.,
"fullWidth"/"fixedWidth"); replace the current value "Naplňte nádobu" with a
consistent label such as "Plné vyplnění" (or another adjective+noun variant like
"Plná výplň") so UI labels are stylistically consistent.
In `@packages/visual-editor/locales/platform/fi/visual-editor.json`:
- Around line 476-477: The key "showBrow" currently uses the Finnish string
"Näytä kulmien teksti" which may not accurately convey the UI concept of an
"eyebrow"/overline label; please confirm the preferred Finnish term with the
localization team and update the value of "showBrow" in the JSON to the agreed
translation (e.g., a more accurate term like "Näytä ylätunniste" or the
localization team's choice) while leaving the key name showBrow unchanged;
ensure the new string matches tone and length of other keys such as
"showBusinessName".
- Around line 268-269: Replace the English value for the "imageConstrain" key in
the Finnish locale with a Finnish translation; update the string value for
imageConstrain to the appropriate Finnish text (e.g., "Näytä kuvarajoitus") so
it matches the rest of the Finnish translations and UI wording.
In `@packages/visual-editor/locales/platform/ja/visual-editor.json`:
- Around line 373-375: Replace the unnatural Japanese label for the
"fillContainer" key with a more natural UI copy—change the value of the
"fillContainer" JSON key from "コンテナに充填する" to a suggested alternative such as
"コンテナに合わせる" or "コンテナを満たす" (confirm preferred phrasing with the localization
team); ensure the same replacement is applied to the other occurrence of the
"fillContainer" key mentioned in the review.
In `@packages/visual-editor/locales/platform/lv/visual-editor.json`:
- Line 394: The label for the "minimal" variant uses the adverb "Minimāli";
change the value for the "minimal" key to the adjective form to match other
variant labels (e.g., replace "Minimāli" with "Minimāls") so it aligns with the
adjectives like "Klasisks" and "Ieskaujošs".
- Line 373: The translation for the key "fillContainer" uses an imperative
("Aizpildiet konteineru") but neighbors use adjectives/nouns; update the
"fillContainer" value to match the same part of speech as surrounding option
labels (e.g., use an adjective/descriptor or noun form such as "Aizpildīts" or a
short phrase like "Aizpildīts konteiners"/"Pietilpst konteinerā" depending on
context) and ensure wording is consistent with adjacent keys (review sibling
keys in the same JSON block) before committing.
In `@packages/visual-editor/locales/platform/nl/visual-editor.json`:
- Line 268: Replace the inconsistent Dutch term for "image constraint" in the
translation key "imageConstrain" so it matches the rest of the file (use
"Afbeeldingsbeperking" to align with "showImageConstrain" and other keys like
"imageHeight" and "imageUrl"); update the value for imageConstrain from
"Beeldbeperking" to "Afbeeldingsbeperking".
- Line 496: The translation for the "showTitle" key is inconsistent ("Titel
tonen")—update the value to match the other show* entries by changing
"showTitle" to "Toon titel" (use the same verb-first pattern and lowercase noun)
in the visual-editor.json so it aligns with keys like
showDescription/showPrice/showImage.
In `@packages/visual-editor/locales/platform/pl/visual-editor.json`:
- Around line 374-376: Update the translation value for the key "fillContainer"
from "Napełnij pojemnik" to "Wypełnij kontener" so it uses the same term
"kontener"/"kontenera" used elsewhere (e.g., "containerAlignment") and aligns
with existing localization conventions; modify the value for the "fillContainer"
entry only, leaving "fixed" and "fixedWidth" untouched.
- Around line 477-497: The translation for the key "showBrow" is non-idiomatic;
update the value for "showBrow" in visual-editor.json to use the standard Polish
UI term "Pokaż nadtytuł" (replace the current "Pokaż tekst brwi") so the UI
displays the correct localized label.
In `@packages/visual-editor/locales/platform/pt/visual-editor.json`:
- Line 476: The current translation value for the key "showBrow" is too literal;
update the JSON entry for the "showBrow" key to use a more idiomatic Portuguese
UI string such as "Mostrar sobretítulo" (or "Mostrar texto de sobretítulo" if
you prefer the longer form), and ensure any related keys using "brow" are
updated for consistency in the same locale file.
- Around line 373-375: Update the Portuguese translations for the keys
"fillContainer", "fixed", and "fixedWidth" to follow standard UI localization:
change the value for fillContainer from "Encher recipiente" to "Preencher
contêiner", change fixed from "Fixo" to sentence case "Fixo" is acceptable but
confirm context (if used as adjective in UI label, keep "Fixo"; if used as
verb/menu item, use "Fixar"), and change fixedWidth from "Largura Fixa" to
sentence case "Largura fixa"; ensure the three keys in visual-editor.json are
replaced accordingly to use "Preencher contêiner" and "Largura fixa" and adjust
"fixed" based on its UI role.
🧹 Nitpick comments (5)
packages/visual-editor/locales/platform/fr/visual-editor.json (1)
475-475: Consider alternative terminology for "brow text".The translation "texte du front" is a literal interpretation of "brow" (forehead). In French UI contexts, "brow text" (eyebrow text above a heading) is sometimes rendered as "surtitre" or "accroche". If this term is already established in your product's French localization, please disregard.
packages/visual-editor/locales/platform/es/visual-editor.json (1)
475-475: Consider clarifying the "brow" translation."Brow" in UI terminology typically refers to "eyebrow text" — a small line of text positioned above a main heading. The current translation "texto de frente" (front text) may not convey this meaning clearly to Spanish-speaking users. Consider alternatives like "Mostrar sobretítulo" or "Mostrar texto superior" for better clarity.
packages/visual-editor/locales/platform/en/visual-editor.json (1)
267-267: Consider "Image Constraint" for grammatical correctness."Constrain" is a verb, while "Constraint" is the noun form. "Image Constraint" and "Show Image Constraint" would be more grammatically natural.
📝 Suggested terminology adjustment
- "imageConstrain": "Image Constrain", + "imageConstrain": "Image Constraint",- "showImageConstrain": "Show Image Constrain", + "showImageConstrain": "Show Image Constraint",Also applies to: 486-486
packages/visual-editor/locales/platform/zh-TW/visual-editor.json (1)
202-202: Consider using "卡片變體" for more standard UI terminology."卡牌" typically refers to playing cards, while "卡片" is the more common term for UI card components in Traditional Chinese. This is a minor terminology preference.
Suggested change
- "cardVariant": "卡牌變體", + "cardVariant": "卡片變體",packages/visual-editor/locales/platform/lt/visual-editor.json (1)
373-375: Consider aligning "fillContainer" wording with other option styles.Most options are nouns/adjectives (e.g., "Fiksuotas", "Minimalus"), but "Užpildykite konteinerį" is imperative. Consider using an infinitive or noun phrase for consistency (e.g., "Užpildyti konteinerį" or similar).
Possible wording tweak
- "fillContainer": "Užpildykite konteinerį", + "fillContainer": "Užpildyti konteinerį",Also applies to: 394-394
packages/visual-editor/src/components/pageSections/ProductSection/ProductSectionContext.tsx
Outdated
Show resolved
Hide resolved
...mponents/testing/screenshots/ProductSection/[desktop] version 7 props with entity values.png
Show resolved
Hide resolved
packages/visual-editor/src/components/migrations/0054_product_variants.ts
Show resolved
Hide resolved
packages/visual-editor/src/components/migrations/0054_product_variants.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx`:
- Around line 430-456: The visibility logic in resolveData currently only checks
slot props (priceSlotProps/browSlotProps) which can be unset before parentData
is attached, causing fields to be hidden; update the showPrice and showBrow
computation to first check for data.props.parentData (e.g.,
data.props.parentData.product or parentData.richText) and derive visibility from
those product fields when present, falling back to the existing slot-based
checks (priceSlotProps.data.text via resolveYextEntityField and
browSlotProps.data.text) only if parentData is not available; modify the boolean
expressions for showPrice and showBrow in resolveData to prefer parentData
values before using priceSlotProps/browSlotProps.
♻️ Duplicate comments (1)
packages/visual-editor/locales/platform/nl/visual-editor.json (1)
268-268: Align “imageConstrain” terminology with other image labels.
This still uses “Beeldbeperking” while related labels use “Afbeeldings…”. Consider aligning for consistency.Suggested fix
- "imageConstrain": "Beeldbeperking", + "imageConstrain": "Afbeeldingsbeperking",
packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx
Outdated
Show resolved
Hide resolved
...mponents/testing/screenshots/ProductSection/[mobile] version 32 props with entity values.png
Show resolved
Hide resolved
…sual-editor into product-section-variants
|
Unfortunately the photo gallery changes took migration 53, LGTM otherwise |
...omponents/testing/screenshots/ProductSection/[mobile] version 7 props with entity values.png
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx`:
- Around line 513-514: Remove the debug console logging in the resolveData path:
delete the console.log("resolvedCTA", resolvedCTA) call so that resolvedCTA and
showCTA (Boolean(resolvedCTA)) are computed without leaking data to the console
in editor/preview sessions; if you need structured debugging, use a conditional
debug logger behind an env flag or developer-only utility instead of console.log
in the ProductCard/resolveData logic.
- Around line 440-442: The CTA is never rendered because the expression includes
a hardcoded "&& false"; update the render condition in ProductCard (the JSX that
checks conditionalRender?.cta and showCTA before rendering slots.CTASlot) by
removing the "&& false" so the CTA is shown when conditionalRender?.cta &&
showCTA are true; ensure the existing props (style={{ height: "auto" }} and
allow={[]}) on slots.CTASlot remain unchanged.
packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/pageSections/ProductSection/ProductCard.tsx
Outdated
Show resolved
Hide resolved
...omponents/testing/screenshots/ProductSection/[mobile] version 0 props with entity values.png
Show resolved
Hide resolved
4 participants
This adds new Show/Hide props for each slot, as well as a Variant props that allows the user to switch between Immersive, Classic, and Minimal.
Mocks: https://www.figma.com/design/sX3bkzkEpQ3g2mJrfSSdmd/Quickstart-Template--2025-?node-id=18773-227889&m=dev
Screen.Recording.2026-01-20.at.8.20.15.PM.mov