Skip to content

Core popup service: a plugin asks for a map-anchored card over the bus and gets a promise back #298

Description

@CarsonDavis

Motivation

Plugins needing a small card anchored to a spot on the map — the Area of Interest plugin's analyze/cancel card is the first — each build their own, with their own sanitising and close handling. One core-owned card, requested over the bus, keeps that in one place and keeps plugins decoupled from the map engines, as the vision requires.

How it should work

A plugin sends one request with a location, a title, an optional HTML body, and up to two labelled actions. The core sanitises the HTML, builds the card — title, body, action row, theme colours — hands it to the map engine to place, puts focus on the first action button, and answers with a promise resolving on how the card closed: primary action, secondary action, dismissed by the user (the map library's own close control, or a click on the map), or closed by code (replaced by a newer card, hidden on request, map re-initialised).

One card at a time: a new request replaces the current one, which resolves as closed, never dismissed. Scripts, handlers and stylesheets are stripped from the body. Both shapes are documented for plugin authors in the bus API docs.

Done when

Verified in the unit suite against a fake engine, and in the demo mission once Area of Interest is migrated.

  • Pressing an action closes the card and resolves with that action.
  • The map library closing the card — its close control, or a click on the map — resolves as dismissed.
  • A second request or a hide closes the open card, which resolves as closed; an invalid request is rejected and leaves it alone.
  • Script, handlers, script-running URLs and stylesheets never reach the page.
  • Focus lands on the first action button once the card is on the map.
  • A hide works even when the map engine throws.
  • The bus API docs cover the request and its outcomes; the unit suite is green.

Out of scope

  • Placement, and everything the map library's own popup does about closing, keyboard and focus — the previous PR.
  • Ownership (refusing another plugin's hide) and closing a destroyed plugin's card — the teardown PR, off this stack.
  • The Area of Interest migration — the next PR.
  • Shadow roots and focus traps. Settled: neither.
Draft implementation plan — written as of a34edaf on 2026-09-17. Rough guide; re-verify against latest code.

Current behavior

There is no popup service on development. git grep -n "showPopup\|MapPopup" origin/development -- src returns nothing, and IMapEngine carries only the older addOverlay (src/essence/Basics/MapEngines/IMapEngine.ts:447), which takes a mount function — the thing this service replaces on the public contract. showPopup(latlng, element, onClose) / hidePopup() arrive on IMapEngine in the placement PR; this PR consumes them, and the onClose report is the only way this service learns of a dismissal.

Bus providers all register in one array in src/essence/Basics/Map_/Map_.js. _providerCleanups is declared at :66, and on each map init the previous cleanups are run (:279) and the array is rebuilt (:280:396) out of window.mmgisAPI.provide(...) return values — e.g. map:getBounds at :282, map:fitBounds at :289, map:addOverlay/map:removeOverlay at :323/:327.

mmgisAPI.request(name, data) (src/essence/mmgisAPI/mmgisAPI.js:977) takes exactly two positional arguments and calls handler(data). There is no third argument, positional or options object, and a provider handler therefore receives one argument and no caller stamp. The reference's (request, { caller }) handler signature belongs to the plugin-handles PR (#414) and cannot be used here.

forPlugin(pluginId) (:1021) returns { emit, provide, getVars, pluginId, prefix } — no request. ToolController_.js:489 already hands every tool a tool.api, but a plugin asking for a popup calls window.mmgisAPI.request('map:showPopup', …) directly. The docs example must be written that way, not as api.request(...) (which the reference doc hunk does).

plugins:destroyed and plugins:allDestroyed do not exist on development — another reason the teardown listeners stay out.

DOMPurify is already a dependency: "dompurify": "^3.4.3" in package.json, locked at 3.4.3, used today in src/essence/Basics/Markdown_/Markdown_.ts and src/essence/Basics/ToolController_/ToolMetadataUtils.js. (PR #416, off development, tightens the lock; not a dependency of this work.)

Theme tokens are CSS custom properties emitted by src/styles/_theme-export.scss as --theme-color-* (--theme-color-white, --theme-color-ink, --theme-color-base-lighter, --theme-color-primary, --theme-color-shadow), --theme-font-* and --theme-spacing-*. They are not --color-* — that is the legacy palette in src/css/mmgis.css:77 — and they are not in src/styles/<brand>/_theme-tokens.scss, which holds USWDS Sass settings (@use "uswds-core" with (…)) rather than the properties themselves. Card CSS uses the --theme-* names with literal fallbacks, since the classic UI loads no theme stylesheet.

The bus doc is docs/pages/APIs/JavaScript/Main/Event-Bus-API.md (791 lines); the Map Providers table is at :388:395.

Tests are vitest under tests/unit/; npm run test:unit.

Where the change lands & rough plan

New src/essence/Basics/MapPopup_/ with types.ts, MapPopup_.ts and MapPopup.css; a map:showPopup / map:hidePopup pair in the Map_.js provider array plus one _providerCleanups.push(() => MapPopup_.hide()) so re-initialising the map drops the card; the Event-Bus-API addition; tests/unit/MapPopup_.spec.ts against a fake engine.

The reference implementation is a snapshot, not a target: worktree /Users/cdavis/github/MMGIS-worktrees/298-popup, diff from the main checkout with git diff feature/350-tool-identity-v2...feature/298-map-popup-service-v2. Its MapPopup_.ts is 512 lines. With placement in the engine and closing left to the library, roughly 120 of those survive — about half of what an earlier read of this plan expected. Its comment density is not a target.

Carried across, each with its test:

  • Request validation: finite lat/lng; title/html strings when given; blank reads as absent; a card needs a title or html (buttons are not content); an invalid request rejects and leaves an open card alone.
  • POPUP_SANITIZE_CONFIG — DOMPurify defaults plus FORBID_TAGS: ['style'] and FORBID_ATTR: ['popover', 'popovertarget'], sanitised with RETURN_DOM_FRAGMENT. Unchanged from the reference.
  • buildPopupCard (title as text, content, actions row) and buildActionButton, including a lone secondary rendering with primary styling while still reporting its own slot. The card is plain content inside the library's popup: the frame, the close control and the closing behaviour are the library's, so no close button of our own and no dialog bookkeeping beyond what the markup gives for free.
  • Focus after placement goes to the first action button. With no actions, make no focus call and let the library do whatever it does.
  • The promise plumbing: settle/fail captured out of the executor, this._open recorded before wiring so a wiring failure can unwind, reject-before-unwind so the failure is the answer rather than the teardown's closed.
  • hide()'s shape: take the card off the engine inside try (a destroyed engine throws), settle the promise in the outer finally — settled once, whatever teardown did.
  • The engine's close report → this.hide({ action: 'dismiss' }), which is the only dismissal path there is.

Not carried:

  • ANCHOR_GAP, VIEWPORT_MARGIN, PARKED, clampBetween, popupHost, _reposition, _hideForZoom — all placement.
  • Every engine and window subscription — move, moveend, zoomstart, zoomend, resize and click. The service subscribes to nothing; the engine's close report reaches it through the callback it passed to showPopup.
  • Our own close button, the keydown handler, and the document.activeElement bookkeeping. All of that is the library's behaviour now, and where the library does nothing, nothing happens — recorded in the placement issue's "Later, not now".
  • guardNavigation and its click / submit capture listeners.
  • The 12-line UserInterfaceModern_.css hunk. It exists only because the reference mounted the card beside the map; a card inside the map container needs no z-index at all.
  • hideForCaller, the owner field, and the plugins:destroyed / plugins:allDestroyed listeners in the Map_.js hunk. map:hidePopup here has no ownership, and its handler takes no caller.
  • tests/unit/pluginTeardownPopup.spec.js.
  • The reference doc hunk's ownership prose for map:hidePopup, and its api.request(...) example.

Tests. Twelve of the reference's 30 it blocks port, plus one new block for the engine's close report:

  1. mounts one popup and leaves its request pending until it closes
  2. renders both actions and resolves with the primary on its click
  3. styles a lone secondary action as the primary button
  4. drops an action whose label is unusable
  5. renders the title as text rather than as markup
  6. rejects a request with neither a title nor html to show
  7. leaves an open popup alone when a later request is invalid
  8. resolves dismiss when the engine reports the library closed the popup (new)
  9. replaces the current popup and resolves the replaced request with closed
  10. retracts the popup for map:hidePopup, resolving its request with closed
  11. takes the card down even when the engine throws while retracting it
  12. rejects and unwinds when wiring the popup fails
  13. keeps the markup an author needs and strips the rest

#13 absorbs the reference's separate refuses script, handlers, and urls that run something (:599), which tests DOMPurify's own defaults rather than our config; its script/handler/javascript: cases fold into :583, where FORBID_TAGS: ['style'] is actually exercised. #10 loses the reference's hideForCaller (a plain hide now).

Everything else in the reference is placement, ownership, or behaviour the library now owns: the six repositioning/clamping/parking blocks (:386:467), retracts only for the caller that opened the popup (:554), puts the card beside the map container rather than inside it (:651), the two focus blocks (:295, :632), the dismissal-by-click blocks, the keyboard block, unsubscribes from the engine and the window when hidden, and refuses at the click anything that would navigate in place.

⚠️ Gotcha: Leaflet's popup _updateContent empties its content node and re-appends the element on open (node.removeChild in a loop, then node.appendChild(content)). Detaching a focused element blurs it, so focus the first action after the engine has placed the card, not before handing it over. The reference focuses inside its own mount, which is safe only because it mounted the card itself.

⚠️ Gotcha: the engine's close report must not fire for a hide this service asked for, or every map:hidePopup and every replacement would settle as dismiss instead of closed. That guard lives in the adapters (placement PR); the settle-once shape here is the second line of defence, not the first.

⚠️ Gotcha: the engine detaches the card on hide, so keep the reference to the element in the open-popup record rather than looking it up from the DOM.

References

  • src/essence/Basics/Map_/Map_.js:66, :279:396 — provider array and cleanups
  • src/essence/mmgisAPI/mmgisAPI.js:959, :977, :1021provide / request / forPlugin
  • src/essence/Basics/MapEngines/IMapEngine.ts — where showPopup / hidePopup land in the placement PR
  • src/styles/_theme-export.scss — the --theme-* custom properties
  • docs/pages/APIs/JavaScript/Main/Event-Bus-API.md:388:395 — Map Providers table
  • Reference snapshot: git diff feature/350-tool-identity-v2...feature/298-map-popup-service-v2; worktree /Users/cdavis/github/MMGIS-worktrees/298-popup

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions