Skip to content

fix: remove separate locator results scrollbar for mobile#1237

Merged
anguyen-yext2 merged 5 commits into
mainfrom
locator-results
Jun 12, 2026
Merged

fix: remove separate locator results scrollbar for mobile#1237
anguyen-yext2 merged 5 commits into
mainfrom
locator-results

Conversation

@anguyen-yext2

@anguyen-yext2 anguyen-yext2 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

In mobile view, display a "show more" button at the end of the search results instead of pagination.

J=WAT-5580
TEST=manual

tested in dev mode and platform
https://drive.google.com/file/d/1Bw7xFbmvdOncZl37DXCalcTiLl3zod0n/view?usp=sharing

In mobile view, display a "show more" button insteas of pagination.

J=WAT-5580
TEST=manual

tested in dev mode
@anguyen-yext2 anguyen-yext2 requested review from a team June 11, 2026 21:35
@anguyen-yext2 anguyen-yext2 added the create-dev-release Triggers dev release workflow label Jun 11, 2026
@github-actions

Copy link
Copy Markdown
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.

@pkg-pr-new

pkg-pr-new Bot commented Jun 11, 2026

Copy link
Copy Markdown

commit: 75cd400

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 325de106-887c-4f89-978b-0e8ba8e5b5ad

📥 Commits

Reviewing files that changed from the base of the PR and between feffc22 and 4b7ccd9.

⛔ Files ignored due to path filters (2)
  • packages/visual-editor/src/components/testing/screenshots/Locator/[desktop] latest version default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/Locator/[tablet] latest version default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (1)
  • packages/visual-editor/src/components/Locator.test.tsx

Walkthrough

This PR centralizes viewport breakpoint logic into a new hook module (VIEWPORT_BREAKPOINTS, getViewport, useWindowWidth), migrates header components to those shared utilities, and refactors Locator to derive searchResults from headless state, detect mobile by preview window width, accumulate mobileResults for "Show more locations" pagination, add a MobileLocatorResultsSection for mobile rendering, and avoid desktop scroll-to-top when on mobile. Multiple locale JSON files were updated to add the showMoreLocations key.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant LocatorComponent
  participant HeadlessSearchState
  participant MobileLocatorResultsSection
  User->>LocatorComponent: Perform search / open locator
  LocatorComponent->>HeadlessSearchState: Read state.vertical.results
  LocatorComponent->>LocatorComponent: Determine isMobile via preview window width -> getViewport
  alt Mobile viewport
    LocatorComponent->>MobileLocatorResultsSection: Render mobile cards + "Show more locations"
    User->>MobileLocatorResultsSection: Click "Show more locations"
    MobileLocatorResultsSection->>LocatorComponent: request offset increment
    LocatorComponent->>HeadlessSearchState: set offset / fetch next results
    HeadlessSearchState->>LocatorComponent: return new results
    LocatorComponent->>LocatorComponent: append new results to mobileResults
    LocatorComponent->>MobileLocatorResultsSection: render appended list
  else Desktop viewport
    LocatorComponent->>LocatorComponent: Render VerticalResults and handle desktop scroll-to-top on offset change
  end
Loading

Possibly related PRs

  • yext/visual-editor#924: Both PRs modify packages/visual-editor/src/components/Locator.tsx to change locator pagination/offset handling (including reset/setOffset logic and scroll-to-top behavior).
  • yext/visual-editor#863: Overlaps in Locator pagination/offset-driven search behavior and mobile "show more" handling.

Suggested reviewers

  • vijay267
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing separate scrollbar behavior on mobile by implementing a show-more button approach.
Description check ✅ Passed The description directly relates to the changeset by explaining the mobile UX change (show more button instead of pagination) and references the ticket and testing approach.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch locator-results

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@packages/visual-editor/src/components/Locator.tsx`:
- Around line 1729-1739: When entering mobile viewport with a non-zero
currentOffset the existing useEffect appends only the current page
(setMobileResults), which causes mobileResults.length to be an incorrect offset
for subsequent "Show more" requests; modify the effect that depends on
currentOffset/isMobileViewport/searchResults to detect the transition into
mobile (isMobileViewport true when previously false) or a non-zero currentOffset
and then either reset mobileResults to an empty array and set offset to 0, or
rebuild mobileResults as the full prefix of pages up to currentOffset by
aggregating past pages (using currentOffset, searchResults and any stored page
history) before appending the current page, so that mobileResults.length
accurately reflects the next offset used by the "Show more" logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 600c01b8-d9ca-41d0-b65e-9f012cc1332b

📥 Commits

Reviewing files that changed from the base of the PR and between d03f15f and bee67ed.

📒 Files selected for processing (5)
  • packages/visual-editor/src/components/Locator.tsx
  • packages/visual-editor/src/components/header/HeaderLinks.tsx
  • packages/visual-editor/src/components/header/PrimaryHeaderSlot.tsx
  • packages/visual-editor/src/components/header/viewport.ts
  • packages/visual-editor/src/hooks/useViewport.ts
💤 Files with no reviewable changes (1)
  • packages/visual-editor/src/components/header/viewport.ts

Comment thread packages/visual-editor/src/components/Locator.tsx Outdated
vijay267
vijay267 previously approved these changes Jun 11, 2026

@vijay267 vijay267 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but will defer to Sumo as well

@asanehisa asanehisa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test is failing?

@jwartofsky-yext jwartofsky-yext left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

I would like to reiterate that I think the Locator.tsx file should be broken up a bit as it is over 2k lines long

It would be reasonable to make a Locator/ folder with different files for Locator.tsx, Card.tsx, ResultsCounts.tsx, etc

@anguyen-yext2

Copy link
Copy Markdown
Contributor Author

Looks good to me

I would like to reiterate that I think the Locator.tsx file should be broken up a bit as it is over 2k lines long

It would be reasonable to make a Locator/ folder with different files for Locator.tsx, Card.tsx, ResultsCounts.tsx, etc

I agree. Created an item dedicated for this so we don't postpone forever, but I'll close this PR out as is

@anguyen-yext2 anguyen-yext2 merged commit e211f13 into main Jun 12, 2026
18 checks passed
@anguyen-yext2 anguyen-yext2 deleted the locator-results branch June 12, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

create-dev-release Triggers dev release workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants