Skip to content

Commit 690a0d4

Browse files
alesiahilvkozio
andauthored
fix(map-ruler): disable layer clicks during measurement (#1236)
* fix(map-ruler): disable layer clicks during measurement * test(map-ruler): mock registerMapListener correctly * Delete src/features/map_ruler/renderers/MapRulerRenderer.test.ts --------- Co-authored-by: VK <112831093+vkozio@users.noreply.github.com>
1 parent 7a78545 commit 690a0d4

File tree

4 files changed

+21
-16
lines changed

4 files changed

+21
-16
lines changed

docs/architecture/ADR-002-MapPopover-Event-System-Integration.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ graph TD
4545
A["MapLibre Click Event"] --> B["ConnectedMap Event Handler"]
4646
4747
B --> D["Priority Chain Execution"]
48-
D --> E1["Map Ruler (P:1)"]
48+
D --> E1["Map Ruler (P:100)"]
4949
D --> E2["Draw Tools (P:10)"]
5050
D --> E3["Boundary Selector (P:50)"]
5151
D --> E4["Legacy Renderers (P:60)"]
@@ -68,7 +68,7 @@ graph TD
6868
**Problem 1: Event System Conflicts**
6969

7070
- MapPopover operates independently of priority system
71-
- Tools like Map Ruler (priority 1) and Draw Tools (priority 10) can block all other interactions via `return false`
71+
- Tools like Map Ruler (priority 100) and Draw Tools (priority 10) can block all other interactions via `return false`
7272
- MapPopover still triggers despite tool exclusivity, creating UX inconsistencies
7373

7474
**Problem 2: Duplicate Event Handling**
@@ -103,7 +103,7 @@ useMapPopoverInteraction({
103103

104104
| System | Priority | Propagation Behavior | Integration Point |
105105
| ----------------- | -------- | ---------------------------- | ------------------------------------------------------------------------------------------------------ |
106-
| Map Ruler | 1 | `return false` (blocks all) | [`MapRulerRenderer.ts:109`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L109) |
106+
| Map Ruler | 100 | `return false` (blocks all) | [`MapRulerRenderer.ts:110`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L110-L112) |
107107
| Draw Tools | 10 | `return false` (blocks all) | [`DrawModeRenderer.ts:181`](../../src/core/draw_tools/renderers/DrawModeRenderer.ts#L181) |
108108
| Boundary Selector | 50 | `return false` (blocks all) | [`clickCoordinatesAtom.ts:27`](../../src/features/boundary_selector/atoms/clickCoordinatesAtom.ts#L27) |
109109
| Legacy Renderers | 60 | `return true` (non-blocking) | [`GenericRenderer.ts:248`](../../src/core/logical_layers/renderers/GenericRenderer.ts#L248) |
@@ -272,7 +272,7 @@ For ConnectedMap integration, the service call uses existing priority system:
272272

273273
| System | Priority | Behavior | Integration Method |
274274
| ----------------- | -------- | ------------------------ | -------------------------------------- |
275-
| Map Ruler | 1 | Blocks all (unchanged) | Existing priority listener |
275+
| Map Ruler | 100 | Blocks all (unchanged) | Existing priority listener |
276276
| Draw Tools | 10 | Blocks all (unchanged) | Existing priority listener |
277277
| Boundary Selector | 50 | Blocks all (unchanged) | Existing priority listener |
278278
| **MapPopover** | **55** | **Non-blocking** | **Service call via priority listener** |

docs/investigations/R006-map-event-management.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ graph TD
4343
D --> E6
4444
4545
subgraph "Consumer Systems"
46-
F1["Map Ruler<br/>(Priority: 1)"]
46+
F1["Map Ruler<br/>(Priority: 100)"]
4747
F2["Draw Tools<br/>(Priority: 10)"]
4848
F2_5["Boundary Selector<br/>(Priority: 10)"]
4949
F3["Generic/Feature Renderers<br/>(Priority: 60)"]
@@ -139,7 +139,7 @@ Each listener function returns a boolean value that controls event propagation:
139139
**Example Implementation (Blocking)**:
140140

141141
```typescript
142-
// From MapRulerRenderer (Priority: 1)
142+
// From MapRulerRenderer (Priority: 100)
143143
function preventClicking(e) {
144144
e.preventDefault();
145145
return false; // STOPS all other listeners
@@ -218,7 +218,7 @@ sequenceDiagram
218218
participant M as MapLibre Map
219219
participant C as ConnectedMap
220220
participant R as mapListenersAtom
221-
participant L1 as Map Ruler<br/>(P: 1)
221+
participant L1 as Map Ruler<br/>(P: 100)
222222
participant L2 as Draw Tools<br/>(P: 10)
223223
participant L3 as Boundary Selector<br/>(P: 10)
224224
participant L4 as Generic Renderer<br/>(P: 60)
@@ -268,7 +268,7 @@ Based on a full codebase analysis, these are the current priority assignments:
268268

269269
| System | Priority | Events | Location | Stops Propagation? |
270270
| :------------------------ | :------- | :--------------------------------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :----------------- |
271-
| **Map Ruler** | 1 | `click`, `mousemove` | [`MapRulerRenderer.ts:109`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L109) ||
271+
| **Map Ruler** | 100 | `click`, `mousemove` | [`MapRulerRenderer.ts:110`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L110-L112) ||
272272
| **Draw Tools** | 10 | `click`, `mousemove` | [`DrawModeRenderer.ts:181`](../../src/core/draw_tools/renderers/DrawModeRenderer.ts#L181) ||
273273
| **Boundary Selector** | 10 | `click`, `mousemove` | [`clickCoordinatesAtom.ts:27`](../../src/features/boundary_selector/atoms/clickCoordinatesAtom.ts#L27) ||
274274
| **Map Popover** | 55 | `click` | [`useMapPopoverPriorityIntegration.ts`](../../src/core/map/hooks/useMapPopoverPriorityIntegration.ts)<br/>[`ConnectedMap.tsx`](../../src/components/ConnectedMap/ConnectedMap.tsx) ||
@@ -287,7 +287,7 @@ This section provides a detailed breakdown of what each registered listener does
287287

288288
These systems are designed to take exclusive control of the map and prevent any other interactions.
289289

290-
1. **Map Ruler** (Priority: 1)
290+
1. **Map Ruler** (Priority: 100)
291291

292292
- **Location**: `MapRulerRenderer.ts:109`
293293
- **`click` handler**: Calls `event.preventDefault()` and returns `false`.

docs/investigations/map-event-management-system.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ graph TD
4040
D --> E3
4141
4242
subgraph "Consumer Systems"
43-
F1["Map Ruler<br/>(Priority: 1)"]
43+
F1["Map Ruler<br/>(Priority: 100)"]
4444
F2["Draw Tools<br/>(Priority: 10)"]
4545
F2_5["Boundary Selector<br/>(Priority: 50)"]
4646
F3["Generic/Feature Renderers<br/>(Priority: 60)"]
@@ -128,7 +128,7 @@ Each listener function returns a boolean value that controls event propagation:
128128
**Example Implementation (Blocking)**:
129129

130130
```typescript
131-
// From MapRulerRenderer (Priority: 1)
131+
// From MapRulerRenderer (Priority: 100)
132132
function preventClicking(e) {
133133
e.preventDefault();
134134
return false; // STOPS all other listeners
@@ -168,7 +168,7 @@ sequenceDiagram
168168
participant M as MapLibre Map
169169
participant C as ConnectedMap
170170
participant R as mapListenersAtom
171-
participant L1 as Map Ruler<br/>(P: 1)
171+
participant L1 as Map Ruler<br/>(P: 100)
172172
participant L2 as Draw Tools<br/>(P: 10)
173173
participant L3 as Boundary Selector<br/>(P: 50)
174174
participant L4 as Generic Renderer<br/>(P: 60)
@@ -211,7 +211,7 @@ Based on a full codebase analysis, these are the current priority assignments:
211211

212212
| System | Priority | Events | Location | Stops Propagation? |
213213
| :------------------------ | :------- | :------------------- | :------------------------------------------------------------------------------------------------------------------------------------------ | :----------------- |
214-
| **Map Ruler** | 1 | `click`, `mousemove` | [`MapRulerRenderer.ts:109`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L109) ||
214+
| **Map Ruler** | 100 | `click`, `mousemove` | [`MapRulerRenderer.ts:110`](../../src/features/map_ruler/renderers/MapRulerRenderer.ts#L110-L112) ||
215215
| **Draw Tools** | 10 | `click`, `mousemove` | [`DrawModeRenderer.ts:181`](../../src/core/draw_tools/renderers/DrawModeRenderer.ts#L181) ||
216216
| **Boundary Selector** | 10 | `click`, `mousemove` | [`clickCoordinatesAtom.ts:27`](../../src/features/boundary_selector/atoms/clickCoordinatesAtom.ts#L27) ||
217217
| **Active Contributors** | 60 | `click` | [`GenericRenderer.ts:231`](../../src/core/logical_layers/renderers/GenericRenderer.ts#L231) ||
@@ -229,7 +229,7 @@ This section provides a detailed breakdown of what each registered listener does
229229

230230
These systems are designed to take exclusive control of the map and prevent any other interactions.
231231

232-
1. **Map Ruler** (Priority: 1)
232+
1. **Map Ruler** (Priority: 100)
233233

234234
- **Location**: `MapRulerRenderer.ts:109`
235235
- **`click` handler**: Calls `event.preventDefault()` and returns `false`.

src/features/map_ruler/renderers/MapRulerRenderer.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,12 @@ export class MapRulerRenderer extends LogicalLayerDefaultRenderer {
107107
function preventMousemove(e) {
108108
return false;
109109
}
110-
this._removeClickListener = registerMapListener('click', preventClicking, 1);
111-
this._removeMousemoveListener = registerMapListener('mousemove', preventMousemove, 1);
110+
// Use the highest priority to block map interactions from other layers
111+
this._removeClickListener = registerMapListener('click', preventClicking, 100);
112+
this._removeMousemoveListener = registerMapListener(
113+
'mousemove',
114+
preventMousemove,
115+
100,
116+
);
112117
}
113118
}

0 commit comments

Comments
 (0)