Conversation
commit: |
briantstephan
requested review from
asanehisa,
benlife5,
jwartofsky-yext and
mkilpatrick
|
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 pull request adds localization keys across 23 language locale files in the visual-editor package (richText, text, fontStyle, italic, regular, textSize) and introduces supporting code changes. A new Text content block component is created to replace BodyText usage in multiple card components (InsightCard, ProductCard, TeamCard, FAQCard). The FAQCard data model is updated to accept both plain text and rich text for question fields. A migration module (0062_text_atom_updates) is added to transform existing text-related data during schema updates. Text and TextSlot are added to the AdvancedCoreInfoCategory and SlotsCategory respectively. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🤖 Fix all issues with AI agents
In `@packages/visual-editor/locales/platform/pl/visual-editor.json`:
- Line 409: The translation for the "italic" key is incorrect; update the JSON
entry with the proper Polish term by replacing the value for the "italic" key in
visual-editor.json (the "italic" property) from "italski" to "Kursywa" so the UI
displays the correct Polish typography term.
🧹 Nitpick comments (7)
packages/visual-editor/locales/platform/zh/visual-editor.json (1)
426-426: Minor: consider using "常规" instead of "常规的" for "Regular".In Chinese UI labels, "常规" (without the adjectival suffix "的") is more conventional for font style options, matching typical usage in applications like Microsoft Office and Adobe products.
Suggested tweak
- "regular": "常规的", + "regular": "常规",packages/visual-editor/src/components/contentBlocks/Text.tsx (1)
44-80: Consider narrowing theanytype parameter.Line 48 uses
YextField<any, ...>. While likely a framework constraint, using a more specific type (or at leastunknown) would improve type safety.Suggested change
- text: YextField<any, TranslatableString | TranslatableRichText>( + text: YextField<unknown, TranslatableString | TranslatableRichText>(packages/visual-editor/src/utils/plainText.ts (3)
11-25: Static analysis flag forinnerHTMLis a false positive here — but the SSR fallback is incomplete.Using
textarea.innerHTMLfor entity decoding is a well-known safe pattern (detached textarea never executes scripts). The static analysis warning (CWE-79) doesn't apply.However, the SSR fallback (lines 18–24) only handles 6 named entities. Numeric entities (e.g.,
—) and other named entities (e.g.,—,…) will pass through un-decoded in SSR environments. This is acceptable if rich text content in practice only uses common entities, but worth noting.Consider adding numeric entity decoding for the SSR fallback
return value .replace(/ /gi, " ") .replace(/&/gi, "&") .replace(/</gi, "<") .replace(/>/gi, ">") .replace(/"/gi, '"') - .replace(/'/gi, "'"); + .replace(/'/gi, "'") + .replace(/&#(\d+);/g, (_, dec) => String.fromCharCode(Number(dec))) + .replace(/&#x([0-9a-f]+);/gi, (_, hex) => String.fromCharCode(parseInt(hex, 16)));
73-98: RedundanthasLocalizedValuehandling.
hasLocalizedValueis already set inside thereducecallback (line 75–77). The second check at lines 94–96 sets the same property again with the same value. You can safely remove one of the two.Remove redundant check
const localizedValues = Object.entries(value).reduce( (acc, [key, localizedValue]) => { if (key === "hasLocalizedValue") { acc.hasLocalizedValue = "true"; return acc; } if ( typeof localizedValue === "string" || isRichText(localizedValue) || localizedValue === undefined || localizedValue === null ) { acc[key] = richTextToPlainText(localizedValue ?? ""); } return acc; }, {} as Record<string, string> ); - if ("hasLocalizedValue" in value) { - localizedValues.hasLocalizedValue = "true"; - } - return localizedValues as TranslatableString;
7-9:isRichTextcould match any object with anhtmlproperty.The type guard only checks for the presence of an
htmlkey. Any object with anhtmlproperty (e.g.,{ html: 42 }) would pass. In this codebase the inputs are typed, so this is unlikely to be a practical issue, but a tighter check could add safety.packages/visual-editor/src/components/pageSections/InsightSection/InsightCard.tsx (1)
575-583: Stale type assertion — minor nit.The cast
as YextEntityField<TranslatableRichText>on line 577 is narrower than the actualTextProps["data"]["text"]type, which isYextEntityField<TranslatableString | TranslatableRichText>. It's harmless sinceresolveComponentDataaccepts both, but updating the assertion would be more accurate.✏️ Suggested type alignment
const category = resolveComponentData( updatedData.props.slots.CategorySlot[0]?.props.data - .text as YextEntityField<TranslatableRichText>, + .text as YextEntityField<TranslatableString | TranslatableRichText>, locale,packages/visual-editor/src/utils/resolveComponentData.tsx (1)
307-317:toPlainTextStringsilently returns""for non-primitive values.If
resolveTranslatableTypeToPlainTextever yields an unresolved object (e.g., a translatable structure with an unexpected shape), this will silently produce an empty string. For the current Text-atom callers (category, price, title, question) this is a safe fallback, but it could mask data-resolution bugs in future use sites. Consider logging a dev-mode warning when the value is a non-null object so issues surface during development.💡 Optional: add a dev-mode warning
function toPlainTextString(value: any): string { if (typeof value === "string") { return value; } if (typeof value === "number" || typeof value === "boolean") { return String(value); } + if (process.env.NODE_ENV === "development" && value != null) { + console.warn( + "toPlainTextString received a non-primitive value; returning empty string.", + value + ); + } + return ""; }
...ponents/testing/screenshots/ProductSection/[desktop] version 32 props with static values.png
Show resolved
Hide resolved
packages/visual-editor/src/components/migrations/0062_text_atom_updates.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In `@packages/visual-editor/locales/platform/cs/visual-editor.json`:
- Line 432: The translation for the "regular" key is incorrect — update the
value of the "regular" JSON key in visual-editor.json (the "regular" entry) to a
proper Czech term for font style, e.g., replace "Pravidelný" with "Běžný" or
"Normální" so the font style reads correctly in Czech; ensure you only change
the string value for the "regular" key and keep JSON formatting intact.
- Line 414: The "italic" translation value is lowercased while the corresponding
"bold" key uses sentence-style capitalization; update the "italic" entry (key
"italic") from "kurzíva" to the properly capitalized Czech form "Kurzíva" so it
matches the capitalization style used by the "bold"/"Tučné" translation.
In `@packages/visual-editor/locales/platform/da/visual-editor.json`:
- Line 269: The translation for the "fontStyle" key is incorrect—replace its
value "Skrifttype" with the more accurate Danish "Skriftstil" to reflect "Font
Style" (not "Font/Typeface"); update the "fontStyle" entry in
packages/visual-editor/locales/platform/da/visual-editor.json and ensure
consistency with the existing "fontSize" → "Skrifttypestørrelse" usage.
- Line 433: The Danish translation for the "regular" key is incorrect—replace
the value "Fast" with the correct Danish word for the font style (e.g.,
"Normal") by updating the JSON entry for the "regular" key so it reads
"regular": "Normal" (ensuring proper quotes/JSON formatting) to reflect the
intended "Regular" font style.
In `@packages/visual-editor/locales/platform/es/visual-editor.json`:
- Line 407: Update the Spanish translation value for the "italic" key in
visual-editor.json: replace "Itálico" with the standard UI term "Cursiva" for
the "italic" string to align with common Spanish localization conventions.
In `@packages/visual-editor/locales/platform/fi/visual-editor.json`:
- Line 426: The Finnish translation for the "regular" font-style key currently
set as "Säännöllinen" is incorrect; update the value for the JSON key "regular"
in packages/visual-editor/locales/platform/fi/visual-editor.json to a
font-appropriate term such as "Normaali" (or "Tavallinen") so the UI shows the
correct font-style label; locate the "regular" key in the file and replace its
string value accordingly.
In `@packages/visual-editor/locales/platform/hr/visual-editor.json`:
- Line 408: The translation for the JSON key "italic" uses lowercase "kurziv"
while "bold" uses capitalized "Podebljano"; update the value for the "italic"
key to "Kurziv" so casing is consistent with the "bold" entry and other
font-style labels in visual-editor.json.
- Line 426: Update the Croatian locale entry for the font style key "regular" in
visual-editor.json: replace the current value "Redovno" with "Obično" so the
"regular" translation uses the correct font-style terminology; ensure the change
preserves JSON formatting and encoding (accented character) to match the
surrounding entries like "bold" and "italic".
In `@packages/visual-editor/locales/platform/nb/visual-editor.json`:
- Line 426: The translation for the "regular" key currently uses "Regelmessig",
which is incorrect for a font-style label; update the value for the "regular"
key in packages/visual-editor/locales/platform/nb/visual-editor.json to the
typography-appropriate term "Normal" (matching the existing "normal": "Normal"
entry) so the font-style labels are consistent and accurate.
In `@packages/visual-editor/locales/platform/pt/visual-editor.json`:
- Line 408: The "italic" translation value is incorrectly lowercase; update the
value for the "italic" key to use an initial capital letter ("Itálico") so it
matches the capitalization convention used by other option values (e.g.,
"Audacioso", "Regular") in the same locale file; locate the "italic" JSON entry
and change its string to "Itálico".
In `@packages/visual-editor/locales/platform/sv/visual-editor.json`:
- Line 427: The translation for the JSON key "regular" is incorrect—replace the
value "Regelbunden" with the correct font-style term "Normal" (or "Regular" if
you prefer the loanword) in the object containing the "regular" key so the label
reflects the font-weight sense rather than "periodic/recurring".
🧹 Nitpick comments (1)
packages/visual-editor/locales/platform/sv/visual-editor.json (1)
133-133: "Rich Text" is left untranslated.This may be intentional since "Rich Text" is a widely recognized technical term often kept in English in Swedish UIs. If the intent is to translate all user-facing strings, consider using "Rik text" or "Formaterad text." Otherwise, no action needed.
...onents/testing/screenshots/ProductSection/[mobile] version 15 props with missing ctaType.png
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/visual-editor/locales/platform/fi/visual-editor.json`:
- Line 133: The "richText" label in the Finnish locale file is left
untranslated; update the JSON entry for the "richText" key in
packages/visual-editor/locales/platform/fi/visual-editor.json to use a Finnish
translation (e.g., "Muotoiltu teksti") to match other translated component
labels, or leave a comment/NOTE if the term must remain English across locales;
ensure the updated value remains a valid JSON string.
In `@packages/visual-editor/locales/platform/lv/visual-editor.json`:
- Line 426: The localization string for the font-style label uses the adverb
"Regulāri" but should be an adjective; update the value for the "regular" key in
packages/visual-editor/locales/platform/lv/visual-editor.json from "Regulāri" to
an adjectival form such as "Parasts" so it reads correctly alongside
"Bold"/"Italic" in the UI.
In `@packages/visual-editor/locales/platform/sk/visual-editor.json`:
- Line 427: The "regular" translation value in the JSON locale is incorrect for
font-style context; update the value of the "regular" key in
packages/visual-editor/locales/platform/sk/visual-editor.json (the "regular"
string) from "Pravidelné" to a proper font-style term such as "Normálne" (or
"Bežné") so it aligns with the existing "bold"/"Tučné" and "italic"/"kurzíva"
entries.
- Line 409: The JSON entry for the "italic" label is inconsistently lowercased;
update the value for the "italic" key in visual-editor.json from "kurzíva" to
"Kurzíva" so it matches the capitalization style used by other option values
(e.g., "bold": "Tučné") and keep consistent localization formatting for UI
labels.
| "professionalHeroSection": "Ammattimainen sankariosasto", | ||
| "promoSection": "Promoosio", | ||
| "reviewsSection": "Arvosteluosa", | ||
| "richText": "Rich Text", |
There was a problem hiding this comment.
"Rich Text" is left untranslated in the Finnish locale.
Other component labels in this file are translated (e.g., "bodyText": "Kehon teksti" on line 79). Consider using a Finnish translation such as "Muotoiltu teksti" for consistency, unless "Rich Text" is intentionally kept as-is across all locales.
🤖 Prompt for AI Agents
In `@packages/visual-editor/locales/platform/fi/visual-editor.json` at line 133,
The "richText" label in the Finnish locale file is left untranslated; update the
JSON entry for the "richText" key in
packages/visual-editor/locales/platform/fi/visual-editor.json to use a Finnish
translation (e.g., "Muotoiltu teksti") to match other translated component
labels, or leave a comment/NOTE if the term must remain English across locales;
ensure the updated value remains a valid JSON string.
| "normal": "Normāls", | ||
| "one": "Viens", | ||
| "phone": "Tālrunis", | ||
| "regular": "Regulāri", |
There was a problem hiding this comment.
"Regulāri" is an adverb ("regularly"); use the adjective form for a UI label.
In the context of a font style selector (alongside "Bold" / "Italic"), the label should be an adjective. "Regulāri" means "regularly" (adverb). Consider "Parasts" (meaning "regular/normal") which is the masculine nominative adjective form and more natural for a button label describing a font style.
Proposed fix
- "regular": "Regulāri",
+ "regular": "Parasts",📝 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.
| "regular": "Regulāri", | |
| "regular": "Parasts", |
🤖 Prompt for AI Agents
In `@packages/visual-editor/locales/platform/lv/visual-editor.json` at line 426,
The localization string for the font-style label uses the adverb "Regulāri" but
should be an adjective; update the value for the "regular" key in
packages/visual-editor/locales/platform/lv/visual-editor.json from "Regulāri" to
an adjectival form such as "Parasts" so it reads correctly alongside
"Bold"/"Italic" in the UI.
| "includeOpenNow": "Zahrňte tlačidlo Otvorené teraz", | ||
| "international": "Medzinárodný", | ||
| "international_phone number": "Medzinárodné", | ||
| "italic": "kurzíva", |
There was a problem hiding this comment.
Inconsistent capitalization: "kurzíva" should be "Kurzíva".
Other option values in this file are capitalized (e.g., line 340 "bold": "Tučné", line 339 "base_text size": "Základňa"). This should follow the same pattern.
Proposed fix
- "italic": "kurzíva",
+ "italic": "Kurzíva",📝 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.
| "italic": "kurzíva", | |
| "italic": "Kurzíva", |
🤖 Prompt for AI Agents
In `@packages/visual-editor/locales/platform/sk/visual-editor.json` at line 409,
The JSON entry for the "italic" label is inconsistently lowercased; update the
value for the "italic" key in visual-editor.json from "kurzíva" to "Kurzíva" so
it matches the capitalization style used by other option values (e.g., "bold":
"Tučné") and keep consistent localization formatting for UI labels.
| "normal": "Normálne", | ||
| "one": "Jeden", | ||
| "phone": "Telefón", | ||
| "regular": "Pravidelné", |
There was a problem hiding this comment.
Incorrect Slovak translation for "regular" (font style context).
"Pravidelné" means "regular" in the sense of "periodic/recurring." In the context of font style (alongside "bold"/"Tučné" and "italic"/"kurzíva"), the correct Slovak term would be "Normálne" or "Bežné."
Proposed fix
- "regular": "Pravidelné",
+ "regular": "Normálne",📝 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.
| "regular": "Pravidelné", | |
| "regular": "Normálne", |
🤖 Prompt for AI Agents
In `@packages/visual-editor/locales/platform/sk/visual-editor.json` at line 427,
The "regular" translation value in the JSON locale is incorrect for font-style
context; update the value of the "regular" key in
packages/visual-editor/locales/platform/sk/visual-editor.json (the "regular"
string) from "Pravidelné" to a proper font-style term such as "Normálne" (or
"Bežné") so it aligns with the existing "bold"/"Tučné" and "italic"/"kurzíva"
entries.
3 participants
This adds a new Text atom for simple slots that do not need the complexity of RTF. It provides options for text size, color, and font style. Slots that are moving from RTF body text to this simpler text atom have the text migrated without the styling. The following slots have been updated: