Track selected school across map views and map state for back-button return to map from school detail #241

thomhickey · 2024-09-28T01:57:05Z

MapContext keeps track of the selected school and the state of the map. This enables a ux where once a user clicks through to a school and then uses the browser back button, they are returned to exactly where they were. Note that MapContext is available across back-button refresh of pages via a combination of React + Next.js caching. Typically of course this would not work as a back-button navigation bootstraps the entire app again, clearing any context. A hard-refresh or nav-landing onto a page will not preserve context, which is what we want.

Details:

uses React Provider pattern to provide map context to components. context includes the state of the map view (map or list) and what the selected school is along with a setter function to set the selected school. this context can be used on back-button return to the map to reset the user's last view and last selected school.
map and list view now persist the selected school across the two views.
removes the /map#list hashed route solution for back button return to previous view.
removes the need to set className on the div id="root" element from the map, instead we can also use MapContext from RootLayout to determine how to set the h-auto and h-dvh-with-fallback css classes.
tracks initial interaction of the user with the map so that on back-button return to the map view we don't zoom/flyTo as it is dizzying to the user.
mapbox is now torn-down correctly to avoid leaks, etc.

Merge Develop to Main

…ady there

vercel · 2024-09-28T01:57:08Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
support-sfusd	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 28, 2024 4:03pm

thomhickey · 2024-09-28T16:18:09Z

Here's a screen grab of the interaction

map.context.mov

thomhickey · 2024-09-28T18:21:40Z

.gitignore

@@ -30,6 +30,7 @@ yarn-error.log*

 # vercel
 .vercel
+.vscode


should be standard in our .gitignore?

missed this ... we need to whitelist some files. will demo in upcoming PR.

thomhickey · 2024-09-28T18:22:17Z

src/components/MapList.tsx

