perf: improve mobile location detail LCP - #665
Conversation
There was a problem hiding this comment.
The new responsive map-loading logic is insufficiently validated and may regress mobile map navigation.
Blocking issues
- On mobile,
locationDetailStubunconditionally prevents Google Maps from loading even whenshowMapViewOnMobileis true. If users can switch from a location detail panel to map view, this produces a blank map. Clarify/fix the intended state logic and add tests covering mobile detail, list, map-toggle, resize, and desktop behavior.
There was a problem hiding this comment.
The map-loading change lacks coverage for the risky component-level behavior it introduces.
Blocking issues
- Tests cover only the pure predicate, not LocationsMap's deferred mount, store-driven toggle, resize listener, or unmount/remount behavior. Add component/integration coverage verifying desktop initialization, mobile map toggling, resizing in both directions, and preservation or intentional loss of map state.
There was a problem hiding this comment.
The mobile location-detail view can regress after this change.
Blocking issues
- MainComponent no longer forces the side panel on
/locations/:slug. IfshowMapViewOnMobileremains true when navigating from the mobile map to a location detail route, the map stays visible and the location details remain hidden. No replacement state reset or regression test covers this route transition.
There was a problem hiding this comment.
The mobile location-detail experience is materially changed without coverage or documented confirmation.
Blocking issues
street-view.tsxremoves the interactive mobile map, location marker, and map controls and replaces them with a static Street View image. This is a user-facing functional regression unless explicitly intended, and no tests verify the new responsive behavior or fallback when a Street View image is unavailable.
|
@adambard1 I just ran a Lighthouse test for this pr deploment.
|
|
Thanks @shakilhossain1 I saw that there's a github action requested changes but it doesn't have any content, so I am unsure what to do. |
|
I ran it on mobile @adambard1 . And 'The GitHub action- 1 requested changes' means Codex hasn't approved this pr, and the content is in the latest Codex comment. Blocking issues
|
|
ok, thanks again @shakilhossain1 perhaps you can show me how to solve this issue using codex (if it can solve it) on Monday? |
|
leaving this for you @shakilhossain1 to pick up. |

Motivation
Description
src/components/location-detail/street-view.tsx, including explicitwidth/height,loading="eager",fetchPriority="high", anddecoding="async"on the preview image to improve LCP.shouldLoadMapflag insrc/components/map.tsxthat only mountsAPIProviderwhen on desktop or when the map is actually visible on mobile, and recomputes on resize.Testing
npm run check-typesand it completed successfully.npm run check-format(Prettier) and it reported no formatting issues.npm run test:unit:run(Vitest) and all unit tests passed.npm run buildfailed in CI due to inability to fetch the Inter font from Google Fonts during the build environment network call (not a change in application code).Codex Task