Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions scripts/tests/browser_smoke.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,54 @@ try {
await page.locator('.analysis-history__empty').waitFor();
assert.equal(await artifactCard(page, 'Renamed A-only').count(), 0);

const markerA = page.locator('.analysis-marker--a');
await markerA.waitFor();
assert.equal(
await markerA.getAttribute('title'),
'Map marker. Drag to adjust the location.',
);
const markerBounds = await markerA.boundingBox();
assert.ok(markerBounds, 'Draggable Point A marker must have visible bounds');
const pointRefreshRequestsBeforeDrag = networkControl.pointRefreshRequests;
const markerCenter = {
x: markerBounds.x + markerBounds.width / 2,
y: markerBounds.y + markerBounds.height / 2,
};
await page.mouse.move(
markerCenter.x,
markerCenter.y,
);
await page.mouse.down();
await page.mouse.move(
markerBounds.x + markerBounds.width / 2 + 36,
markerBounds.y + markerBounds.height / 2 + 18,
{ steps: 6 },
);
await page.waitForTimeout(500);
assert.equal(
networkControl.pointRefreshRequests,
pointRefreshRequestsBeforeDrag,
'Holding a dragged marker still must not start Crime calculation',
);
await page.mouse.up();
await page.waitForTimeout(150);
assert.equal(
networkControl.pointRefreshRequests,
pointRefreshRequestsBeforeDrag,
'The marker settle delay must elapse before Crime calculation starts',
);
await page.waitForResponse((response) => {
const request = response.request();
return request.url().startsWith('https://phl.carto.com/')
&& /format=GeoJSON/i.test(decodeURIComponent(request.postData() || ''));
});
assert.equal(
networkControl.pointRefreshRequests - pointRefreshRequestsBeforeDrag,
1,
'The final settled marker position must start exactly one Crime calculation',
);
assert.equal(await page.locator('#addrA').inputValue(), 'Map point A');

const layout = await page.evaluate(() => {
const side = document.getElementById('sidepanel');
const compare = document.getElementById('compare-card');
Expand Down
87 changes: 87 additions & 0 deletions scripts/tests/crime_async_contracts.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,93 @@ test('each refresh reads one snapshot and superseding aborts the previous genera
assert.deepEqual(await second, { applied: true });
});

test('manual cancellation aborts the active Crime refresh without starting another generation', async () => {
const gate = deferred();
let snapshotReads = 0;
let activeSignal = null;
const owner = createCrimeRefreshOwner({
readSnapshot: () => ({ sequence: ++snapshotReads }),
runRefresh: (_snapshot, context) => {
activeSignal = context.signal;
return gate.promise;
},
});

const refresh = owner.refresh();
assert.equal(typeof owner.cancel, 'function');
owner.cancel();

assert.equal(activeSignal.aborted, true);
assert.equal(snapshotReads, 1);
gate.resolve({ applied: true });
assert.deepEqual(await refresh, { applied: false });
});

test('marker dragging updates position immediately and refreshes once after the final settle delay', async () => {
const { wireSettledMarkerDrag } = await import('../../src/routes_crime/index.js');
assert.equal(typeof wireSettledMarkerDrag, 'function');

const listeners = new Map();
const marker = {
position: { lng: -75.16, lat: 39.95 },
on(event, listener) { listeners.set(event, listener); return this; },
off(event, listener) {
if (listeners.get(event) === listener) listeners.delete(event);
return this;
},
getLngLat() { return this.position; },
};
const timers = new Map();
const waits = [];
let nextTimerId = 1;
const scheduler = {
setTimeout(callback, wait) {
const id = nextTimerId++;
timers.set(id, callback);
waits.push(wait);
return id;
},
clearTimeout(id) { timers.delete(id); },
};
const dragStarts = [];
const moves = [];
const settled = [];
const dispose = wireSettledMarkerDrag(marker, {
scheduler,
onDragStart: () => dragStarts.push('start'),
onMove: (position) => moves.push(position),
onSettled: (position) => settled.push(position),
});

listeners.get('dragstart')();
marker.position = { lng: -75.17, lat: 39.96 };
listeners.get('drag')();
listeners.get('dragend')();
assert.deepEqual(dragStarts, ['start']);
assert.deepEqual(moves, [{ lng: -75.17, lat: 39.96 }]);
assert.deepEqual(settled, []);
assert.deepEqual(waits, [350]);
assert.equal(timers.size, 1);

listeners.get('dragstart')();
assert.equal(timers.size, 0);
marker.position = { lng: -75.18, lat: 39.97 };
listeners.get('drag')();
listeners.get('dragend')();
const [timerId, callback] = timers.entries().next().value;
timers.delete(timerId);
callback();

assert.deepEqual(moves, [
{ lng: -75.17, lat: 39.96 },
{ lng: -75.18, lat: 39.97 },
]);
assert.deepEqual(settled, [{ lng: -75.18, lat: 39.97 }]);
dispose();
assert.equal(timers.size, 0);
assert.equal(listeners.size, 0);
});

test('deactivation invalidates work and reactivation uses a fresh signal', async () => {
const runs = [];
const owner = createCrimeRefreshOwner({
Expand Down
2 changes: 2 additions & 0 deletions scripts/tests/i18n_contracts.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ test('localization runtime normalizes, persists, translates, and notifies', asyn
assert.equal(controller.getLanguage(), 'zh-CN');
assert.equal(controller.t('language.switch'), 'English');
assert.equal(controller.t('crime.findingPoint', { target: 'A' }), '正在查找 A 点…');
assert.equal(messages.en['map.marker'], 'Map marker. Drag to adjust the location.');
assert.equal(messages['zh-CN']['map.marker'], '地图标记,可拖动调整位置。');

const observed = [];
const unsubscribe = controller.subscribe((language) => observed.push(language));
Expand Down
2 changes: 1 addition & 1 deletion src/i18n/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const messagePairs = Object.freeze({

'map.resetExtent': ['Reset map extent', '重置地图范围'],
'map.canvas': ['Map', '地图'],
'map.marker': ['Map marker', '地图标记'],
'map.marker': ['Map marker. Drag to adjust the location.', '地图标记,可拖动调整位置。'],
'map.zoomIn': ['Zoom in', '放大'],
'map.zoomOut': ['Zoom out', '缩小'],
'map.resetBearing': ['Reset bearing to north', '重置为正北方向'],
Expand Down
3 changes: 3 additions & 0 deletions src/routes_crime/crime_refresh_owner.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ export function createCrimeRefreshOwner({ readSnapshot, runRefresh }) {
if (generation === requestGeneration) controller = null;
}
},
cancel() {
invalidate();
},
setActive(next) {
const shouldActivate = Boolean(next);
if (active === shouldActivate) return;
Expand Down
59 changes: 59 additions & 0 deletions src/routes_crime/draggable_marker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
export const MARKER_DRAG_SETTLE_MS = 350;

function readMarkerPosition(marker) {
const position = marker.getLngLat?.();
const lng = Number(position?.lng);
const lat = Number(position?.lat);
return Number.isFinite(lng) && Number.isFinite(lat) ? { lng, lat } : null;
}

export function wireSettledMarkerDrag(marker, {
scheduler = globalThis,
settleMs = MARKER_DRAG_SETTLE_MS,
isActive = () => true,
onDragStart = () => {},
onMove = () => {},
onSettled = () => {},
} = {}) {
let disposed = false;
let settleTimer = null;

const cancelSettle = () => {
if (settleTimer == null) return;
scheduler.clearTimeout(settleTimer);
settleTimer = null;
};
const handleDragStart = () => {
cancelSettle();
if (!disposed && isActive()) onDragStart();
};
const handleDrag = () => {
cancelSettle();
if (disposed || !isActive()) return;
const position = readMarkerPosition(marker);
if (position) onMove(position);
};
const handleDragEnd = () => {
cancelSettle();
if (disposed || !isActive()) return;
settleTimer = scheduler.setTimeout(() => {
settleTimer = null;
if (disposed || !isActive()) return;
const position = readMarkerPosition(marker);
if (position) onSettled(position);
}, settleMs);
};

marker.on('dragstart', handleDragStart);
marker.on('drag', handleDrag);
marker.on('dragend', handleDragEnd);

return () => {
if (disposed) return;
disposed = true;
cancelSettle();
marker.off('dragstart', handleDragStart);
marker.off('drag', handleDrag);
marker.off('dragend', handleDragEnd);
};
}
71 changes: 64 additions & 7 deletions src/routes_crime/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ import {
geometryBounds,
} from '../map/camera_fit.js';
import { describeCrimeDataScope } from '../ui/data_scope.js';
import { wireSettledMarkerDrag } from './draggable_marker.js';

export { wireSettledMarkerDrag };

const CRIME_LAYER_IDS = [
'districts-fill',
Expand Down Expand Up @@ -154,6 +157,8 @@ export async function initCrimeMode(map, {
const mapReady = waitForMapReady(map);
let markerA = null;
let markerB = null;
let disposeMarkerADrag = null;
let disposeMarkerBDrag = null;
let tractClickWired = false;
let districtClickWired = false;
let active = true;
Expand Down Expand Up @@ -403,26 +408,74 @@ export async function initCrimeMode(map, {
}

function syncComparisonOverlays({ centerLonLat, centerBLonLat, radiusM, queryMode }) {
if (queryMode !== 'buffer') {
removeBufferOverlay(map);
const removeMarkerA = () => {
disposeMarkerADrag?.();
disposeMarkerADrag = null;
markerA?.remove();
markerB?.remove();
markerA = null;
};
const removeMarkerB = () => {
disposeMarkerBDrag?.();
disposeMarkerBDrag = null;
markerB?.remove();
markerB = null;
};
const updateDraggedPoint = (target, { lng, lat }) => {
store.setComparisonPoint(target, lng, lat, t('crime.mapPoint', { target }));
const center = target === 'B' ? store.centerBLonLat : store.centerLonLat;
if (target === 'B') upsertBufferB(map, { centerLonLat: center, radiusM: store.radius });
else upsertBufferA(map, { centerLonLat: center, radiusM: store.radius });
};
const wireMarkerDrag = (marker, target) => wireSettledMarkerDrag(marker, {
isActive: () => active && isActive() && store.queryMode === 'buffer',
onDragStart: () => {
refreshOwner.cancel();
publishCurrentSelection(undefined, { origin: 'map' });
Comment on lines +431 to +433

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop an in-progress camera fit when dragging starts

If the user starts another adjustment during the 450 ms fitBoundsWithPanel animation launched by the previous settled drag, this cancellation only invalidates the refresh and does not stop the map animation or release the points controller's programmatic-move ownership. The map can therefore continue translating beneath a held pointer, moving the marker away from the intended drop position before dragend; stop the active camera transition on drag start or avoid initiating a fit for settled marker drags.

Useful? React with 👍 / 👎.

},
onMove: (position) => {
updateDraggedPoint(target, position);
publishCurrentSelection(undefined, { origin: 'drag' });
},
onSettled: (position) => {
updateDraggedPoint(target, position);
Comment on lines +439 to +440

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Invalidate pending settles when a point is cleared

If Point B is cleared shortly after being dropped, the panel sets centerBLonLat to null but waits 300 ms before refreshing, while this 350 ms settle callback remains armed. For example, clearing B more than 50 ms after dragend lets this callback run before the panel refresh; updateDraggedPoint then recreates B, so the delayed refresh observes the resurrected comparison and the user's clear action is undone. Cancel or version pending settles when a point is replaced or cleared, or verify that the target still exists before committing.

Useful? React with 👍 / 👎.

publishCurrentSelection(undefined, { origin: 'map' });
onPointChange(target);
void requestRefresh();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route the settled refresh through the coordinator

When dragging begins while a coordinator-owned refresh is in flight—including during initial Crime loading—refreshOwner.cancel() makes that request return superseded, which src/mode_coordinator.js does not settle. The replacement call here invokes the route's private requestRefresh() directly, bypassing coordinator status publication, so a successful drag refresh can leave the surface skeleton, aria-busy, and data status stuck in the loading state indefinitely; route this refresh through the coordinator or otherwise propagate its result to the status owner.

Useful? React with 👍 / 👎.

},
});

if (queryMode !== 'buffer') {
removeBufferOverlay(map);
removeMarkerA();
removeMarkerB();
return;
}
if (centerLonLat) {
markerA ||= createMapMarker({ color: '#c86b00', className: 'analysis-marker analysis-marker--a' });
if (!markerA) {
markerA = createMapMarker({
color: '#c86b00',
className: 'analysis-marker analysis-marker--a',
draggable: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Provide keyboard controls for draggable markers

For keyboard-only users, enabling MapLibre's pointer dragging does not provide any way to focus and move the marker: the added wiring listens only for drag events, while localizeMapMarker supplies only a title and aria-label. The label now advertises location adjustment that cannot be operated without a pointer, so add focusable keyboard movement controls (for example, arrow-key adjustments) or an equivalent accessible adjustment interface.

Useful? React with 👍 / 👎.

});
disposeMarkerADrag = wireMarkerDrag(markerA, 'A');
}
markerA.setLngLat(centerLonLat).addTo(map);
localizeMapMarker(markerA);
upsertBufferA(map, { centerLonLat, radiusM });
}
} else removeMarkerA();
if (centerBLonLat) {
markerB ||= createMapMarker({ color: '#0a6c74', className: 'analysis-marker analysis-marker--b' });
if (!markerB) {
markerB = createMapMarker({
color: '#0a6c74',
className: 'analysis-marker analysis-marker--b',
draggable: true,
});
disposeMarkerBDrag = wireMarkerDrag(markerB, 'B');
}
markerB.setLngLat(centerBLonLat).addTo(map);
localizeMapMarker(markerB);
upsertBufferB(map, { centerLonLat: centerBLonLat, radiusM });
}
} else removeMarkerB();
}

async function ensureTractOutline({ signal, isCurrent, onSourceResolved }) {
Expand Down Expand Up @@ -544,6 +597,10 @@ export async function initCrimeMode(map, {
} else if (!active) {
lastCameraSelectionKey = null;
pointsController.clear();
disposeMarkerADrag?.();
disposeMarkerBDrag?.();
disposeMarkerADrag = null;
disposeMarkerBDrag = null;
markerA?.remove();
markerB?.remove();
markerA = null;
Expand Down
5 changes: 5 additions & 0 deletions src/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -1493,10 +1493,15 @@ summary {
}

.analysis-marker {
cursor: grab;
filter: drop-shadow(0 0 0 rgba(200, 107, 0, 0.45)) drop-shadow(0 4px 8px rgba(16, 32, 51, 0.35));
animation: analysis-marker-arrive 420ms ease-out 1;
}

.analysis-marker:active {
cursor: grabbing;
}

@keyframes analysis-marker-arrive {
0% { opacity: 0.2; transform: scale(0.65); }
70% { opacity: 1; transform: scale(1.12); }
Expand Down