feat: add default meta values for locator/directory #990
Conversation
commit: |
WalkthroughThis PR adds directory metadata resolution for dm_-prefixed entities: a new migration (0052) backfills title/description meta fields with localized/PLACEHOLDER structures; getPageMetadata gains resolveDirectoryRootProps and DIRECTORY_META_DEFAULTS to replace PLACEHOLDERs using streamDocument; migrate now passes streamDocument into propTransformations; directoryConfig adds root.resolveData to call resolveDirectoryRootProps; the directory template injects translations after resolveAllData; default layout data versions and placeholders updated; tests for English fallback and PLACEHOLDER resolution were added. Sequence DiagramsequenceDiagram
participant Template as "Directory Template"
participant Migration as "Migration System"
participant Config as "directoryConfig"
participant Resolver as "resolveDirectoryRootProps"
participant Translator as "injectTranslations"
Template->>Migration: run migrations (propTransformation with streamDocument)
Migration-->>Template: return migratedData
Template->>Config: resolveAllData(migratedData, directoryConfig, {streamDocument})
activate Config
Config->>Config: trigger root.resolveData(data, params)
Config->>Resolver: resolveDirectoryRootProps(data.props, params.metadata.streamDocument)
Resolver->>Resolver: replace PLACEHOLDERs with locale-specific values
Resolver-->>Config: return updated props
Config-->>Template: return updatedData
deactivate Config
Template->>Translator: injectTranslations(updatedData)
Translator-->>Template: return final translated data
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: 1
🤖 Fix all issues with AI agents
In `@packages/visual-editor/src/vite-plugin/defaultLayoutData.ts`:
- Around line 4761-4762: Remove the debug console.log call that prints
defaultLayoutData.locator; specifically delete the line with
console.log(defaultLayoutData.locator) in the defaultLayoutData module (so the
locator data isn't logged on every import), or replace it with a
non-production-safe logger/conditional debug flag if you need runtime
diagnostics; ensure no other stray console.log statements remain in the
defaultLayoutData code path.
🧹 Nitpick comments (1)
packages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.ts (1)
1-2: Inconsistent import path style.Line 1 uses a path alias style (
"src/utils/applyTheme.ts") while line 2 uses a relative path ("../../utils/migrate.ts"). Consider using consistent import paths throughout the file for maintainability.♻️ Suggested fix using relative imports
-import { StreamDocument } from "src/utils/applyTheme.ts"; +import { StreamDocument } from "../../utils/applyTheme.ts"; import { Migration } from "../../utils/migrate.ts";
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/visual-editor/src/components/configs/directoryConfig.tsxpackages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.tspackages/visual-editor/src/components/migrations/migrationRegistry.tspackages/visual-editor/src/utils/getPageMetadata.test.tspackages/visual-editor/src/utils/getPageMetadata.tspackages/visual-editor/src/utils/migrate.tspackages/visual-editor/src/vite-plugin/defaultLayoutData.tspackages/visual-editor/src/vite-plugin/templates/directory.tsxstarter/localData/dev-dm-city-stream__en__8932945.json
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-12T20:17:37.033Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 946
File: packages/visual-editor/src/components/migrations/migrationRegistry.ts:47-48
Timestamp: 2025-12-12T20:17:37.033Z
Learning: In the yext/visual-editor repository, empty object placeholders (`{}`) in the migrationRegistry array are sometimes intentionally used to reserve migration slots for parallel PRs to prevent merge conflicts and maintain consistent migration indices.
Applied to files:
packages/visual-editor/src/components/migrations/migrationRegistry.tspackages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.ts
📚 Learning: 2026-01-12T21:31:40.196Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 980
File: packages/visual-editor/src/components/pageSections/AboutSection/AboutSectionDetailsColumn.tsx:310-316
Timestamp: 2026-01-12T21:31:40.196Z
Learning: Do not use React hooks inside arbitrary callback functions. Hooks must be called only at the top level of React function components or custom hooks. If a callback (e.g., getItemSummary in a framework context like Puck) is not a component or a custom hook, refactor to move hook usage to a component/hook, or pass necessary data via props/state. This guideline applies to all TSX files in the repo.
Applied to files:
packages/visual-editor/src/vite-plugin/templates/directory.tsxpackages/visual-editor/src/components/configs/directoryConfig.tsx
📚 Learning: 2025-12-23T16:30:55.497Z
Learnt from: benlife5
Repo: yext/visual-editor PR: 958
File: packages/visual-editor/src/components/pageSections/heroVariants/CompactHero.tsx:38-41
Timestamp: 2025-12-23T16:30:55.497Z
Learning: In CompactHero and CompactPromo components, duplicate IDs on image elements are intentional: conditional visibility classes (hidden/sm:block/sm:hidden) ensure only one image renders at a time based on responsive breakpoints and desktopImagePosition/mobileImagePosition settings.
Applied to files:
packages/visual-editor/src/vite-plugin/defaultLayoutData.ts
🧬 Code graph analysis (5)
packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
packages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.ts (1)
addDefaultLocatorAndDirectoryMetaFields(10-92)
packages/visual-editor/src/components/configs/directoryConfig.tsx (1)
packages/visual-editor/src/utils/getPageMetadata.ts (1)
resolveDirectoryRootProps(82-118)
packages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.ts (1)
packages/visual-editor/src/utils/migrate.ts (1)
Migration(27-36)
packages/visual-editor/src/utils/getPageMetadata.test.ts (1)
packages/visual-editor/src/utils/getPageMetadata.ts (1)
getPageMetadata(11-41)
packages/visual-editor/src/utils/getPageMetadata.ts (1)
packages/visual-editor/src/utils/index.ts (2)
resolveComponentData(18-18)StreamDocument(7-7)
⏰ 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). (5)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (22.x)
- GitHub Check: call_unit_test / unit_tests (24.x)
- GitHub Check: semgrep/ci
- GitHub Check: create-dev-release
🔇 Additional comments (14)
starter/localData/dev-dm-city-stream__en__8932945.json (1)
153-155: Entity type update aligns with directory fixtures.Switching this child entry to
dm_citymatches the new directory metadata handling path.packages/visual-editor/src/utils/migrate.ts (1)
67-70: LGTM – root migrations now receive stream context.Passing
streamDocumentinto rootpropTransformationkeeps migration inputs consistent.packages/visual-editor/src/vite-plugin/templates/directory.tsx (1)
116-122: LGTM – returns the transformed data.This ensures the latest transformed props drive the render output.
packages/visual-editor/src/utils/getPageMetadata.ts (3)
1-38: Directory metadata resolution + title fallback look solid.Resolving directory root props before metadata extraction and falling back to English title keeps head metadata populated for dm_ pages.
48-53: Escaping&,<,>is a good hardening step.
56-115: Verify placeholder replacement for non‑English primary locales.
resolveDirectoryRootPropsonly checksconstantValue.enfor"PLACEHOLDER". If placeholders are written under a different locale key, defaults won’t be injected. Please confirm placeholders always land inen, or broaden the check to the primary locale key.packages/visual-editor/src/components/configs/directoryConfig.tsx (1)
62-69: LGTM – root props normalization for directory pages.Applying
resolveDirectoryRootPropsat the root keeps editor data aligned with directory defaults.packages/visual-editor/src/utils/getPageMetadata.test.ts (1)
105-187: Well-structured test coverage for the new directory metadata features.The two new tests properly validate:
- English fallback behavior for title (but not description) when the localized value is empty
- PLACEHOLDER resolution for directory pages based on entity type and address fields
These tests align well with the PR objectives and provide good regression protection for the new functionality.
packages/visual-editor/src/vite-plugin/defaultLayoutData.ts (2)
3888-3904: Directory layout configuration updated with PLACEHOLDER approach.The directory layout now uses
PLACEHOLDERvalues for title and description, which will be dynamically resolved byresolveDirectoryRootPropsbased on the entity type (dm_root, dm_country, dm_region, dm_city). This aligns with the migration logic and the test expectations.
4672-4754: Locator layout configuration with comprehensive resultCard structure.The locator layout now includes:
- Default title "Find Locations" and description "Find the right location for you."
- A detailed resultCard configuration with field selections and visibility controls
This provides a good starting structure for locator pages.
packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
53-53: Migration properly imported and registered.The new migration follows the established pattern and is correctly added at the end of the registry array.
Also applies to: 112-112
packages/visual-editor/src/components/migrations/0052_add_default_locator_and_directory_meta_fields.ts (3)
18-24: Guard clause correctly filters applicable entities.The migration appropriately skips non-locator and non-directory entities, preventing unintended changes to regular entity page sets.
10-91: Migration logic is well-structured for the intended use case.The migration:
- Correctly guards against non-applicable entity types
- Initializes missing title/description structures
- Sets appropriate defaults (actual values for locator, PLACEHOLDER for directory)
- Enables
constantValueEnabledwhen field is emptyThe PLACEHOLDER approach for directory entities is a clean pattern that defers resolution to runtime where entity-specific data is available.
50-79: Edge case handling is intentional; downstream code gracefully handles empty constantValue objects.The check
constantValue === ""only applies defaults whenconstantValueis an empty string. If an existing layout hasconstantValue: {}before migration, it won't be updated by this migration. However, downstreamresolveComponentDataandescapeHtmlhandle this defensively—empty objects are processed throughresolveTranslatableType(which returns the object unchanged) and thenescapeHtmlconverts non-string values to empty strings. This is intentional design to avoid overwriting user customizations in existing layouts; new layouts will always receive proper defaults since the migration initializes missing fields withconstantValue: "".
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const metaData: RootConfig = {}; | ||
|
|
||
| if (document.meta?.entityType?.id?.startsWith("dm_")) { | ||
| rootProps = resolveDirectoryRootProps(rootProps, document); |
There was a problem hiding this comment.
Isn't this already called by the dirConfig's resolveData?
There was a problem hiding this comment.
getPageMetadata gets called in the getHeadConfig which doesn't get the props after the resolveAllData call in transformProps, so I had to do the fallback separately here
| }); | ||
|
|
||
| // For title, fallback to en value if localized value is empty | ||
| if (metaData.title === "") { |
There was a problem hiding this comment.
I feel like this should be part of the rootProps resolvers?
benlife5
dismissed stale reviews from briantstephan and asanehisa
via
0bd8fe0
benlife5
force-pushed
the
default-meta-values
branch
from
4ac350d to
0bd8fe0
Compare
|
Accidentally pushed the test fix here :/ should be reset now |
Locator: set default constant values for title/description
Directory: set default constant values for title/description to "PLACEHOLDER", then update to the correct default value in getPageMetadata (for live page) and resolveData (for editor fields)
All pages: use english translation for title if there is no value for the locale