feat: adding comma seperated prop for TextList Component #902
Conversation
rkeerthient
changed the title
WalkthroughAdds a new boolean prop Sequence Diagram(s)sequenceDiagram
autonumber
actor Editor
participant Storage
participant MigrationRegistry
participant Renderer as TextListComponent
Editor->>Storage: Save TextList entity (props without commaSeparated)
Storage->>MigrationRegistry: Request apply migrations on load
MigrationRegistry->>Storage: addCommaSeparatedToTextList.propTransformation
note right of MigrationRegistry `#DDEEFF`: If props.commaSeparated is undefined\nset to false
MigrationRegistry-->>Storage: Store transformed entity (props.commaSeparated = false)
Storage->>Renderer: Render TextList with props.commaSeparated = false
alt commaSeparated = true
Renderer->>Renderer: Render inline items with comma separators
else commaSeparated = false
Renderer->>Renderer: Render as bulleted list
end
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
🚧 Files skipped from review as they are similar to previous changes (17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (10)
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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
packages/visual-editor/locales/cs/visual-editor.json(1 hunks)packages/visual-editor/locales/da/visual-editor.json(1 hunks)packages/visual-editor/locales/de/visual-editor.json(1 hunks)packages/visual-editor/locales/en-GB/visual-editor.json(1 hunks)packages/visual-editor/locales/en/visual-editor.json(1 hunks)packages/visual-editor/locales/es/visual-editor.json(1 hunks)packages/visual-editor/locales/et/visual-editor.json(1 hunks)packages/visual-editor/locales/fi/visual-editor.json(1 hunks)packages/visual-editor/locales/fr/visual-editor.json(1 hunks)packages/visual-editor/locales/hr/visual-editor.json(1 hunks)packages/visual-editor/locales/hu/visual-editor.json(1 hunks)packages/visual-editor/locales/it/visual-editor.json(1 hunks)packages/visual-editor/locales/ja/visual-editor.json(1 hunks)packages/visual-editor/locales/lt/visual-editor.json(1 hunks)packages/visual-editor/locales/lv/visual-editor.json(1 hunks)packages/visual-editor/locales/nb/visual-editor.json(1 hunks)packages/visual-editor/locales/nl/visual-editor.json(1 hunks)packages/visual-editor/locales/pl/visual-editor.json(1 hunks)packages/visual-editor/locales/pt/visual-editor.json(1 hunks)packages/visual-editor/locales/ro/visual-editor.json(1 hunks)packages/visual-editor/locales/sk/visual-editor.json(1 hunks)packages/visual-editor/locales/sv/visual-editor.json(1 hunks)packages/visual-editor/locales/tr/visual-editor.json(1 hunks)packages/visual-editor/locales/zh-TW/visual-editor.json(1 hunks)packages/visual-editor/locales/zh/visual-editor.json(1 hunks)packages/visual-editor/src/components/contentBlocks/TextList.tsx(4 hunks)packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts(1 hunks)packages/visual-editor/src/components/migrations/migrationRegistry.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/visual-editor/src/components/contentBlocks/TextList.tsx (2)
packages/visual-editor/src/editor/YextField.tsx (1)
YextField(163-284)packages/visual-editor/src/utils/resolveComponentData.tsx (1)
resolveComponentData(52-99)
packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts (1)
addCommaSeparatedToTextList(3-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
🔇 Additional comments (16)
packages/visual-editor/locales/nb/visual-editor.json (1)
147-147: Consistent localization addition, but note systemic property naming issue.The addition of
"commaSeperated": "Kommadelt"is correct and properly positioned. However, the property name usescommaSeperated(with an 'e') instead of the standard English spellingcommaSeparated(with an 'a'). This typo appears consistent with the implementation (per the migration file mentioned in the PR summary), so it is not an error in this file. Consider opening a follow-up issue to standardize the property name across the codebase if not already tracked.packages/visual-editor/locales/sk/visual-editor.json (1)
148-148: LGTM. Correctly placed and translated.packages/visual-editor/locales/lv/visual-editor.json (1)
147-147: LGTM. Correctly placed and translated.packages/visual-editor/locales/hr/visual-editor.json (1)
147-147: LGTM. Correctly placed and translated.packages/visual-editor/locales/zh-TW/visual-editor.json (1)
147-147: LGTM. Correctly placed and translated.packages/visual-editor/locales/pl/visual-editor.json (1)
148-148: LGTM. Correctly placed and translated.packages/visual-editor/locales/de/visual-editor.json (1)
146-146: LGTM. Correctly placed and translated.packages/visual-editor/locales/zh/visual-editor.json (1)
147-147: LGTM. Correctly placed and translated.packages/visual-editor/locales/sv/visual-editor.json (1)
148-148: Consistent property misspelling across all locale files.The key
commaSeperated(one 'r' in "separated") is misspelled. The correct spelling iscommaSeparated(two 'r's). This misspelling appears in all 8 locale files in this PR. To maintain code quality and avoid future confusion, consider a global rename to the correct spelling before merging.Swedish translation is appropriate: "Kommaseparerad" = "comma-separated."
packages/visual-editor/locales/ja/visual-editor.json (1)
147-147: Same spelling inconsistency as other locale files.Line 147 also uses
commaSeperated(misspelled). See note above regarding correcting tocommaSeparatedacross all files. Japanese translation "カンマ区切り" is appropriate and correctly conveys "comma-separated."packages/visual-editor/locales/fr/visual-editor.json (1)
146-146: Same misspelled property name across all locales.Line 146 continues the pattern of using
commaSeperatedinstead of the correct English spellingcommaSeparated. This should be corrected globally. The French translation "Séparé par des virgules" is grammatically correct and appropriate for the UI.packages/visual-editor/locales/ro/visual-editor.json (1)
147-147: Misspelled property name persists in Romanian locale.Line 147 adds
commaSeperated(should becommaSeparated). This is now the 5th file with the same inconsistency. The Romanian translation "Separat prin virgulă" is appropriate and correctly means "separated by comma."packages/visual-editor/locales/es/visual-editor.json (1)
146-146: Property misspelling consistent across 6 locale files so far.Line 146 uses
commaSeperated(misspelled). The Spanish translation "Separados por comas" is correct and appropriately means "separated by commas." However, the underlying property name should be corrected globally tocommaSeparatedbefore release.packages/visual-editor/locales/hu/visual-editor.json (1)
147-147: Misspelled property name continues in Hungarian locale (7th file).Line 147 adds
commaSeperatedrather thancommaSeparated. The Hungarian translation "Vesszővel elválasztva" is grammatically appropriate and conveys "separated by comma." However, the underlying property name misspelling should be corrected globally.packages/visual-editor/locales/en-GB/visual-editor.json (1)
146-146: Consistent localization addition across all 8 locales.The new
commaSeperatedtranslation key has been added to all provided locale files in alphabetical order within thefieldsobject. All entries follow proper JSON formatting and are positioned consistently.Note: The key spelling
commaSeperated(instead ofcommaSeparated) appears to be intentional and matches the code implementation referenced in the PR summary, so no changes needed.packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
25-25: Verify the migration number sequence.The migration registry jumps from
0023_add_id_to_schemato0025_add_commaSeparated_to_textList, skipping0024. While this might be intentional (e.g., a deleted or reserved migration), please confirm whether migration0024exists or if the new migration should be renumbered to0024.Otherwise, the import and registration look correct structurally. Once the spelling of
commaSeparatedis fixed throughout the codebase (see comments on other files), these changes will be ready.Also applies to: 56-56
packages/visual-editor/src/components/contentBlocks/TextList.tsx
Outdated
Show resolved
Hide resolved
packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/visual-editor/locales/it/visual-editor.json (1)
147-148: Remove the misspelledcommaSeperatedkey; keep onlycommaSeparated.Both lines add translation keys, but line 148 introduces a misspelled duplicate. The correct key is
commaSeparated(line 147); the misspelled variantcommaSeperatedshould be removed entirely.Apply this diff:
"commaSeparated": "Separati da virgole", - "commaSeperated": "Separati da virgole",This issue affects all 24 locale files in the PR. The misspelling must be corrected across the entire codebase—including the prop name in TextList.tsx, the migration file, and all other locale files—to avoid confusion and runtime bugs.
packages/visual-editor/locales/en/visual-editor.json (1)
146-147: Remove the misspelledcommaSeperatedkey and fix its value.Line 146 correctly adds
commaSeparated: "Comma Separated", but line 147 introduces a misspelled duplicate with both an incorrect key (commaSeperated) and an incorrect value ("Comma Seperated"). This typo is user-facing and will appear in the editor UI. Remove line 147 entirely.Apply this diff:
"commaSeparated": "Comma Separated", - "commaSeperated": "Comma Seperated",Important: This correction must cascade across all 24 locale files, the TextList component prop definition, the migration file (ensure it uses the correct key name), and migrationRegistry.ts. A find-and-replace of
commaSeperated→commaSeparatedthroughout the codebase is required to prevent runtime errors and inconsistencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/visual-editor/locales/cs/visual-editor.json(1 hunks)packages/visual-editor/locales/da/visual-editor.json(1 hunks)packages/visual-editor/locales/de/visual-editor.json(1 hunks)packages/visual-editor/locales/en-GB/visual-editor.json(1 hunks)packages/visual-editor/locales/en/visual-editor.json(1 hunks)packages/visual-editor/locales/es/visual-editor.json(1 hunks)packages/visual-editor/locales/et/visual-editor.json(1 hunks)packages/visual-editor/locales/fi/visual-editor.json(1 hunks)packages/visual-editor/locales/fr/visual-editor.json(1 hunks)packages/visual-editor/locales/hr/visual-editor.json(1 hunks)packages/visual-editor/locales/hu/visual-editor.json(1 hunks)packages/visual-editor/locales/it/visual-editor.json(1 hunks)packages/visual-editor/locales/ja/visual-editor.json(1 hunks)packages/visual-editor/locales/lt/visual-editor.json(1 hunks)packages/visual-editor/locales/lv/visual-editor.json(1 hunks)packages/visual-editor/locales/nb/visual-editor.json(1 hunks)packages/visual-editor/locales/nl/visual-editor.json(1 hunks)packages/visual-editor/locales/pl/visual-editor.json(1 hunks)packages/visual-editor/locales/pt/visual-editor.json(1 hunks)packages/visual-editor/locales/ro/visual-editor.json(1 hunks)packages/visual-editor/locales/sk/visual-editor.json(1 hunks)packages/visual-editor/locales/sv/visual-editor.json(1 hunks)packages/visual-editor/locales/tr/visual-editor.json(1 hunks)packages/visual-editor/locales/zh-TW/visual-editor.json(1 hunks)packages/visual-editor/locales/zh/visual-editor.json(1 hunks)packages/visual-editor/src/components/contentBlocks/TextList.tsx(4 hunks)packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/visual-editor/locales/pl/visual-editor.json
- packages/visual-editor/locales/nl/visual-editor.json
- packages/visual-editor/locales/pt/visual-editor.json
- packages/visual-editor/locales/es/visual-editor.json
- packages/visual-editor/locales/da/visual-editor.json
- packages/visual-editor/locales/sk/visual-editor.json
- packages/visual-editor/locales/tr/visual-editor.json
- packages/visual-editor/locales/et/visual-editor.json
- packages/visual-editor/locales/ro/visual-editor.json
- packages/visual-editor/locales/lv/visual-editor.json
- packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts
- packages/visual-editor/locales/fi/visual-editor.json
- packages/visual-editor/locales/fr/visual-editor.json
- packages/visual-editor/src/components/contentBlocks/TextList.tsx
- packages/visual-editor/locales/sv/visual-editor.json
- packages/visual-editor/locales/en-GB/visual-editor.json
- packages/visual-editor/locales/de/visual-editor.json
- packages/visual-editor/locales/cs/visual-editor.json
- packages/visual-editor/locales/zh/visual-editor.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: semgrep/ci
rkeerthient
requested review from
Fondryext,
asanehisa,
benlife5 and
jwartofsky-yext
packages/visual-editor/src/components/migrations/0025_add_commaSeparated_to_textList.ts
Outdated
Show resolved
Hide resolved
|
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. |
On professional VLE pages (location templates), we’re following the new two-column layout, where, the left side shows the description, while the right side lists details like products, services, and specialties. Right now, the right column looks too large because the list is rendered as a
<ul>. To fix this, there will be a new propComma Separatedon the TextList component. When toggled on, the list will display as a comma-separated string instead of a bulleted list. By default, this prop stays off.TextList.mov