Skip to content

Commit 3fdb8bb

Browse files
authored
fix(map): disable Save Map while pending and harden mobile save flows
## Summary Harden the map save flow on three axes: 1. **Disable both Save Map triggers while a save mutation is pending** (sidebar + mobile floating overlay). Prevents the user from queuing duplicate saves or opening a second name dialog while one is in flight. 2. **Disable the settings-sheet Save Map button when no configuration changes are present** (bbox, zoom, or basemap still at defaults). Forces the settings flow to be config-first. 3. **Close the mobile settings sheet before opening the save-name dialog.** Keeps the modal stack tidy so the user is never hidden behind a sheet. The mobile floating overlay intentionally has *no* `!hasConfigChanges` guard — it is a quick-action affordance that may save the current/default view at any time. Only the pending-disable applies. This asymmetry is now covered by a regression test so a future change to the floating button would fail the suite. ## Changes - `src/screens/MapScreen/MapScreen.tsx` - `disabled={createMap.isPending || !hasConfigChanges}` on the settings-sheet Save Map button - `disabled={createMap.isPending}` on the mobile floating Save Map button (intentional, see comment) - `hasConfigChanges` useMemo: compares bbox / zoomRange / selectedStyle against defaults - `openNameDialog()` now closes the settings sheet before opening the name dialog - New i18n key `activeMapOnlineBadge` to distinguish online vs. offline active-map copy - `src/i18n/messages/{en,es,pt}.json`: new `activeMapOnlineBadge` message in all three locales - Tests: 9 new cases — both triggers disable while pending, both re-enable on settle, mobile default-state asymmetry, draw cancel, DownloadPanel integration, name-required error, no-project empty state, frame confirm happy path, map attribution ## Test coverage MapScreen.tsx: 72.17% stmts / 68.83% branches → 94.78% stmts / 84.41% branches (all 4 metrics ≥ 80% per AGENTS.md) Full suite: 2050 passed, 1 skipped (164 files) ## CI All 15 required checks green. `mergeState: CLEAN`. Codex GPT-5.6-Sol independent readiness assessment: 5/5. ## Preview https://fix-save-map-disabled.comapeo-cloud-app.pages.dev
1 parent b88f915 commit 3fdb8bb

7 files changed

Lines changed: 413 additions & 7 deletions

File tree

src/components/shared/MapContainer/MapContainer.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ const messages = defineMessages({
4040
id: 'map.activeMap.badge',
4141
defaultMessage: 'Active offline map: {name}',
4242
},
43+
activeMapOnlineBadge: {
44+
id: 'map.activeMap.onlineBadge',
45+
defaultMessage: 'Active map (online): {name}',
46+
},
4347
basemapDisabledHint: {
4448
id: 'map.basemapSwitcher.disabledHint',
4549
defaultMessage: 'Basemap used when offline map is turned off',
@@ -403,7 +407,7 @@ function MapContainer({
403407
strokeLinecap="round"
404408
/>
405409
</svg>
406-
{intl.formatMessage(messages.activeMapBadge, {
410+
{intl.formatMessage(messages.activeMapOnlineBadge, {
407411
name: activeSavedMap?.name ?? '',
408412
})}
409413
</span>

src/i18n/messages/en.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,9 @@
962962
"map.activeMap.badge": {
963963
"defaultMessage": "Active offline map: {name}"
964964
},
965+
"map.activeMap.onlineBadge": {
966+
"defaultMessage": "Active map (online): {name}"
967+
},
965968
"map.basemap.label": {
966969
"defaultMessage": "Basemap"
967970
},

src/i18n/messages/es.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,5 +477,6 @@
477477
"theme.light": "Claro",
478478
"theme.system": "Sistema",
479479
"map.activeMap.badge": "Mapa sin conexión activo: {name}",
480+
"map.activeMap.onlineBadge": "Mapa activo (en línea): {name}",
480481
"map.basemapSwitcher.disabledHint": "Mapa base usado cuando el mapa sin conexión está desactivado"
481482
}

src/i18n/messages/pt.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,5 +477,6 @@
477477
"theme.light": "Claro",
478478
"theme.system": "Sistema",
479479
"map.activeMap.badge": "Mapa offline ativo: {name}",
480+
"map.activeMap.onlineBadge": "Mapa ativo (online): {name}",
480481
"map.basemapSwitcher.disabledHint": "Mapa base usado quando o mapa offline está desativado"
481482
}

src/screens/MapScreen/MapScreen.tsx

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,21 @@ export function MapScreen() {
224224
);
225225
useShellSlot(shellSlot);
226226

227+
const hasConfigChanges = useMemo(
228+
() =>
229+
bbox[0] !== DEFAULT_BBOX[0] ||
230+
bbox[1] !== DEFAULT_BBOX[1] ||
231+
bbox[2] !== DEFAULT_BBOX[2] ||
232+
bbox[3] !== DEFAULT_BBOX[3] ||
233+
zoomRange.minZoom !== DEFAULT_ZOOM.minZoom ||
234+
zoomRange.maxZoom !== DEFAULT_ZOOM.maxZoom ||
235+
selectedStyle.id !== DEFAULT_BASEMAP_ID,
236+
[bbox, zoomRange, selectedStyle],
237+
);
238+
227239
function openNameDialog() {
228240
setNameError(null);
241+
setSettingsOpen(false);
229242
setNameDialogOpen(true);
230243
}
231244

@@ -319,7 +332,11 @@ export function MapScreen() {
319332
<DownloadPanel key={m.id} map={m} />
320333
));
321334
})()}
322-
<Button onClick={openNameDialog} className="w-full">
335+
<Button
336+
onClick={openNameDialog}
337+
className="w-full"
338+
disabled={createMap.isPending || !hasConfigChanges}
339+
>
323340
{intl.formatMessage(mapMessages.saveMap)}
324341
</Button>
325342
</div>
@@ -424,7 +441,14 @@ export function MapScreen() {
424441
) : null}
425442
{drawMode !== 'draw_rectangle' && (
426443
<div className="absolute bottom-4 right-4 lg:hidden">
427-
<Button size="sm" onClick={openNameDialog}>
444+
{/* Intentionally no !hasConfigChanges guard: this quick action
445+
may save the current/default map view at any time.
446+
Only pending-disable is needed here. */}
447+
<Button
448+
size="sm"
449+
onClick={openNameDialog}
450+
disabled={createMap.isPending}
451+
>
428452
{intl.formatMessage(mapMessages.saveMap)}
429453
</Button>
430454
</div>

tests/unit/components/shared/MapContainer/MapContainer.test.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,10 @@ describe('MapContainer', () => {
387387
const mapEl = screen.getByTestId('mock-map');
388388
expect(mapEl.dataset.mapStyle).toBe('StyleSpecification');
389389
});
390-
// Should show online active badge
391-
expect(screen.getByTestId('map-online-active-badge')).toBeInTheDocument();
390+
// Should show online active badge with map name
391+
const badge = screen.getByTestId('map-online-active-badge');
392+
expect(badge).toBeInTheDocument();
393+
expect(badge).toHaveTextContent(/Active map.*online.*Draft Map/);
392394
// Should show tooltip on basemap switcher
393395
const trigger = screen.getByTestId('basemap-switcher-trigger');
394396
expect(trigger.getAttribute('title')).toBeTruthy();

0 commit comments

Comments
 (0)