fix: comingSoon in locator results#1225
Merged
Merged
Conversation
Contributor
|
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. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds support for a sequenceDiagram
participant LocatorResultCard
participant HoursSection
participant HoursStatusAtom
LocatorResultCard->>HoursSection: pass locator data (includes comingSoon)
HoursSection->>HoursStatusAtom: render status (comingSoon, timezone)
alt comingSoon === true
HoursSection->>HoursStatusAtom: render compact row (no Accordion)
else comingSoon === false
HoursSection->>HoursStatusAtom: render inside Accordion trigger
HoursSection->>HoursSection: show HoursTableAtom in Accordion content on expand
end
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/visual-editor/src/components/Locator.test.tsx (2)
292-304: ⚡ Quick winKeep
comingSoonon the fixture results, not the document.
LocatorResultCardbranches onresult.rawData.comingSoon, but this mock derives that field fromdocument.comingSoonand applies it to every result. That makes mixed-result coverage impossible and weakens the test contract. Prefer settingcomingSoondirectly in the relevantcreateLocatorApiResult(...)fixtures and returningresultsunchanged here.♻️ Suggested change
- JSON.stringify( - createLocatorVerticalSearchResponse( - document.comingSoon - ? results.map((result: any) => ({ - ...result, - data: { - ...result.data, - comingSoon: true, - }, - })) - : results - ) - ), + JSON.stringify(createLocatorVerticalSearchResponse(results)),// Build the intended state into the fixtures instead, e.g. createLocatorApiResult({ ..., comingSoon: true, });🤖 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/Locator.test.tsx` around lines 292 - 304, The test is mutating the fixture by applying document.comingSoon to every result in the createLocatorVerticalSearchResponse call, which prevents testing mixed comingSoon states used by LocatorResultCard (it checks result.rawData.comingSoon); instead, remove the conditional mapping that sets comingSoon from document and ensure each test fixture created via createLocatorApiResult explicitly includes the intended comingSoon value (true/false) so results can be returned unchanged to createLocatorVerticalSearchResponse; update tests that relied on document.comingSoon to set comingSoon to instead construct appropriate createLocatorApiResult(...) entries with comingSoon set.
364-394: ⚡ Quick winAdd a direct assertion for the non-accordion behavior.
This case only validates the
comingSoonpath via screenshot/axe. Please also assert that the “Coming Soon” status renders and the hours accordion trigger/content are absent so this contract stays covered even when screenshot diffs are noisy.🤖 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/Locator.test.tsx` around lines 364 - 394, In the "version 74 comingSoon" test case in Locator.test.tsx add explicit assertions to verify the non-accordion behavior: assert that the "Coming Soon" status text is present (using the rendered output for LocatorComponent / LocatorComponent.defaultProps) and assert that the hours accordion trigger and its content are absent (use queryByText/queryByRole or the HoursAccordion test-id/selector to confirm neither the accordion button nor the hours content exists). This ensures the test checks both presence of the coming-soon message and absence of the hours accordion UI.
🤖 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/Locator.test.tsx`:
- Around line 292-304: The test is mutating the fixture by applying
document.comingSoon to every result in the createLocatorVerticalSearchResponse
call, which prevents testing mixed comingSoon states used by LocatorResultCard
(it checks result.rawData.comingSoon); instead, remove the conditional mapping
that sets comingSoon from document and ensure each test fixture created via
createLocatorApiResult explicitly includes the intended comingSoon value
(true/false) so results can be returned unchanged to
createLocatorVerticalSearchResponse; update tests that relied on
document.comingSoon to set comingSoon to instead construct appropriate
createLocatorApiResult(...) entries with comingSoon set.
- Around line 364-394: In the "version 74 comingSoon" test case in
Locator.test.tsx add explicit assertions to verify the non-accordion behavior:
assert that the "Coming Soon" status text is present (using the rendered output
for LocatorComponent / LocatorComponent.defaultProps) and assert that the hours
accordion trigger and its content are absent (use queryByText/queryByRole or the
HoursAccordion test-id/selector to confirm neither the accordion button nor the
hours content exists). This ensures the test checks both presence of the
coming-soon message and absence of the hours accordion UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4075d002-ee78-45dc-8556-d74a12250e52
📒 Files selected for processing (2)
packages/visual-editor/src/components/Locator.test.tsxpackages/visual-editor/src/components/LocatorResultCard.tsx
auto-screenshot-update: true
asanehisa
requested review from
benlife5,
briantstephan,
jwartofsky-yext and
mkilpatrick
mkilpatrick
previously approved these changes
Jun 2, 2026
briantstephan
previously approved these changes
Jun 2, 2026
auto-screenshot-update: true
mkilpatrick
approved these changes
Jun 2, 2026
asanehisa
added a commit
that referenced
this pull request
Jun 2, 2026
Same fix as #1225 --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before:
Now the accordion is hidden when comingSoon is true.