Skip to content

Wire the detail drawer, compare sheet, share card, Street View preview and category icon into the map - #338

Merged
yakew7 merged 3 commits into
StudentSuite:mainfrom
propcgamer20-png:feat/wire-detail-compare
Nov 6, 2026
Merged

yakew7 merged 3 commits into
StudentSuite:mainfrom
propcgamer20-png:feat/wire-detail-compare

Conversation

@propcgamer20-png

Copy link
Copy Markdown
Member

What this changes

Part of #313. Wires five more of the standalone components that no page imported yet. Each is its own checkbox in that issue.

  • PlaceDetailDrawer: every row in the results list gets an info button ("Details for ") that opens the drawer; PlacesMap holds the open place. The drawer is now scrollable.
  • SharePlaceCard and PlacePhotoGallery: shown inside that drawer, under the directions button (copy a link to the focused pin, copy name and address as text, and a Street View preview with the component's own "not every location has coverage" note).
  • ComparePlacesSheet: each row gets a "Compare " checkbox (a native input, labelled); ticking 2 or 3 enables a "Compare (n)" button above the list, and the sheet shows the places side by side with pairwise distances. A fourth tick is disabled, and "Clear selection" resets. Selection lives in PlacesMap, so it survives filtering as long as the places exist.
  • CategoryIcon: replaces the plain colour dot in the results rows, the pin popup's type chip, the drawer and the compare sheet, which is the duplication the issue pointed at. The icon is tinted with the same PLACE_TYPE_COLORS, so the colour still matches the pin. The category chips keep their dots on purpose: they are the map legend for the clustered pie pins.

Not touched: PlaceReviewBadge and RecentAdditionsFeed need data the schema does not have (ratings, added-on dates); HeatmapToggle needs heatmap rendering first. #313 stays open for those three.

This branch is stacked on #337 (country filter), because both change places-map.tsx and map-panel.tsx; that commit drops out of this diff when #337 merges.

Proof

  • New tests: results-list.test.tsx (+3: details button, nothing extra by default, compare needs two picks and caps at three) and places-map.test.tsx (+2: the drawer opens with the Street View card and copy button, two picked places open the compare sheet).
  • npm run typecheck, npm run lint, npm run format:check, npm run build clean; unit tests: 72 files, 685 pass.

Not verified

I did not open /map in a browser. Not checked by eye: how the drawer and the compare sheet sit next to the map on a phone, whether the Street View iframe renders for a given place (Google serves it, and it can be blank where there is no coverage, as the component itself says), or the extra checkbox and info button making rows tight in the narrow sidebar.

Type of change

  • Code or fix
  • New public place(s)
  • New or updated benefit guide
  • New or updated resource link
  • Docs

@vercel

vercel Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

@propcgamer20-png is attempting to deploy a commit to the studentsuite Team on Vercel.

A member of the Team first needs to authorize it.

@AnayDhawan

Copy link
Copy Markdown
Member

Thanks for the PR. Held for a rebase, not rejected.

#337 was squash-merged as 9a1fca7, so the stacked commit in this branch no longer matches main. Merging main into this branch conflicts in:

  • src/components/map/places-map.tsx
  • src/components/map/places-map.test.tsx

Please rebase onto current main (or git rebase --onto origin/main <last #337 commit>), keep both the country filter from #337 and the drawer/compare wiring from this PR, then re-run typecheck, lint and unit tests.

What I checked on the branch as it stands: typecheck clean, 685 of 685 unit tests pass. Not checked: the drawer, compare sheet and Street View card in a browser, which the PR description also lists as unverified. I will look at those after the rebase.

@AnayDhawan

Copy link
Copy Markdown
Member

I rebased this locally to check it. git rebase --onto origin/main 32b1d21 drops the stacked #337 commit and applies cleanly, with no conflicts. They only show up when merging main in, because #337 was squashed.

On the rebased branch, typecheck, lint, format:check, and build all pass, and so do the unit tests. The results-list paging test goes from 2.3s to 3.2s with the extra row controls. That is still under the 5s timeout, though it timed out once in a full local run under load.

I also opened /map in a browser at 1280px and 390px:

  • The details button opens the drawer with the category chip, the Street View preview, and the share card.
  • Two compare ticks enable "Compare (2)", and the sheet shows both places plus the distance between them.

One thing from that check: with the checkbox and three icon buttons, names in the desktop sidebar are cut to about 14 characters. Mumbai Terminal 1 and Terminal 2 look identical in the list. That doesn't block this PR; a title attribute or a two-line clamp would fix it.

Please run the rebase above and force-push your branch. Once it's in, this is ready to merge.

@propcgamer20-png

Copy link
Copy Markdown
Member Author

Status update (2026-11-06): the conflict with main is resolved. #337 landed while this branch was stacked on it, and both touch places-map.tsx and its test. I merged main in, kept the drawer and compare code, and re-ran everything: typecheck, lint, prettier and the full unit suite (73 files, 689 tests) pass.

Still not checked by eye: how the drawer and the compare sheet sit next to the map on a phone, and whether the Street View iframe renders for a given place. That is described in the PR body.

@yakew7
yakew7 merged commit 52dc644 into StudentSuite:main Nov 6, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants