Panel and plugin commands on the request/provide bus - #322
Merged
Conversation
sandesh-sp
force-pushed
the
feat/panel-collapse-bus-api
branch
from
August 19, 2026 14:12
fcb518c to
afc1f80
Compare
sandesh-sp
changed the title
sandesh-sp
changed the base branch from
development
to
fix/bus-emit-listener-isolation
August 19, 2026 14:12
sandesh-sp
force-pushed
the
feat/panel-collapse-bus-api
branch
2 times, most recently
from
August 20, 2026 19:56
b7040c5 to
0be3440
Compare
Base automatically changed from
fix/bus-emit-listener-isolation
to
development
August 25, 2026 19:20
sandesh-sp
added a commit
that referenced
this pull request
Aug 25, 2026
## Why `mmgisAPI.emit` was mitt's `emit` directly. mitt iterates its handler array and calls each one unguarded, so the first listener that throws takes the whole emit down with it — every listener registered after it is silently skipped, and the exception surfaces at the emitter, which has nothing to do with the fault. That is a poor property for a bus whose entire purpose is decoupling. A plugin subscribing to `layer:toggle` can currently suppress core's own handling of it. ## What `emit` walks the handler list itself and runs each subscriber in a try/catch. A handler that throws is reported with `console.error` naming the event, and the emit continues. Wildcard listeners get the same treatment. The list is copied before iterating, so a handler that subscribes or unsubscribes during the emit cannot corrupt the walk it is part of. This reaches into `events.all`, mitt's internal Map of event name to handler array. That is implementation, not documented API, and the comment says so — a mitt upgrade is a place to re-check this. ## Scope One method. No caller changes, no signature change. ## Testing `tests/unit/mmgisAPIBus.spec.js` covers a throwing listener not blocking the ones after it, the error being reported, and subscribe/unsubscribe during an emit. **1413 tests passing, `tsc --noEmit` clean.** ## Related First of a three-part stack. This one is independent of the rest and can merge on its own — it is a live bug fix. The panel and plugin command work builds on it in #322.
PanelManager_ signalled refusal by throwing, which a caller across a bus
boundary cannot catch, and offered only a toggle — which acts on whatever it
finds rather than what the caller asked for, so a control working from a stale
read flips the wrong way.
State changes now return { ok, state, changed } or { ok, reason }, with
canSetState exposing the same constraints a control needs to decide whether to
draw itself. showPanel replaces the toggle and names the state it resolves to.
Plugin lifecycle gets the matching treatment on ToolControllerModern_: the
loaded and hidden flags become one of unloaded, hidden or visible, reachable
through setPluginState.
Both broadcast their listing on panels:changed and plugins:changed, replacing
mmgis-panel-layout-changed. Payloads are frozen projections holding no core
references, so they survive a structured clone.
Panels and plugins were reachable only by importing their managers directly, which a sandboxed plugin cannot do. Eight handlers put them on the bus plugins already use for everything else: getAll, setState, and show/hide sugar for each. Handlers register at module load and stay registered, so an absent layout is reported as a layout-inactive result rather than a missing handler. Callers see one failure vocabulary whichever layer refused. The managers are read through getters rather than captured at registration, since the UI clears them on teardown. Matching typed wrappers land in the shared plugin client, which is the only place the bus name strings appear.
Only setPluginState broadcast, so the paths the layout drives itself stayed silent: the startup queue loads and deferred registrations. A plugin that seeds from plugins:getAll during its own make() captures the listing partway through that queue — request runs its handler synchronously — and then follows events, which the docs say is enough. It never hears about the plugins that loaded after it. The queue runs through the controller and broadcasts once when the batch is done. One event per settling rather than one per plugin: a batch is a single layout change, and per-plugin events would expose states no command asked for, since a plugin bound for hidden passes through visible on the way. Teardown stays quiet. destroyAllTools runs when the subscribers are themselves being destroyed, so an event there reaches nobody who can act on it.
An unrecognised state name reached panels as state-not-allowed — the reason reserved for a real state a panel's own constraints forbid — so a typo in a payload read as a layout misconfiguration and sent a caller auditing panel config. Plugins already called it bad-request. A shared guard judges the state against the subsystem's vocabulary before the id is resolved, so both commands agree on the reason and on which half of a doubly-wrong payload to report. The refusal vocabulary is written out once, with each reason defined, and the panel result type draws on it rather than naming three of the seven. The plugin-side client keeps its own copy, since a plugin reaches core only over the bus, and a new spec holds the two to the same set. That spec also covers a gap the client-side tests cannot: they stub window.mmgisAPI, so a request name that drifts from the provider answering it still passes, then degrades at runtime into a plausible layout-inactive refusal with nothing logged. It checks all eight names, and both event names, against the real bus. Also covered: restoring a hidden plugin, the transition that gives 'hidden' a reason to exist and the one path through the lifecycle nothing exercised; that unloading runs the tool's own destroy rather than only updating the registries, which two close buttons depend on; that show walks past a collapsed default; and that the panel projection carries the tool ids it claims to. Bus names and the two event names move into exported constants, so the client has one spelling of each and the reconciliation spec has something to check.
sandesh-sp
force-pushed
the
feat/panel-collapse-bus-api
branch
from
August 25, 2026 19:20
0be3440 to
5df5861
Compare
CarsonDavis
approved these changes
Aug 26, 2026
sandesh-sp
added a commit
that referenced
this pull request
Aug 27, 2026
## Why With the request surface in place (#322), there were three ways to collapse a panel: a direct `PanelManager_` call, an `mmgisAPI.hidePanel` method, and a `core:hidePanel` event. Two of them are strictly worse than the third. `core:*` events are fire-and-forget, so a refusal has nowhere to go — a config naming a panel that does not exist produced nothing at all. The direct `mmgisAPI` methods cannot cross a sandbox boundary, since `postMessage` cannot carry a function reference. Both are removed here. ## What **Removed:** `_initCoreCommandDispatcher` and the seven `core:*` command events; `showPanel`, `hidePanel`, `togglePanel`, `showPlugin`, `hidePlugin`, `loadPlugin`, `unloadPlugin`, `isPluginLoaded`, `isPluginHidden` from `mmgisAPI`; `togglePanelCollapsed` from `PanelManager_`. **Panel controls** issue requests and log the refusal reason rather than clicking dead, and decide whether to draw themselves from `canSetState` instead of reading `allowedStates` directly — which missed the float-position restriction, so a float panel offered a Minimize button that stranded it. **Config action strings** resolve through one shared resolver, so a mission author reads the same request names the API documents rather than a second tool-specific vocabulary. `core:` is reserved rather than reused: its verbs have no expressible successor here (`togglePanel` is gone; `unloadPlugin` needs a state argument a colon-delimited string cannot carry), so a config still carrying `core:showPlugin:LayersTool` gets a warning naming the supported actions instead of emitting an event nothing listens for. **Consumers migrated:** Chart (`plugins:show` on analysis-ready, `plugins:setState` on close), AOI (unload on close), Title (through the resolver), AddTempLayer. **Teardown announces itself.** `panels:changed` and `plugins:changed` were silent when a layout came down, on the reasoning that teardown destroys the subscribers with it. The bus outlives the layout, so a plugin that subscribed is still subscribed and still holding a roster of panels and plugins that no longer exist. Both events now fire once more with an empty listing, which agrees with what the providers report from that point on. `PanelManager_` gains `clear()` so that goes out once rather than once per panel. ## Regression fixed AddTempLayer was calling `window.mmgisAPI.showPlugin` / `hidePlugin` through a locally declared optional type. Those methods are removed here, and optional chaining would have turned both calls into silent no-ops. The tool starts hidden and ships no trigger of its own, so its only door would have stopped opening and its close button would have stopped closing — with no error anywhere. It goes through the shared client's plugin wrappers instead, which report a refusal. Two other defects turned up while tracing impact — `panels:show` shrinking an already-visible panel, and a failed plugin transition reporting `not-found`. Both are in the state owners #322 reworks, so they are fixed there and written up in that PR's body; nothing about them is in this diff. ## Testing Specs for the panel controls, the config resolver, and each migrated consumer, including a negative check run on every new test — the behaviour was broken to confirm the test goes red, then restored. **1485 tests passing, `tsc --noEmit` clean.** ## Related Third of a three-part stack: #324 → #322 → **this**. Merge in order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Panels and plugins can only be driven from inside core.
PanelManager_andToolControllerModern_are reachable by direct import, which a sandboxed plugin cannot do, so a plugin that wants to collapse the panel it lives in — or reveal a sibling plugin — has no way to read the state or act on it.The state owners were also awkward to drive even from inside core. They signalled refusal by throwing, which does not survive a bus boundary, and panels offered only
togglePanelCollapsed: a toggle acts on whatever it finds rather than on what the caller asked for, so a control working from a listing it read a moment ago flips the wrong way, and a retry undoes itself.What
Results instead of exceptions.
setPanelStatereturns{ ok: true, state, changed }or{ ok: false, reason }.canSetStateexposes the same constraints, so a control can decide whether to draw itself at all rather than discovering the answer by being clicked.showPanelreplaces the toggle and names the state it resolves to.Plugin lifecycle as states.
ToolControllerModern_'s separate loaded and hidden flags become one ofunloaded,hidden,visible, reachable throughsetPluginState. Asking forvisibleon an unloaded plugin loads it.Eight handlers on the bus plugins already use for everything else:
panels:getAll/plugins:getAllpanels:setState/plugins:setStatepanels:show/panels:hideplugins:show/plugins:hideHandlers register at module load and stay registered. An absent layout is reported as a
layout-inactiveresult, not a missing handler, so a caller sees one failure vocabulary whichever layer refused. The managers are read through getters rather than captured at registration, since the UI clears them on teardown.Two events.
panels:changedandplugins:changedcarry the same listing thegetAllhandlers return, so a subscriber never has to re-request.panels:changedreplacesmmgis-panel-layout-changed. Payloads are frozen projections holding no core references, so they survive a structured clone — which is what a future sandbox bridge will have to do to them.Matching typed wrappers land in
src/essence/Tools/_shared/adapters/mmgisAPI.ts, the one place the bus name strings appear.Scope
Mostly additive, with two breaking changes worth knowing about before merge:
PanelManager_.togglePanelCollapsedis removed, from both the class and the interface. A toggle acts on whatever it finds, which is the problem this PR exists to fix;showPanelandsetPanelStatename the state they resolve to instead.mmgis-panel-layout-changedis renamed topanels:changed.Everything else is additive. The existing
mmgisAPI.showPanel/hidePanel/togglePanelmethods and thecore:*command events all still work — they are reimplemented on the new primitives here and removed in #325.Defects fixed along the way
Two, both in the state owners this PR reworks:
panels:showon an already-visible panel resolved a target state and so could shrink it. Every stock layout gives the left panel aniconifieddefault, so showing an expanded panel iconified it. Show now only ever lifts a panel out ofcollapsed.not-found, pointing a reader at the plugin id — the one thing already known to be good, since the id was resolved before the transition was attempted. It reportstransition-failed.Testing
Provider specs for all eight handlers (refusal vocabulary, malformed payloads, absent layout, the float-position restriction),
PanelManager_command specs, plugin state-transition specs, and the typed wrappers.1458 tests passing,
tsc --noEmitclean. Every commit in the stack was verified green in isolation, not just the tip.Related
Second of a three-part stack: #324 → this → #325.
Note for #323: this replaces the API that branch was written against, and the break lands when this PR merges — not #325.
panels:toggleCollapsedis not registered (toggle was dropped deliberately, see above),collapsibleis not on the listing, and the event is nowpanels:changed. #323 needs a follow-up to move ontopanels:show/panels:hidedriven off thestateit already reads.