feat: use ref_listings #846
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. |
WalkthroughThis PR removes coordinate-based CTA props and coordinate editing from the visual editor. CTAProps no longer includes a coordinate field. The CTA component now uses useDocument to read stream data and resolves "getDirections" links from props.link, listings references, or display coordinates (in that order), falling back to "#". Editor UI and constant configs no longer surface coordinate fields; helper getCTAType replaces getCTATypeAndCoordinate and returns only the CTA type. Call sites throughout the codebase stop passing coordinate to CTA. Sequence Diagram(s)sequenceDiagram
participant Component as Page / Content Component
participant CTA as CTA Component
participant useDoc as useDocument Hook
participant Stream as Stream Document
Component->>CTA: Render CTA (props: link?, label?, ctaType)
activate CTA
alt ctaType == "getDirections"
CTA->>useDoc: useDocument()
activate useDoc
useDoc->>Stream: read streamDocument
Stream-->>useDoc: ref_listings, yextDisplayCoordinate
useDoc-->>CTA: document data
deactivate useDoc
Note over CTA: Resolve link in priority order
alt props.link present
CTA->>CTA: use props.link
else ref_listings present
CTA->>CTA: use listingsLink (from ref_listings)
else yextDisplayCoordinate present
CTA->>CTA: use coordinateLink (from yextDisplayCoordinate)
else
CTA->>CTA: use "#"
end
CTA->>CTA: set label / ariaLabel via i18n
end
CTA-->>Component: rendered CTA output
deactivate CTA
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
commit: |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
…al-editor into adjust-to-use-listings
Updated the constantValue structure for address to include city, line1, postalCode, and countryCode.
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
54-104: IncludestreamDocumentin the memo dependencies.
useMemorelies onstreamDocumentto build directions links, but the dependency array omits it. When the editor updates the document (e.g., address or listings), this hook keeps returning stale links. AddstreamDocumentto the dependency list so edits reflect immediately.Apply this diff to keep the memo in sync:
- }, [props, ariaLabel]); + }, [props, ariaLabel, streamDocument]);
🧹 Nitpick comments (1)
packages/visual-editor/src/components/contentBlocks/Address.tsx (1)
82-91: Clarify comment and consider extracting duplicated logic.The logic correctly implements the ref_listings preference described in the PR objectives. However, the comment could be more explicit about the inverse case to improve readability.
The same logic is duplicated in
CoreInfoSection.tsx(lines 338-348). Consider extracting this into a shared utility function to reduce duplication and ensure consistency.Apply this diff to improve the comment clarity:
- // If ref_listings doesn't exist or the address field selected isn't just address, use the address link. + // Use addressLink when: (1) a custom address field is selected, or (2) ref_listings is unavailable. + // Otherwise (standard 'address' field + ref_listings exists), pass undefined to let CTA use ref_listings. const useAddressLink: boolean = data.address.field !== "address" || !streamDocument.ref_listings?.length;Optional: Extract the logic into a shared utility:
// In a shared utilities file export const shouldUseAddressLink = ( addressField: string, refListings: unknown[] | undefined ): boolean => { return addressField !== "address" || !refListings?.length; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/visual-editor/src/components/testing/screenshots/BreadcrumbsSection/[tablet] default props with document data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/BreadcrumbsSection/[tablet] version 4 props.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**packages/visual-editor/src/components/testing/screenshots/BreadcrumbsSection/[tablet] version 8 with non-default props with document data.pngis excluded by!**/*.png,!packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (18)
packages/visual-editor/src/components/atoms/cta.tsx(2 hunks)packages/visual-editor/src/components/contentBlocks/Address.tsx(2 hunks)packages/visual-editor/src/components/contentBlocks/CTAGroup.tsx(2 hunks)packages/visual-editor/src/components/contentBlocks/CtaWrapper.tsx(2 hunks)packages/visual-editor/src/components/contentBlocks/GetDirections.tsx(1 hunks)packages/visual-editor/src/components/pageSections/CoreInfoSection.tsx(4 hunks)packages/visual-editor/src/components/pageSections/EventSection.tsx(0 hunks)packages/visual-editor/src/components/pageSections/InsightSection.tsx(0 hunks)packages/visual-editor/src/components/pageSections/ProductSection.tsx(0 hunks)packages/visual-editor/src/components/pageSections/PromoSection.tsx(0 hunks)packages/visual-editor/src/components/pageSections/TeamSection.tsx(0 hunks)packages/visual-editor/src/components/pageSections/heroVariants/HeroContent.tsx(0 hunks)packages/visual-editor/src/docs/ai/components.d.ts(1 hunks)packages/visual-editor/src/editor/YextEntityFieldSelector.tsx(0 hunks)packages/visual-editor/src/internal/puck/constant-value-fields/Address.tsx(0 hunks)packages/visual-editor/src/internal/puck/constant-value-fields/EnhancedCallToAction.tsx(1 hunks)packages/visual-editor/src/vite-plugin/defaultLayoutData.ts(1 hunks)starter/src/templates/dev.tsx(1 hunks)
💤 Files with no reviewable changes (8)
- packages/visual-editor/src/components/pageSections/ProductSection.tsx
- packages/visual-editor/src/components/pageSections/PromoSection.tsx
- packages/visual-editor/src/components/pageSections/InsightSection.tsx
- packages/visual-editor/src/components/pageSections/TeamSection.tsx
- packages/visual-editor/src/editor/YextEntityFieldSelector.tsx
- packages/visual-editor/src/internal/puck/constant-value-fields/Address.tsx
- packages/visual-editor/src/components/pageSections/EventSection.tsx
- packages/visual-editor/src/components/pageSections/heroVariants/HeroContent.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
packages/visual-editor/src/components/contentBlocks/Address.tsx (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
CTA(138-185)
packages/visual-editor/src/components/contentBlocks/GetDirections.tsx (1)
packages/visual-editor/src/components/atoms/cta.tsx (2)
CTAVariant(40-40)CTA(138-185)
packages/visual-editor/src/components/pageSections/CoreInfoSection.tsx (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
CTA(138-185)
packages/visual-editor/src/internal/puck/constant-value-fields/EnhancedCallToAction.tsx (1)
packages/visual-editor/src/editor/YextEntityFieldSelector.tsx (1)
YextEntityField(54-71)
packages/visual-editor/src/components/contentBlocks/CTAGroup.tsx (1)
packages/visual-editor/src/internal/puck/constant-value-fields/EnhancedCallToAction.tsx (1)
getCTAType(41-51)
packages/visual-editor/src/components/contentBlocks/CtaWrapper.tsx (1)
packages/visual-editor/src/internal/puck/constant-value-fields/EnhancedCallToAction.tsx (1)
getCTAType(41-51)
🪛 Biome (2.1.2)
packages/visual-editor/src/components/atoms/cta.tsx
[error] 57-57: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 58-64: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 65-71: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (7)
packages/visual-editor/src/docs/ai/components.d.ts (1)
1-11: No concerns with import reordering.The
Coordinateimport is still actively used in the file (e.g.,NearbyLocationsDataat line 799 andEnhancedTranslatableCTAat line 1363), so the reordering is a safe, non-breaking change.packages/visual-editor/src/components/contentBlocks/CTAGroup.tsx (1)
81-94: LGTM! Clean refactoring to remove coordinate handling.The change correctly adapts to the new
getCTATypeAPI by extracting onlyctaTypeand removing thecoordinateprop from the CTA component. This aligns with the broader architectural change to delegate link resolution to the CTA component itself.packages/visual-editor/src/components/contentBlocks/GetDirections.tsx (1)
17-30: LGTM! Excellent simplification of the Get Directions component.The component now delegates all link resolution to the CTA component via
ctaType="getDirections", removing the need for local EntityField wrapping and coordinate handling. This centralizes the logic for preferringref_listingswhen available, as described in the PR objectives.packages/visual-editor/src/components/pageSections/CoreInfoSection.tsx (3)
338-348: Same logic and comment as Address.tsx—consider shared utility.This block duplicates the addressLink computation and useAddressLink logic from
Address.tsx(lines 82-91). As noted in that file's review, consider extracting this into a shared utility function to maintain consistency and reduce duplication.
423-433: LGTM! Proper integration of address-based Get Directions.The CTA rendering correctly integrates the new
ctaType="getDirections"approach with conditional link passing based onuseAddressLink. The logic ensures thatref_listingsis preferred when the standard address field is used and listings data is available.
650-650: Region field addition is consistent with existing address structure across the codebase.The
region: ""field in CoreInfoSection.tsx line 650 aligns with the established address structure. The same field already exists in Address.tsx's defaultProps (line 141) and is actively used in address rendering (CoreInfoSection.tsx line 418 displays it as part of the address lines). The field is part of the standard AddressType structure used throughout the codebase.packages/visual-editor/src/components/contentBlocks/Address.tsx (1)
82-87: Type cast is correct and justified—no changes needed.The
address as AddressTypecast is a necessary TypeScript type narrowing operation. TheresolveComponentDatafunction returns a broader union type (includingundefinedand other possibilities), andgetDirectionsrequires the narrowerAddressType.The code is safe because:
- The address is guarded with a null check before rendering (line 26:
{address && (...))- This pattern is consistently used throughout the codebase (e.g.,
CoreInfoSection.tsxline 339)AddressTypeis from the trusted external library@yext/pages-components, ensuring type compatibility
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
54-104: AddstreamDocumentto theuseMemodependencies array.
streamDocumentis accessed inside theuseMemocallback (lines 57, 70) but is missing from the dependencies array on line 104. This causes the memo to return stale data when the document changes, breaking the link resolution logic forgetDirectionsCTAs.Apply this diff to fix the missing dependency:
- }, [props, ariaLabel]); + }, [props, ariaLabel, streamDocument, t]);Note:
ariaLabelis redundant since it's already part ofprops, but including both maintains compatibility. Thetfunction fromuseTranslationis typically stable but should be included for completeness.
♻️ Duplicate comments (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
56-79: LGTM! Previous lint issue resolved.The
getDirectionscase is now properly wrapped in a block, resolving thenoSwitchDeclarationslint error flagged in the previous review. The link resolution logic correctly prioritizes user-provided links, then listings-based links, then coordinate-based links, with a sensible fallback to "#".
asanehisa
requested review from
benlife5,
briantstephan,
colton-demetriou and
mkilpatrick
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/components/atoms/cta.tsx (1)
56-79: Excellent implementation - lint issue fixed and logic aligns with PR objectives.The
getDirectionscase now correctly:
- Wraps declarations in a block, resolving the previous
noSwitchDeclarationslint error- Implements the preference order: user-provided link → listings link → coordinate link → fallback
- Uses nullish coalescing for
ref_listingsto handle undefined/null safely- Leverages i18n for default labels, improving accessibility
The link resolution strategy matches the PR's goal to prefer
ref_listingswhen available.Optional: Update the comment on line 72 for completeness.
Consider mentioning the final
"#"fallback:- // Prefer user-provided link, then listings link, then coordinate link + // Prefer user-provided link, then listings link, then coordinate link, then "#" fallback
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/visual-editor/src/components/atoms/cta.tsx(3 hunks)
🔇 Additional comments (2)
packages/visual-editor/src/components/atoms/cta.tsx (2)
6-6: LGTM - Import addition supports document context access.The addition of
useDocumentenables the component to access stream document data for resolving Get Directions links.
104-104: Dependency array is correct.The dependency array
[props, streamDocument]correctly includesstreamDocumentbecause it's used inside theuseMemo(lines 57 and 70). This follows React's exhaustive-deps rule and ensures the memoized value updates when the document changes.
For Get Directions previously using Coordinates: ``` if (ref_listings): build link with ref_listings else: build link with yextDisplayCoordinate ``` For Get Directions previously using Address: ``` if currentlySelectedAddress is address && ref_listings: build link with ref_listings else build link with Address ``` Remove static value from Address prop. Don't allow users to select coordinates to use, always use "yextDisplayCoordinate" Tested in platform and confirmed setting address field to a diff field works. Confirmed ref_listings is used if it exists and gives a valid link. Confirmed fallbacks work as expected. --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…ent (#860) Updates the locator to derive the Get Directions link on result cards from the ref_listings field in the search response if present, otherwise fallback to the yextDisplayCoordinate. If that is also not present, don't display the get direction button. Copies the logic in #846 to determine the URL. J=WAT-5136 TEST=manual Created a locator in the Krusty dev account (514581), which has some entities with ref_listings data pointing to simulated URLs. Confirmed that when the ref_listings field in the search response had a listingUrl for a GOOGLEMYBUSINESS publisher, the Get Directions button pointed to the listingUrl rather than the Google Maps one generated from the yextDisplayCoordinate field. Confirmed that when the ref_listings field was empty or did not have a GOOGLEMYBUSINESS publisher, the yextDisplayCoordinate field was used to generate the Get Direction URL instead. jam:https://jam.dev/c/a41f5a6d-1491-4a7a-96aa-936766e204e6
3 participants
For Get Directions previously using Coordinates:
For Get Directions previously using Address:
Remove static value from Address prop. Don't allow users to select coordinates to use, always use "yextDisplayCoordinate"
Tested in platform and confirmed setting address field to a diff field works. Confirmed ref_listings is used if it exists and gives a valid link. Confirmed fallbacks work as expected.