ERA-13723: Blue start/end patrol pins with patrol-name labels by tiffanynwong · Pull Request #1666 · PADAS/das-web-react

tiffanynwong · 2026-07-20T22:59:07Z

Summary

Updates the patrol start and end map markers:

Shared blue pins replace the per-patrol-type icon at the track endpoints — a play glyph (▶) for start and a square (■) for end, in the design blue (#0056C7) with a white border.
Labels show the patrol name instead of "Patrol Start" / "Patrol End": patrol title → patrol type display name → the generic "Patrol Start" / "Patrol End" fallback (the (Est) suffix for estimated points is preserved).
The name renders to the right of the pin, vertically centered, as black text with a white halo (no background chip).
Pins render 32px tall at full zoom and scale down when zoomed out, matching the other map icons.

New assets

src/common/images/icons/patrol-start-pin.svg
src/common/images/icons/patrol-end-pin.svg

Notes for reviewers

ClustersLayer/utils.js (out of scope): removes a pre-existing duplicate import from ../constants that was preventing the app from compiling on this branch. Included because it blocks the build; happy to split it out if preferred.
Reliability fix: the start/stop symbol layer is added by a memoized child whose add-layer effect runs before the parent creates the map source. When patrol data is already loaded at mount, the source didn't exist yet and the pins silently failed to attach. The layer now waits for the source (via a sourcedata event) before mounting the symbol child.

Testing

yarn test — src/utils/patrols.test.js passes.
yarn lint — clean on changed files.
Verified locally against a dev tenant: start/end pins render with patrol-name labels; confirmed the layer attaches reliably and the icon-size expression is accepted by Mapbox.

Follow-ups / open questions

Long patrol names wrap to two lines (existing Mapbox label behavior); not truncated with an ellipsis like the mock. Can cap to one line if desired.
Dropped the pin drop-shadow so the pin fills its icon box and matches other icon heights — can re-add a subtle shadow if wanted.

🤖 Generated with Claude Code

Replace the per-patrol-type icon at patrol track endpoints with two shared blue map pins (play glyph = start, square = end) and label each with the patrol name (title, else patrol type display name, else "Patrol Start" / "Patrol End"). The name renders to the right of the pin as black text with a white halo, no background chip. Also make the start/stop symbol layer attach reliably when the patrol data is already loaded at mount (wait for the source via a sourcedata event before mounting the memoized symbol-layer child). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Updates patrol track endpoint rendering to use shared blue start/end pin assets and to label those pins with the patrol name, while also addressing a Mapbox source/layer ordering issue that could prevent the start/stop symbol layer from attaching.

Changes:

Replace per-patrol-type endpoint icons with shared start/end pin SVG assets, loaded as Mapbox images.
Update start/end marker labels to prefer patrol title, then patrol type display name, with generic fallbacks (preserving “(Est)”).
Add a “wait for source” gate before mounting the memoized symbol-layer child to improve layer-attach reliability.

Reviewed changes

Copilot reviewed 2 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/utils/patrols.js	Switches start/end point feature `image` to new pin assets and updates label selection logic.
src/PatrolStartStopLayer/layer.js	Adjusts symbol styling/layout for right-of-pin labels and adds source-ready gating before mounting the symbol layer.
src/common/images/icons/patrol-start-pin.svg	New blue start pin SVG asset (play glyph).
src/common/images/icons/patrol-end-pin.svg	New blue end pin SVG asset (square glyph).

+  const { data: { patrolTypes } } = store.getState();
+  const matchingPatrolType = patrolType
+    ? (patrolTypes || []).find(({ value, id }) => value === patrolType || id === patrolType)
+    : null;
+  const patrolName = patrol.title || matchingPatrolType?.display || null;


+  const patrolName = patrol.title || matchingPatrolType?.display || null;
+  const startLabel = patrolName || 'Patrol Start';
+  const endLabel = patrolName || 'Patrol End';
+


+  const [isSourceReady, setIsSourceReady] = useState(false);
+  useEffect(() => {
+    if (!map) return undefined;
+
+    const handleSourceData = () => {
+      if (map.getSource(sourceId)) {
+        setIsSourceReady(true);
+        map.off('sourcedata', handleSourceData);
+      }
+    };
+    map.on('sourcedata', handleSourceData);
+
+    return () => map.off('sourcedata', handleSourceData);
+  }, [map, sourceId]);


luixlive

Thanks Tiffany 😄 Nice PR, sorry it took me that much to review it. I have some feedback for it, but most of it can be delegated to Claude. Just ask it to read my comments from the PR and fix them.

Three high level comments:
1- Tests are broken after these changes 🥲 You can ask Claude to see what's going on and fix them. Then you can make sure they pass with yarn test.
2- This branch is a bit old. Let's make sure to update develop and merge it here (you can ask Claude to do all this). And there are a couple conflicts we need to fix.
3- We are missing unit tests for the changes.

The rest are trivial comments, a couple from me, a bunch from a local Claude review. Most seem trivial to fix!

Edit: btw, I also see a couple items from Copilot that we should work on.

luixlive · 2026-08-20T17:22:55Z

  'line-cap': 'round',
 };

+// The patrol name renders on the icon layer (to the right of the pin) as black


