feat: add StyledPlainText and StyledRichText#1257
Conversation
|
Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information. |
auto-screenshot-update: true
jwartofsky-yext
changed the base branch from
main
to
2026-custom-components-templates
benlife5
left a comment
benlife5
left a comment
There was a problem hiding this comment.
General idea LGTM! I do think will help make template generation more straightforward.
I'd tend towards using the YextFields json syntax for defining the fields in the components rather than using createStyledTextConfig => headingConfig.fields. We were originally trying to move away from using functions/variable references in the fields for some of the Puck-in-Storm stuff, but I suppose that doesn't matter anymore. I still prefer the fields to be more explicit and understandable, but I get that createStyledTextConfig makes it simpler... so I'm good either way.
My only other note is that the more stuff we move into the library, the more we need to handle visual-editor versioning and backwards compatibility, but that was going to be important anyway
jwartofsky-yext
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds shared styled-text rendering helpers, a new Sequence Diagram(s)sequenceDiagram
participant Puck
participant StyledTextComponent
participant resolveComponentData
participant StyledTextElement
participant renderStyledRichText
Puck->>StyledTextComponent: render kind and entity props
StyledTextComponent->>resolveComponentData: resolve document fields
resolveComponentData-->>StyledTextComponent: resolved content or empty result
alt plain text
StyledTextComponent->>StyledTextElement: render plain text
else rich text
StyledTextComponent->>renderStyledRichText: render resolved rich content
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (3)
packages/visual-editor/src/components/contentBlocks/styledText.tsx (1)
98-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider deduplicating
getStyledRichTextStyleagainstgetStyledTextStyle.The five
if (text && text.xxx !== "default")blocks mirror the logic already ingetStyledTextStyle(lines 39-53), just mapping to CSS-variable keys instead of standard style props. A small key-mapping table iterated once would avoid maintaining two parallel copies of the same "skip default" logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/components/contentBlocks/styledText.tsx` around lines 98 - 133, getStyledRichTextStyle duplicates the same “skip default” checks already implemented in getStyledTextStyle, so refactor it to reuse that logic via a small key-mapping table or shared helper. Update the getStyledRichTextStyle function to iterate over the text style fields once and assign the corresponding CSS variable keys instead of keeping five separate if blocks. Keep the existing behavior for color handling and default-value skipping, but make the mapping between StyledTextValue properties and the CSS custom properties explicit and centralized.packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx (1)
47-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
tagOptionsis accepted forrichTextkind but silently ignored.
CreateStyledTextConfigOptions.tagOptionsisn't restricted tokind: "plain", so callers of therichTextoverload can passtagOptionsand it will be silently dropped bybuildFields(line 118:kind === "plain" && tagOptions?.length). A discriminated union keyed onkindwould surface this at the type level instead of failing silently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx` around lines 47 - 58, The CreateStyledTextConfigOptions type currently allows tagOptions on both plain and richText, but buildFields only uses it for plain and silently drops it for richText. Refactor the options type into a discriminated union keyed on kind so tagOptions is only valid when kind is plain, and update CreateStyledTextConfig/buildFields to rely on that narrowed shape. This should make callers of the richText path fail at compile time instead of passing ignored tagOptions.packages/visual-editor/src/fields/styledFields/StyledTextField.tsx (1)
91-106: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueColor selector uses derived
currentTextValueinstead of the raw groupedtext.
getGroupedStyledTextValue(value, currentTextValue)rebuilds thetextpayload from the normalizedcurrentTextValue(which merges indefaultBaseTextStylesfor any missing key) rather than reusingvalue.textdirectly. SinceStyledTextValuerequires allBaseTextStyleskeys, this is a no-op today, but it's worth being aware of as a subtle behavior difference if the type ever gains optional fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/fields/styledFields/StyledTextField.tsx` around lines 91 - 106, The color selector in StyledTextField is rebuilding the grouped value from currentTextValue, which can subtly normalize the text payload instead of preserving the original grouped text. Update the onChange path in StyledTextField so the color update reuses the raw grouped text from value.text (or otherwise preserves the existing grouped text object) rather than passing currentTextValue into getGroupedStyledTextValue, keeping the selector behavior aligned with the existing StyledTextValue shape.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/visual-editor/src/components/contentBlocks/styledText.tsx`:
- Around line 174-196: The rich-text typography styles are being applied to
content.props.children instead of the actual wrapper rendered by MaybeRTF, so
the .rtf-wrapper div never receives the font custom properties. Update
styledText’s cloneElement flow to apply richTextStyle to the cloned content
wrapper itself (or thread it through MaybeRTF) and keep the inner content
handling only for nested elements. Use the styledText block and MaybeRTF as the
key references when adjusting where className and style are merged.
In `@packages/visual-editor/src/utils/i18n/distance.ts`:
- Around line 59-73: The validation in toCoordinate currently accepts NaN and
Infinity because it only checks typeof on latitude and longitude. Update
toCoordinate in the distance utility to also require finite numeric values
before returning a GeoCoordinate, and otherwise return undefined so malformed
coordinates are treated like missing data. Keep the fix localized to
toCoordinate and ensure the downstream getDistance and NearbyLocations sorting
logic only receives valid finite coordinates.
---
Nitpick comments:
In
`@packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx`:
- Around line 47-58: The CreateStyledTextConfigOptions type currently allows
tagOptions on both plain and richText, but buildFields only uses it for plain
and silently drops it for richText. Refactor the options type into a
discriminated union keyed on kind so tagOptions is only valid when kind is
plain, and update CreateStyledTextConfig/buildFields to rely on that narrowed
shape. This should make callers of the richText path fail at compile time
instead of passing ignored tagOptions.
In `@packages/visual-editor/src/components/contentBlocks/styledText.tsx`:
- Around line 98-133: getStyledRichTextStyle duplicates the same “skip default”
checks already implemented in getStyledTextStyle, so refactor it to reuse that
logic via a small key-mapping table or shared helper. Update the
getStyledRichTextStyle function to iterate over the text style fields once and
assign the corresponding CSS variable keys instead of keeping five separate if
blocks. Keep the existing behavior for color handling and default-value
skipping, but make the mapping between StyledTextValue properties and the CSS
custom properties explicit and centralized.
In `@packages/visual-editor/src/fields/styledFields/StyledTextField.tsx`:
- Around line 91-106: The color selector in StyledTextField is rebuilding the
grouped value from currentTextValue, which can subtly normalize the text payload
instead of preserving the original grouped text. Update the onChange path in
StyledTextField so the color update reuses the raw grouped text from value.text
(or otherwise preserves the existing grouped text object) rather than passing
currentTextValue into getGroupedStyledTextValue, keeping the selector behavior
aligned with the existing StyledTextValue shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6f84532-0c23-4642-a8b8-a6ae77fe7cec
⛔ Files ignored due to path filters (1)
packages/visual-editor/src/components/testing/screenshots/PhotoGallerySection/[desktop] version 59 with showSectionHeading false.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (15)
packages/visual-editor/src/components/contentBlocks/StyledPlainText.tsxpackages/visual-editor/src/components/contentBlocks/StyledRichText.tsxpackages/visual-editor/src/components/contentBlocks/StyledText.test.tsxpackages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsxpackages/visual-editor/src/components/contentBlocks/index.tspackages/visual-editor/src/components/contentBlocks/styledText.tsxpackages/visual-editor/src/components/pageSections/NearbyLocations/utils.tspackages/visual-editor/src/fields/styledFields/StyledTextField.test.tsxpackages/visual-editor/src/fields/styledFields/StyledTextField.tsxpackages/visual-editor/src/utils/colors.test.tspackages/visual-editor/src/utils/colors.tspackages/visual-editor/src/utils/i18n/distance.test.tspackages/visual-editor/src/utils/i18n/distance.tspackages/visual-editor/src/utils/i18n/index.tspackages/visual-editor/src/utils/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/utils/colors.ts (1)
597-634: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate fallback-merging logic between the two branches.
The per-key fallback merge (backgroundColor/color) is written twice — once inside the "resolved present" branch and once inside the "resolved absent" branch. These can be unified into a single expression guarded by one early-return check, reducing duplication.
♻️ Suggested consolidation
- if (backgroundColor || foregroundColor) { - return { - ...(backgroundColor - ? { backgroundColor } - : options?.fallbackBackgroundColor - ? { backgroundColor: options.fallbackBackgroundColor } - : {}), - ...(foregroundColor - ? { color: foregroundColor } - : options?.fallbackTextColor - ? { color: options.fallbackTextColor } - : {}), - }; - } - - if (!options?.fallbackBackgroundColor && !options?.fallbackTextColor) { - return undefined; - } - - return { - ...(options?.fallbackBackgroundColor - ? { backgroundColor: options.fallbackBackgroundColor } - : {}), - ...(options?.fallbackTextColor ? { color: options.fallbackTextColor } : {}), - }; + if ( + !backgroundColor && + !foregroundColor && + !options?.fallbackBackgroundColor && + !options?.fallbackTextColor + ) { + return undefined; + } + + return { + ...(backgroundColor + ? { backgroundColor } + : options?.fallbackBackgroundColor + ? { backgroundColor: options.fallbackBackgroundColor } + : {}), + ...(foregroundColor + ? { color: foregroundColor } + : options?.fallbackTextColor + ? { color: options.fallbackTextColor } + : {}), + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/utils/colors.ts` around lines 597 - 634, The getSurfaceColorStyle function repeats the same fallback merge for backgroundColor and color in both branches, so consolidate it into one shared return path. Keep the initial resolution of backgroundColor and foregroundColor via getThemeColorCssValue and getDefaultForegroundColor, then use a single early return for the no-style/no-fallback case and one unified object build that applies resolved values first and falls back to options.fallbackBackgroundColor and options.fallbackTextColor when needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/visual-editor/src/utils/colors.ts`:
- Around line 597-634: The getSurfaceColorStyle function repeats the same
fallback merge for backgroundColor and color in both branches, so consolidate it
into one shared return path. Keep the initial resolution of backgroundColor and
foregroundColor via getThemeColorCssValue and getDefaultForegroundColor, then
use a single early return for the no-style/no-fallback case and one unified
object build that applies resolved values first and falls back to
options.fallbackBackgroundColor and options.fallbackTextColor when needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 00a32ea6-c92c-488d-bf5d-3a6d3b17cb94
📒 Files selected for processing (2)
packages/visual-editor/src/utils/colors.test.tspackages/visual-editor/src/utils/colors.ts
jwartofsky-yext
changed the title
jwartofsky-yext
requested review from
asanehisa,
benlife5,
briantstephan and
mkilpatrick
auto-screenshot-update: true
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/components/contentBlocks/styledText.tsx (1)
194-199: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead no-op clone:
innerContentdoesn't change anything.
React.cloneElement(content.props.children, { className: themeManagerCn(content.props.children.props.className), style: content.props.children.props.style })passes through the exact sameclassName/stylevalues it already had — this clone has no effect and can be simplified to justcontent.props.children.♻️ Proposed simplification
- const innerContent = React.isValidElement(content.props.children) - ? React.cloneElement(content.props.children, { - className: themeManagerCn(content.props.children.props.className), - style: content.props.children.props.style, - }) - : content.props.children; - - return React.cloneElement(content, { + return React.cloneElement(content, { className: themeManagerCn(content.props.className, richTextClassName), style: { ...content.props.style, ...richTextStyle, }, - children: innerContent, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/components/contentBlocks/styledText.tsx` around lines 194 - 199, The innerContent branch in styledText.tsx is a no-op because the React.cloneElement call in the content.props.children path only reuses the existing className and style without changing them. Simplify the React.isValidElement(content.props.children) logic by removing the clone and using content.props.children directly, keeping the surrounding themeManagerCn and styledText content handling intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/visual-editor/src/components/contentBlocks/styledText.tsx`:
- Around line 194-199: The innerContent branch in styledText.tsx is a no-op
because the React.cloneElement call in the content.props.children path only
reuses the existing className and style without changing them. Simplify the
React.isValidElement(content.props.children) logic by removing the clone and
using content.props.children directly, keeping the surrounding themeManagerCn
and styledText content handling intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6738358e-7d04-49c3-ab02-e0acfd78ebe4
📒 Files selected for processing (4)
packages/visual-editor/src/components/atoms/maybeRTF.tsxpackages/visual-editor/src/components/contentBlocks/StyledText.test.tsxpackages/visual-editor/src/components/contentBlocks/styledText.tsxpackages/visual-editor/src/utils/richTextStyles.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/contentBlocks/StyledText.test.tsx
…yledTextComponent
auto-screenshot-update: true
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx (1)
130-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused
puckprop typing left over after removing editing-placeholder logic.
StyledTextComponenttypespuck?: { isEditing: boolean }(Lines 135-137) but never destructures or references it in the body (130-195). The tworenderwrappers (Line 232, Line 255) still intersect their prop types with{ puck: { isEditing: boolean } }for the same unused purpose. Per the library context, thepuck.isEditing-gated editing placeholder was intentionally removed, so this typing is now dead weight that could mislead future readers into thinking editing-mode behavior is still handled here. Puck injectspuckintorenderautomatically regardless of the declared type, so dropping it here is safe.♻️ Proposed cleanup
export const StyledTextComponent = < TText extends TranslatableString | TranslatableRichText, >( props: StyledTextConfigProps<TText> & { kind: CreateStyledTextConfigOptions["kind"]; - puck?: { - isEditing: boolean; - }; } ) => { const { data, alignment, fontOptions, kind } = props;- render: ( - props: StyledPlainTextProps & { puck: { isEditing: boolean } } - ) => <StyledTextComponent {...props} kind="plain" />, + render: (props: StyledPlainTextProps) => ( + <StyledTextComponent {...props} kind="plain" /> + ),- render: (props: StyledRichTextProps & { puck: { isEditing: boolean } }) => ( - <StyledTextComponent {...props} kind="richText" /> - ), + render: (props: StyledRichTextProps) => ( + <StyledTextComponent {...props} kind="richText" /> + ),Also applies to: 232-232, 232-232, 255-255, 255-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx` around lines 130 - 140, Remove the leftover editing-mode typing from StyledTextComponent: the local props type still declares puck?: { isEditing: boolean }, but StyledTextComponent never uses it after the placeholder logic was removed. Update the StyledTextComponent prop signature and the two render wrappers that intersect their props with { puck: { isEditing: boolean } } so they no longer mention puck at all, while keeping the component logic in StyledTextComponent and the render call sites otherwise unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsx`:
- Around line 130-140: Remove the leftover editing-mode typing from
StyledTextComponent: the local props type still declares puck?: { isEditing:
boolean }, but StyledTextComponent never uses it after the placeholder logic was
removed. Update the StyledTextComponent prop signature and the two render
wrappers that intersect their props with { puck: { isEditing: boolean } } so
they no longer mention puck at all, while keeping the component logic in
StyledTextComponent and the render call sites otherwise unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e6e8ed24-f98a-4210-ae3a-d8c5cc70f19a
📒 Files selected for processing (3)
packages/visual-editor/src/components/contentBlocks/StyledText.test.tsxpackages/visual-editor/src/components/contentBlocks/createStyledTextConfig.tsxpackages/visual-editor/src/components/contentBlocks/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/visual-editor/src/components/contentBlocks/StyledText.test.tsx
jwartofsky-yext
requested review from
benlife5 and
mkilpatrick
and removed request for
mkilpatrick
jwartofsky-yext
merged commit ccbb6a9
into
2026-custom-components-templates
3 participants
Styled Text API Updates
Summary
This PR updates the styled text API documentation to match the current public surface area.
createStyledTextConfigremains the entry point for building styled text field configsStyledTextComponentis now the shared renderer for both plain text and rich textStyledPlainTextandStyledRichTextare no longer exported render componentsStyledPlainTextPropsandStyledRichTextPropsare still exported for typingtagOptionsonly applies tokind: "plain"includeColorandincludeAlignmentare still supportedUpdated Usage
Create config
Plain text
Render one plain-text instance. It owns its own
data.text.Shared plain-text styling across repeated items
Rich text
Shared rich-text styling across repeated items
StyledPlainTextPropsUse this when you want to type a prop or local variable representing one plain-text instance.
StyledRichTextPropsSame for rich text.