Skip to content

Commit 4b1cf4f

Browse files
authored
fix(time-slider): close the dock when the last temporal binding is removed (#1515)
A plugin (or the Layers panel) opening the Time Slider on the first temporal layer had no symmetric way to close it again. The dock stayed on screen, empty, once the only bound layer was removed or replaced by a non-temporal one, implying a timeline the active data does not have. Only the Layers panel's explicit "Unbind" action closed it. Close a binding-opened dock once the slider has nothing left to drive, watched from a store subscription so every route into that state is covered — removing the bound layer, a project switch, or a plugin swapping datasets. A dock the user opened from the Plugins menu is left alone, and `isTimeSliderIdle` keeps it open while the dock's own raster sources or a KML <TimeSpan> overlay remain, since the dock is the only way to reach those. Also add `deactivatePlugin(pluginId)` to `GeoLibreAppAPI`, the counterpart of `activatePlugin`, so a plugin can close a panel it opened. Like `activatePlugin` it refuses to target the caller, which would unmount the code still running. Fixes #1512
1 parent 9ea7db0 commit 4b1cf4f

9 files changed

Lines changed: 349 additions & 10 deletions

File tree

apps/geolibre-desktop/src/components/layout/DesktopShell.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ import {
9090
usePluginRegistry,
9191
useProjectPluginTrust,
9292
useSwipeSplitViewExclusivity,
93+
useTimeSliderAutoClose,
9394
} from "../../hooks/usePlugins";
9495
import { registerMbtilesProtocol } from "../../lib/mbtiles";
9596
import { hasReverseGeocodeConsent } from "../../lib/reverse-geocode-consent";
@@ -735,6 +736,9 @@ export function DesktopShell({
735736
// Keep Layer Swipe and split view mutually exclusive (#844): entering a
736737
// multi-pane grid turns the swipe slider off.
737738
useSwipeSplitViewExclusivity(mapControllerRef);
739+
// Close a binding-opened Time Slider once the last temporal layer is gone
740+
// (#1512), so the dock does not linger over a map with no timeline.
741+
useTimeSliderAutoClose(mapControllerRef);
738742
// Live-collaboration session. Owned here (rather than in TopToolbar) so both
739743
// the Collaborate dialog and the on-canvas status badge share one socket, and
740744
// so the dialog stays mounted in toolbar-hidden layouts.

apps/geolibre-desktop/src/components/panels/LayerPanel.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,12 @@ import {
6565
placeholderMessage,
6666
} from "@geolibre/map";
6767
import { getIsMobileViewport } from "../../hooks/useIsMobileViewport";
68-
import { bindTemporalLayer, createAppAPI, usePluginRegistry } from "../../hooks/usePlugins";
68+
import {
69+
activateTimeSliderForBinding,
70+
bindTemporalLayer,
71+
createAppAPI,
72+
usePluginRegistry,
73+
} from "../../hooks/usePlugins";
6974
import { useDesktopSettingsStore } from "../../hooks/useDesktopSettings";
7075
import {
7176
clearFeatureSelection,
@@ -1703,9 +1708,7 @@ export function LayerPanel({
17031708
},
17041709
timeFilter: undefined,
17051710
});
1706-
if (!isPluginActive(TIME_SLIDER_PLUGIN_ID)) {
1707-
togglePlugin(TIME_SLIDER_PLUGIN_ID, createAppAPI(mapControllerRef));
1708-
}
1711+
activateTimeSliderForBinding(mapControllerRef);
17091712
closeBindTimeSliderDialog();
17101713
}, [
17111714
bindTimeSliderLayer,
@@ -1717,8 +1720,6 @@ export function LayerPanel({
17171720
bindWindowMode,
17181721
mapControllerRef,
17191722
updateLayer,
1720-
isPluginActive,
1721-
togglePlugin,
17221723
closeBindTimeSliderDialog,
17231724
t,
17241725
]);

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

Lines changed: 106 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
buildSelectorTimeBinding,
1010
registerTemporalLayer,
1111
unregisterTemporalLayer,
12+
isTimeSliderIdle,
1213
TIME_SLIDER_PLUGIN_ID,
1314
type TemporalLayerAdapter,
1415
setZarrLayerSelector,
@@ -106,6 +107,7 @@ import { pickZarrDirectory, zarrDirectoryPickerSupported } from "../lib/zarr-dir
106107
import { openExternalLink } from "../lib/open-external";
107108
import { fetchUrlBytes } from "../lib/native-http";
108109
import { partitionProjectPluginManifestUrls } from "../lib/plugin-trust";
110+
import { setTimeSliderOpenedByBinding, shouldCloseTimeSliderDock } from "../lib/time-slider-dock";
109111
import { createWmsTileUrl, normalizeWmsVersion } from "../components/layout/add-data/helpers";
110112
import { createExternalNativeStoreLayer } from "../lib/external-native-layer";
111113
import { mergeStringLists } from "../lib/string-lists";
@@ -239,6 +241,17 @@ if (zarrDirectoryPickerSupported()) {
239241
setZarrLocalStoreProvider(pickZarrDirectory);
240242
}
241243

244+
// Forget that a binding opened the Time Slider dock as soon as the plugin goes
245+
// inactive by any route (#1512), so a later manual activation is not mistaken
246+
// for a binding-opened one and closed out from under the user.
247+
let timeSliderWasActive = false;
248+
manager.subscribe(() => {
249+
const active = manager.isActive(TIME_SLIDER_PLUGIN_ID);
250+
if (active === timeSliderWasActive) return;
251+
timeSliderWasActive = active;
252+
if (!active) setTimeSliderOpenedByBinding(false);
253+
});
254+
242255
let externalPluginsLoaded = false;
243256
let externalPluginsLoadPromise: Promise<void> | null = null;
244257
let externalPluginsLoadKey: string | null = null;
@@ -704,12 +717,85 @@ export function bindTemporalLayer(
704717
// dialog does when it commits).
705718
timeFilter: undefined,
706719
});
707-
if (!manager.isActive(TIME_SLIDER_PLUGIN_ID)) {
708-
manager.toggle(TIME_SLIDER_PLUGIN_ID, createAppAPI(mapControllerRef));
709-
}
720+
activateTimeSliderForBinding(mapControllerRef);
710721
return true;
711722
}
712723

724+
/**
725+
* Open the Time Slider dock because a layer was just bound to it, and remember
726+
* that the binding is what opened it so {@link useTimeSliderAutoClose} may close
727+
* it again when the last binding goes away (#1512).
728+
*
729+
* A no-op when the dock is already showing — including when the user opened it
730+
* themselves, which deliberately leaves the "opened by a binding" flag false so
731+
* their dock is never taken away underneath them.
732+
*
733+
* Call this **after** the binding has been written to the layer's metadata, so
734+
* the dock adopts it on activation.
735+
*
736+
* @param mapControllerRef - Used to build the app API for activation.
737+
*/
738+
export function activateTimeSliderForBinding(
739+
mapControllerRef?: RefObject<MapController | null>,
740+
): void {
741+
if (manager.isActive(TIME_SLIDER_PLUGIN_ID)) return;
742+
const before = JSON.stringify(projectPluginStateSnapshot());
743+
try {
744+
manager.activate(TIME_SLIDER_PLUGIN_ID, createAppAPI(mapControllerRef));
745+
} catch (error) {
746+
// Plugin controls are imperative MapLibre code, so a throw here would escape
747+
// React's error boundaries. Contain it, exactly as usePluginRegistry.toggle
748+
// does, and leave the project state unwritten.
749+
reportPluginError(TIME_SLIDER_PLUGIN_ID, "toggle", error);
750+
return;
751+
}
752+
setTimeSliderOpenedByBinding(manager.isActive(TIME_SLIDER_PLUGIN_ID));
753+
persistProjectPluginState(before);
754+
}
755+
756+
/**
757+
* Close a binding-opened Time Slider once it has nothing left to drive (#1512).
758+
*
759+
* `activatePlugin` / `registerTemporalLayer({ bind: true })` open the dock when
760+
* the first temporal layer appears, but nothing closed it again when the last
761+
* one went away by a route other than the Layers panel's explicit "Unbind"
762+
* action — removing the bound layer, or a plugin swapping a temporal dataset for
763+
* a single-period one. The dock then lingered over the map, implying a timeline
764+
* no layer has.
765+
*
766+
* Only a dock opened *by* a binding is closed; one the user opened from the
767+
* Plugins menu stays put. `isTimeSliderIdle` additionally keeps it open while
768+
* the dock's own raster sources or a KML `<TimeSpan>` overlay remain, since the
769+
* dock is the only way to reach those.
770+
*
771+
* Mounted once near the app root so it covers every way a binding can
772+
* disappear, not just the Layers panel.
773+
*/
774+
export function useTimeSliderAutoClose(mapControllerRef: RefObject<MapController | null>): void {
775+
useEffect(() => {
776+
// Subscribed rather than selected from the store so no component re-renders
777+
// on every layer edit just to run this check.
778+
const check = () => {
779+
if (!shouldCloseTimeSliderDock(manager.isActive(TIME_SLIDER_PLUGIN_ID), isTimeSliderIdle)) {
780+
return;
781+
}
782+
const before = JSON.stringify(projectPluginStateSnapshot());
783+
try {
784+
// Deactivating prunes the dock's own store layers, which re-enters this
785+
// subscription; those passes find the dock already idle-and-closing and
786+
// the plugin's own deactivate is a no-op once its control is gone.
787+
manager.deactivate(TIME_SLIDER_PLUGIN_ID, createAppAPI(mapControllerRef));
788+
} catch (error) {
789+
reportPluginError(TIME_SLIDER_PLUGIN_ID, "toggle", error);
790+
return;
791+
}
792+
persistProjectPluginState(before);
793+
};
794+
check();
795+
return useAppStore.subscribe(check);
796+
}, [mapControllerRef]);
797+
}
798+
713799
export function createAppAPI(mapControllerRef?: RefObject<MapController | null>) {
714800
const store = useAppStore.getState();
715801
// Captured so methods that delegate to plugin helpers taking the AppAPI
@@ -874,6 +960,23 @@ export function createAppAPI(mapControllerRef?: RefObject<MapController | null>)
874960
if (!activated || !manager.isActive(pluginId)) return false;
875961
return state === undefined ? true : manager.applyPluginState(pluginId, api, state);
876962
},
963+
// The counterpart of activatePlugin, so a plugin that opened another
964+
// plugin's panel can close it again (#1512). Persisted like the toolbar's
965+
// own toggle, so the project records the panel as off; a throw from the
966+
// target's imperative teardown is contained rather than escaping into the
967+
// caller.
968+
deactivatePlugin: (pluginId: string) => {
969+
if (!manager.isActive(pluginId)) return false;
970+
const before = JSON.stringify(projectPluginStateSnapshot());
971+
try {
972+
manager.deactivate(pluginId, api);
973+
} catch (error) {
974+
reportPluginError(pluginId, "deactivate", error);
975+
return false;
976+
}
977+
persistProjectPluginState(before);
978+
return !manager.isActive(pluginId);
979+
},
877980
queryOvertureFeatures,
878981
addLayerGroup: (name?: string, layerIds?: string[]) =>
879982
useAppStore.getState().addLayerGroup(name, layerIds),
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* Whether the Time Slider dock is open *because* a layer was bound to it, as
3+
* opposed to the user opening it from the Plugins menu.
4+
*
5+
* The distinction is the whole policy behind {@link shouldCloseTimeSliderDock}:
6+
* a dock that a binding opened is closed again when the last binding goes away,
7+
* while a dock the user opened stays put even with an empty timeline — they may
8+
* be about to add a raster time series through its own "Add data" form.
9+
*/
10+
let openedByBinding = false;
11+
12+
/**
13+
* Record how the dock came to be open. Set true right after a binding activates
14+
* it, and false whenever the plugin goes inactive by any route so a later manual
15+
* activation starts from a clean slate.
16+
*
17+
* @param opened - True when a binding is what opened the dock.
18+
*/
19+
export function setTimeSliderOpenedByBinding(opened: boolean): void {
20+
openedByBinding = opened;
21+
}
22+
23+
/** Whether a binding, rather than the user, opened the dock. */
24+
export function isTimeSliderOpenedByBinding(): boolean {
25+
return openedByBinding;
26+
}
27+
28+
/**
29+
* Whether a binding-opened Time Slider dock now has nothing left to drive and
30+
* should close itself (#1512).
31+
*
32+
* `isIdle` is what keeps this honest about the dock's other duties: the dock
33+
* stays open while any binding, dock raster source, or KML `<TimeSpan>` overlay
34+
* remains, because it is the only way to reach the last two. It is taken as a
35+
* thunk so the layer scan is skipped on the overwhelmingly common path (a dock
36+
* the user opened, or none at all), and so this module stays free of the plugins
37+
* barrel.
38+
*
39+
* @param active - Whether the Time Slider plugin is currently active.
40+
* @param isIdle - Reads whether the slider has anything left to drive, normally
41+
* `isTimeSliderIdle` from `@geolibre/plugins`.
42+
* @returns True when the dock should be deactivated.
43+
*/
44+
export function shouldCloseTimeSliderDock(active: boolean, isIdle: () => boolean): boolean {
45+
return openedByBinding && active && isIdle();
46+
}
47+
48+
/** Reset the module state between tests. */
49+
export function __resetTimeSliderDockForTests(): void {
50+
openedByBinding = false;
51+
}

docs/plugin-api.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,19 @@ Call the returned function, or `app.unregisterTemporalLayer?.(layerId)`, to drop
568568

569569
A bound cube shares the timeline with any vector bindings and dated overlays: the track spans the union of their extents, and the widest dataset sets the stepping granularity.
570570

571+
**Closing the dock again.** A dock that a binding opened closes itself once the last temporal layer is gone, so it never lingers over a map with no timeline. Removing the bound layer is enough; if your plugin keeps the layer and only drops its binding, clear `layer.metadata.timeBinding` as well as unregistering the adapter. A dock the *user* opened from the Plugins menu is left alone, and so is one that still has raster sources of its own or a KML `<TimeSpan>` overlay to drive.
572+
573+
## Activating and deactivating other plugins
574+
575+
```typescript
576+
await app.activatePlugin?.("maplibre-gl-time-slider");
577+
app.deactivatePlugin?.("maplibre-gl-time-slider");
578+
```
579+
580+
`activatePlugin` takes an optional second argument, a project-state patch applied once the target is active; it resolves false when the plugin is unavailable, refuses to activate, or rejects the state. `deactivatePlugin` is its counterpart and returns true when the plugin ended up inactive.
581+
582+
Neither may target the **calling** plugin: `activatePlugin` on yourself is meaningless, and deactivating yourself would unmount the code still on the stack. Both return false in that case.
583+
571584
## Custom (WebGL) layers and paint ownership
572585

573586
`registerExternalNativeLayer` mirrors a layer the plugin added to the map itself into GeoLibre's layer store, so it appears in the Layers panel and persists with the project:

packages/plugins/src/plugin-manager.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,8 @@ function scopeAppToPlugin(
529529
const { onControlAdded } = options;
530530
const register = app.registerToolbarMenu;
531531
const activatePlugin = app.activatePlugin;
532-
if (!register && !onControlAdded && !activatePlugin) return app;
532+
const deactivatePlugin = app.deactivatePlugin;
533+
if (!register && !onControlAdded && !activatePlugin && !deactivatePlugin) return app;
533534

534535
const scoped: GeoLibreAppAPI = { ...app };
535536

@@ -558,6 +559,14 @@ function scopeAppToPlugin(
558559
targetPluginId === pluginId ? false : activatePlugin(targetPluginId, state);
559560
}
560561

562+
if (deactivatePlugin) {
563+
// Same self-guard as activatePlugin, for a stronger reason: deactivating the
564+
// caller would run its own `deactivate` from inside whichever callback is
565+
// executing, unmounting the code still on the stack.
566+
scoped.deactivatePlugin = (targetPluginId) =>
567+
targetPluginId === pluginId ? false : deactivatePlugin(targetPluginId);
568+
}
569+
561570
return scoped;
562571
}
563572

packages/plugins/src/types.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,21 @@ export interface GeoLibreAppAPI {
424424
* refuses activation, or rejects the state.
425425
*/
426426
activatePlugin?: (pluginId: string, state?: unknown) => Promise<boolean>;
427+
/**
428+
* Deactivate an installed plugin, the counterpart of {@link activatePlugin}.
429+
* Lets a plugin that opened another plugin's panel close it again when the
430+
* reason for opening it is gone — for example a plugin that bound its layer to
431+
* the Time Slider and has since unbound the last one.
432+
*
433+
* A plugin may not deactivate **itself**: tearing a plugin down from inside
434+
* its own callback would unmount the code that is still running. Such a call
435+
* returns false, mirroring how `activatePlugin` refuses to reactivate the
436+
* caller.
437+
*
438+
* Returns true when the plugin ended up inactive, false when it is unknown,
439+
* was not active, is the caller, or threw while unmounting.
440+
*/
441+
deactivatePlugin?: (pluginId: string) => boolean;
427442
/**
428443
* Query a bounded set of features from GeoLibre's official Overture Maps
429444
* PMTiles integration. The host enforces tile and feature limits.

tests/plugin-manager.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,4 +1057,37 @@ describe("PluginManager plugin coordination", () => {
10571057
assert.equal(manager.isActive("first"), true);
10581058
assert.equal(manager.isActive("second"), true);
10591059
});
1060+
1061+
it("lets one plugin deactivate another but never itself", () => {
1062+
const manager = new PluginManager();
1063+
const coordinatingApp = {
1064+
...app,
1065+
deactivatePlugin: (id: string) => {
1066+
manager.deactivate(id, coordinatingApp);
1067+
return !manager.isActive(id);
1068+
},
1069+
} as GeoLibreAppAPI;
1070+
let closer: GeoLibreAppAPI | null = null;
1071+
manager.register(
1072+
testPlugin({
1073+
id: "closer",
1074+
activate: (scopedApp) => {
1075+
closer = scopedApp;
1076+
},
1077+
}),
1078+
);
1079+
manager.register(testPlugin({ id: "dock" }));
1080+
1081+
manager.activate("dock", coordinatingApp);
1082+
manager.activate("closer", coordinatingApp);
1083+
assert.ok(closer);
1084+
1085+
// Deactivating itself is refused, so the plugin that is still running is
1086+
// never unmounted from inside its own call.
1087+
assert.equal(closer!.deactivatePlugin?.("closer"), false);
1088+
assert.equal(manager.isActive("closer"), true);
1089+
1090+
assert.equal(closer!.deactivatePlugin?.("dock"), true);
1091+
assert.equal(manager.isActive("dock"), false);
1092+
});
10601093
});

0 commit comments

Comments
 (0)