I see too much commentary. We've asked the agents to avoid unnecessary blocks of comments when doing changes, but I think this PR was sent before I updated the AGENTS.md 😄

luixlive · 2026-08-20T18:56:26Z

  isPatrolDone) {
    patrol_points.end_location = cloneDeep(patrol_points.start_location);
-    patrol_points.end_location.properties.title = 'Patrol End (Est)';
+    patrol_points.end_location.properties.image = patrolEndPin;


Claude:
The new end-pin assignment in the done-patrol clone branch is unconditionally dead code — the cloned end_location is always deleted three lines later, so this case never gets the square end glyph the PR is about.

luixlive · 2026-08-20T18:56:43Z

+    if (patrol_points.start_location.properties.title !== patrol_points.end_location.properties.title) {
+      patrol_points.start_location.properties.title += ` & ${patrol_points.end_location.properties.title}`;
+    }
    delete patrol_points.end_location;


Claude:
When start and end coincide, delete patrol_points.end_location keeps patrolStartPin, so the new end glyph is silently dropped and — with the new equality guard — the label no longer says the patrol ended there either.

luixlive · 2026-08-20T18:57:06Z

+  // Label the start/end markers with the patrol's name (title, else patrol type
+  // display name), falling back to the generic "Patrol Start"/"Patrol End".
+  const patrolType = firstLeg?.patrol_type;
+  const { data: { patrolTypes } } = store.getState();


Claude:
patrolTypes is read via store.getState() from inside a reselect compute function, so it is not a memoization input and stale generic labels are never recomputed.

luixlive · 2026-08-20T18:57:25Z

      point(patrol_points.start_location.geometry.coordinates),
    )) {
-    patrol_points.start_location.properties.title += ` & ${patrol_points.end_location.properties.title}`;
+    if (patrol_points.start_location.properties.title !== patrol_points.end_location.properties.title) {


Claude:
The new title !== title guard compares titles after the ' (Est)' suffix is appended, so a coincident start/end renders the patrol name twice in one label.

luixlive · 2026-08-20T18:58:11Z


+  // Label the start/end markers with the patrol's name (title, else patrol type
+  // display name), falling back to the generic "Patrol Start"/"Patrol End".
+  const patrolType = firstLeg?.patrol_type;


Claude:
The 8-line patrol-name derivation re-implements displayTitleForPatrol from the same file but drops its leader-name fallback, so the map pin and the track legend show different names for the same patrol.

luixlive · 2026-08-20T18:58:25Z


  if (start_location) {
-    patrol_points.start_location = makePatrolPointFromFeature('Patrol Start', [start_location.longitude, start_location.latitude], icon_id, stroke, start_time);
+    patrol_points.start_location = makePatrolPointFromFeature(startLabel, [start_location.longitude, start_location.latitude], patrolStartPin, stroke, start_time);


Claude:
Replacing the per-patrol-type sprite with one shared blue pin, plus dropping the per-patrol label color, leaves every patrol's endpoints visually identical and desynchronizes the map markers from the track legend.

luixlive · 2026-08-20T18:58:58Z

+const iconLayout = {
+  'icon-anchor': 'bottom',
+  'icon-size': PIN_ICON_SIZE,
  'text-field': '{title}',


Claude:
Moving 'text-field': '{title}' onto the icon layer bypasses mapUserLayoutConfigByLayerId, so Settings → Map → Map Markers → Patrols no longer hides patrol start/end names.

luixlive · 2026-08-20T18:59:11Z

+  // the memoized child never re-renders to retry. Wait for the source to exist
+  // (signalled by a sourcedata event) before mounting the child so its layer
+  // reliably attaches.
+  const [isSourceReady, setIsSourceReady] = useState(false);


Claude:
isSourceReady is a one-way latch fed by a global sourcedata event with no synchronous check and no re-arm, so it both delays and can permanently prevent the symbol layer from attaching.

luixlive · 2026-08-20T18:59:33Z

+        .filter(Boolean)
+        .forEach(({ properties: { image } = {} }) => {
+          if (image && !map.hasImage(calcImgIdFromUrlForMapImages(image))) {
+            addMapImage({ src: image });


Claude:
Two static app-bundled pins are registered from a per-patrol effect keyed on a churning patrolData, with an un-awaited addMapImage and no in-flight guard, instead of once at map init.

tiffanynwong force-pushed the ERA-13723 branch from 8ee7f52 to 319bcf2 Compare July 21, 2026 21:23

tiffanynwong requested review from Copilot and luixlive July 21, 2026 21:29

Copilot started reviewing on behalf of tiffanynwong July 21, 2026 21:29 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

luixlive requested changes Aug 20, 2026

View reviewed changes

Navigation Menu

Uh oh!

ERA-13723: Blue start/end patrol pins with patrol-name labels - #1666

ERA-13723: Blue start/end patrol pins with patrol-name labels#1666
tiffanynwong wants to merge 1 commit into
developfrom
ERA-13723

tiffanynwong commented Jul 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

luixlive left a comment •

edited

Loading

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

luixlive Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tiffanynwong commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

New assets

Notes for reviewers

Testing

Follow-ups / open questions

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

luixlive left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tiffanynwong commented Jul 20, 2026 •

edited

Loading

luixlive left a comment •

edited

Loading