@@ -4,7 +4,7 @@ import { School } from "@/types/school";

 type MapListProps = {
  schools: School[];
-  setSelectedSchool: (school: School | false | null) => void;
+  setSelectedSchool: (school: School | null) => void;


false no longer needed/valid

thomhickey · 2024-09-28T18:26:44Z

src/components/MapListCard.tsx

  const handleTransitionEnd = (e: React.TransitionEvent<HTMLDivElement>) => {
    if (e.propertyName === "max-height" && isExpanded) {
      cardRef.current?.scrollIntoView({ behavior: "smooth", block: "nearest" });
    }
  };

  function onClick(e: React.MouseEvent<HTMLDivElement>) {
+    if (learnMoreRef.current && learnMoreRef.current.contains(e.target as Node)) {


this was causing a setSelectedSchool(null), which we don't want because then we lose the context

thomhickey · 2024-09-28T18:29:44Z

src/components/MapListCard.tsx

    if (isExpanded) {
      setSelectedSchool(null);
    } else {
      setSelectedSchool(school);
    }
  }

+  useEffect(() => {
+    if (selectedSchool && selectedSchool.id === school.id) {


scrolls to selected school on back-button return

thomhickey · 2024-09-28T18:39:13Z

src/components/MapboxMap.tsx

@@ -56,7 +63,8 @@ const MapboxMap = ({

    mapRef.current = map;
    map.on("click", () => {
-      setSelectedSchool(false);
+      setSelectedSchool(null);
+      userHasInteracted.current = true;


needed to differentiate between back-button return and hard landing on the map

thomhickey · 2024-09-28T18:42:53Z

tsconfig.json

@@ -20,7 +20,8 @@
    ],
    "paths": {
      "@/*": ["./src/*"]
-    }
+    },
+    "sourceMap": true


enables front-end debugging inside vscode

thomhickey · 2024-09-28T19:17:51Z

src/components/MapboxMap.tsx

    });
+
+    return () => {


teardown function tears down mapbox on close, avoiding memory leaks, etc.

thomhickey · 2024-09-28T19:19:14Z

src/components/MapboxMap.tsx

@@ -169,26 +179,49 @@ const MapboxMap = ({
      new mapboxgl.Marker(bayBridgeEl)
        .setLngLat([-122.3778, 37.7983])
        .addTo(map);
+
+      setMapLoaded(true);


to avoid race condition where useEffect runs before map.on("load") which prevented scrolling to correct school in list on back-button refresh

BeeSeeWhy

The changes look good. Thanks for the breakdown of everything in the PR!

nickvisut · 2024-10-03T22:35:01Z

src/components/MapboxMap.tsx

  useEffect(() => {
-    if (mapRef.current && selectedSchool && typeof selectedSchool !== 'boolean') {
+    // check mapLoaded to avoid race condition where markersRef is not yet initialized
+    if (mapLoaded && mapRef.current && selectedSchool) {


nickvisut · 2024-10-03T22:37:41Z

src/components/MapboxMap.tsx

+  const userHasInteracted = useRef(false);
+
+  const flyToOptions = useMemo(() => ({
+    zoom: 14, // zoom level to fly to


I'm noticing that we are always zooming. I think it's best not to change zoom unless the user explicitly requests it.

Yeah @nickvisut, it can get zoomy, happy to change anything. I already worked on the settings to slow down the zoom effect to make it less dizzying,. But we can remove the auto-zoom completely. Design team didn't ask for any changes, but maybe they don't know how easy it is to change, not sure.

maybe with 100+ schools it might actually make more sense to auto-zoom as the pins will be right on top of each other. i still can't imagine so many pins in my mind's eye, it is going to look awfully crowded.

Agreed that that could be a future problem, but the experience as of now is suboptimal. Consider: A user arrives at the map and clicks on a school. How do they then click on another school? They will have to manually zoom out in most cases.

There may be a good solution that is the best of both worlds, but let's lean towards best UX w/current set of schools while we figure that out.

nickvisut · 2024-10-04T21:57:58Z

src/pages/map.tsx

-    // the empty instruction card)
-    /* TODO: consider using something more descriptive like an enum instead */
-    setSelectedSchool(false);
+    setSelectedSchool(null);


Double checking here: Is this no longer needed?

(false vs null as per the comments)

nickvisut · 2024-10-05T03:15:46Z

Thanks for putting up with my post-hoc code review 🙂

nickvisut · 2024-10-05T03:17:10Z

The changes look good. Thanks for the breakdown of everything in the PR!

💯 on the detailed breakdown!

nickvisut and others added 11 commits September 3, 2024 17:41

Merge pull request #215 from sfbrigade/develop

2ee7351

Merge Develop to Main

fix: back button loses map context

eaac2ee

fix: not needed anymore

80192c3

feat: source maps for debugging + vscode in gitignore

b2847b2

fix: remove Back to button, wasn't asked for and browser back is alre…

3d243cf

…ady there

fix: root element css class logic no breaking changes

902b7e9

fix: restore comment

4a7e0de

fix: comment not needed anymore

9d919e1

fix: cq

389f541

fix: cq

0aa7ead

Merge branch 'develop' into feat/list-route-redo

989c813

fix: auto-scroll to selected school when returning to list view

f6bd79e

vercel bot deployed to Preview September 28, 2024 06:50 View deployment

fix: don't flyTo on back button return, it's dizzying

15ec5c5

vercel bot deployed to Preview September 28, 2024 15:58 View deployment

feat: comments

65e9995

vercel bot deployed to Preview September 28, 2024 16:03 View deployment

thomhickey changed the title ~~Adds map context using React provider pattern~~ Track selected school across map views and map state for back-button return to map from school detail Sep 28, 2024

thomhickey marked this pull request as ready for review September 28, 2024 16:28

thomhickey commented Sep 28, 2024

View reviewed changes

thomhickey requested review from nickvisut and BeeSeeWhy September 28, 2024 19:28

thomhickey mentioned this pull request Oct 3, 2024

restore page height fix #244

Closed

BeeSeeWhy approved these changes Oct 3, 2024

View reviewed changes

BeeSeeWhy merged commit 5289a2c into develop Oct 3, 2024
2 checks passed

nickvisut reviewed Oct 3, 2024

View reviewed changes

nickvisut reviewed Oct 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track selected school across map views and map state for back-button return to map from school detail #241

Track selected school across map views and map state for back-button return to map from school detail #241

thomhickey commented Sep 28, 2024 •

edited

Loading

vercel bot commented Sep 28, 2024 •

edited

Loading

thomhickey commented Sep 28, 2024

thomhickey Sep 28, 2024 •

edited

Loading

nickvisut Oct 5, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

thomhickey Sep 28, 2024

BeeSeeWhy left a comment

nickvisut Oct 3, 2024

nickvisut Oct 3, 2024

thomhickey Oct 4, 2024

thomhickey Oct 4, 2024

nickvisut Oct 4, 2024

nickvisut Oct 4, 2024

nickvisut Oct 4, 2024

nickvisut commented Oct 5, 2024

nickvisut commented Oct 5, 2024

Track selected school across map views and map state for back-button return to map from school detail #241

Track selected school across map views and map state for back-button return to map from school detail #241

Conversation

thomhickey commented Sep 28, 2024 • edited Loading

vercel bot commented Sep 28, 2024 • edited Loading

thomhickey commented Sep 28, 2024

thomhickey Sep 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BeeSeeWhy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickvisut commented Oct 5, 2024

nickvisut commented Oct 5, 2024

thomhickey commented Sep 28, 2024 •

edited

Loading

vercel bot commented Sep 28, 2024 •

edited

Loading

thomhickey Sep 28, 2024 •

edited

Loading