Skip to content

Commit 261beac

Browse files
authored
fix(view): collapse split-view grid only after swipe activation succeeds (#854)
* fix(view): collapse split-view grid only after swipe activation succeeds Follow-up to #844. The merged change ran setMapGrid(1, 1) before manager.toggle, so a failed swipe activation (a throw, or addMapControl returning false) discarded the user's split-view layout with no swipe to show for it. Activate first, then collapse the grid only when the swipe control is actually live. Addresses a CodeRabbit review note on #852 that landed after the PR was merged. * docs: note swipe-activation-order guard assumes synchronous activate Address review: tie the manager.isActive(id) correctness claim to the synchronous-only activation of maplibre-swipe, so a future dynamic-import path that makes activate async is flagged as needing this guard revisited.
1 parent 54f2da0 commit 261beac

1 file changed

Lines changed: 18 additions & 7 deletions

File tree

apps/geolibre-desktop/src/hooks/usePlugins.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,10 @@ export function usePluginRegistry() {
267267
const before = JSON.stringify(projectPluginStateSnapshot());
268268
// Layer Swipe and split view are mutually exclusive comparison modes:
269269
// stacking the swipe slider over a multi-pane grid fragments the
270-
// workspace (#844). Activating swipe collapses the grid back to a single
271-
// map first; the reverse direction (entering split view turns swipe off)
272-
// is handled by useSwipeSplitViewExclusivity.
273-
if (id === SWIPE_PLUGIN_ID && !manager.isActive(id)) {
274-
const { mapLayout, setMapGrid } = useAppStore.getState();
275-
if (mapLayout.rows * mapLayout.cols > 1) setMapGrid(1, 1);
276-
}
270+
// workspace (#844). The reverse direction (entering split view turns
271+
// swipe off) is handled by useSwipeSplitViewExclusivity.
272+
const collapseGridForSwipe =
273+
id === SWIPE_PLUGIN_ID && !manager.isActive(id);
277274
// Plugin controls are imperative MapLibre code, so a throw here escapes
278275
// React's error boundaries. Contain it so one bad plugin can't break the
279276
// toggle handler — surface it in diagnostics instead. Return without
@@ -288,6 +285,20 @@ export function usePluginRegistry() {
288285
reportPluginError(id, "toggle", error);
289286
return;
290287
}
288+
// Collapse the grid only once swipe actually activated, so a failed
289+
// activation (a throw above, or addMapControl returning false) leaves the
290+
// user's split-view layout intact. Done synchronously before React flushes
291+
// effects so useSwipeSplitViewExclusivity sees the single-pane grid and
292+
// doesn't undo the activation it just allowed.
293+
// Relies on maplibre-swipe activating synchronously (activate returns
294+
// false/undefined, never a Promise). PluginManager.activate marks a plugin
295+
// active optimistically and only rolls back async failures via
296+
// watchAsyncActivation, so isActive() would read true here before an async
297+
// mount confirms — revisit this guard if swipe ever gains a dynamic import.
298+
if (collapseGridForSwipe && manager.isActive(id)) {
299+
const { mapLayout, setMapGrid } = useAppStore.getState();
300+
if (mapLayout.rows * mapLayout.cols > 1) setMapGrid(1, 1);
301+
}
291302
persistProjectPluginState(before);
292303
},
293304
setMapControlPosition: (

0 commit comments

Comments
 (0)