feat(plugins): per-plugin deps, validation & shared discovery#975
Merged
Conversation
- TopBar: use IconButton for sign-in/kebab buttons, use Dropdown for user card popup (replaces hand-rolled outside-click + manual popup with Base UI Menu for keyboard nav, focus trap, accessibility) - BottomBarReact: use IconButton + Tooltip instead of raw <i> + tippy.js - Toolbar: use Tooltip instead of tippy.js for tool button tooltips - Reduce :global() from 33 to 22 instances — all remaining are justified by external jQuery/imperative code references: - SplitScreens: convert #viewerScreen/#globeScreen to scoped classes - Toolbar: convert #toolcontroller_incdiv to scoped class - Splitter: convert .splitterV to scoped class - UserInterfaceLayout: consolidate TopBar styles into TopBar.module.css - BottomBarReact: fully scoped (IconButton handles all button styling) Design system usage: 5 of 6 wrappers now imported across 3 files (Toggle, Dropdown, IconButton, Tooltip — only Button/Modal unused, which is appropriate since there are no standalone button/modal cases in the main site shell). Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Modal.js: React-based with imperative API bridge (Modal.set/remove) so existing jQuery callers (BottomBar, DrawTool, AnimationTool, etc.) work without changes. Uses React state + createRoot for rendering. - ConfirmationModal.js: Removed jQuery dependency, uses native DOM event listeners with the React Modal backend. - Help.js: Removed jQuery dependency, uses native fetch() + DOM event listeners instead of $.get() and $().on(). - ContextMenu.js: Removed jQuery dependency entirely. Uses native DOM APIs (createElement, addEventListener, querySelectorAll) for building and managing the right-click context menu. - Compass.js: Removed jQuery dependency. Uses native DOM APIs (getElementById, querySelector) for compass element creation and bearing updates. - MapLogo.js: Removed jQuery dependency. Uses native DOM APIs for logo element creation and Leaflet container insertion. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Modal.js: React component using Base UI Dialog with shared state store, imperative Modal.set()/remove() API preserved for backwards compatibility. Accepts both HTML strings (legacy callers) and React elements (new callers). - ConfirmationModal.js: React JSX component using design-system Button for Yes/No actions, rendered through Modal service. Same prompt() API. - Help.js: React JSX component using native fetch + showdown for markdown rendering, rendered through Modal service. Same getComponent/finalize API. - ContextMenu.js: Full React component with createRoot, JSX menu items with proper event handlers. Same init()/remove() API. - Compass.js: React component rendered via createRoot into Leaflet bottom-left container. SVG compass with bearing rotation on map events. - MapLogo.js: React component rendered via createRoot into Leaflet bottom-right container. Configurable size and link support. All components use CSS Modules. Old plain CSS files removed. Also fixes: - About modal now respects look.help/look.info config flags - Cleaned up dead #toggleTimeUI references in Coordinates.js Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Modal: Fix About modal not opening due to async createRoot in React 18. ModalHost now initializes from shared state and syncs on mount. Blur management moved to ModalHost (React state-driven), fixing persistent blur after modal close. - Tooltip/Dropdown: Use render prop on Trigger to avoid nesting <button> inside <button> (IconButton is already a button element). - MeasureTool: Migrate from deprecated ReactDOM.render to createRoot. Register Chart.js scales via Chart.register(...registerables) to fix 'linear is not a registered scale' error with react-chartjs-2 v4. - ContextMenu: Fix crash when right-clicking on LithoSphere scene (native events lack originalEvent). Store element and handler refs for proper cleanup, preventing listener leaks. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Remove the Dialog.Close button (curved bottom-left border-radius) - Drop Base UI Dialog wrapper entirely — it was fighting with the imperative Modal.set/remove API. Now uses simple divs with CSS transitions, matching the original jQuery modal behavior exactly. - Blur management is now purely imperative via _applyBlur() called synchronously in set() and remove(). Removed async useEffect approach. - Add 500ms CSS opacity fade-in/fade-out transition matching original. - Closing state: Modal.remove() marks modal as closing (triggers opacity 0 transition), then removes from DOM after 500ms. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… store, fix IconButton forwardRef, add Help.finalize calls - Rename Modal, ConfirmationModal, Help, ContextMenu, Compass, MapLogo from .js to .jsx - Route modal blur through Zustand modalBlurCount instead of imperative DOM manipulation - Remove conflicting jQuery blur animation in Layers_.js - Wrap IconButton with React.forwardRef to fix Tooltip/Menu trigger warnings - Add Help.finalize() calls to ChemistryTool, DrawTool, IsochroneTool - Change aboutModalContent config type from 'markdown' to 'textarea' Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…andles it Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Add null check for feature in handleActionClick WKT placeholder handling - Delay blur removal until after 500ms fade-out completes (blur stays in sync with backdrop opacity) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Dissolve src/essence/Ancillary/ entirely
- UI components → UserInterface_/components/ (Modal, ConfirmationModal, Help,
ContextMenu, Compass, MapLogo, CursorInfo, Attributions, Login)
- Layout chrome → UserInterface_/components/ (Description, Coordinates, Search,
ScaleBar, ScaleBox)
- Pure services → essence/services/ (DataShaders, LocalFilterer, QueryURL, Sprites)
- Stylize.js → design-system/ (theme bridge alongside themeApplier)
- Delete unused Swap.js
- Nest all components into own folders (ComponentName/ComponentName.ext pattern):
- UserInterface_/components/: TopBar/, Toolbar/, ToolPanel/, SplitScreens/,
Splitter/, BottomBar/, BottomElementPositioner/, Layout/, Panels/
- design-system/components/: Button/, IconButton/, Dropdown/, Toggle/, Modal/,
Tooltip/
- Update ~70+ import paths across codebase
- Build verified locally
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… issue - Modal.set() onAddCallback: Replace 50ms setTimeout with MutationObserver that waits for the modal element to appear in DOM before firing callback. Prevents silent jQuery binding failures on slower devices. - themeApplier: Use Proxy to read computed CSS custom properties (set by Stylize.js per-mission overrides) instead of hardcoded theme object values. Stylize.js now calls refreshThemeDOM() after setting CSS variables so inline styles reflect mission-specific color overrides. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
toggleTimeUI() now calls setTimeUIActive() and setTimeUIExpanded() so BottomFloatingBar visibility and BottomElementPositioner offsets reflect actual TimeUI state. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The element was accidentally dropped during the folder restructure move.
TimeUI.js and DrawTool.js check $('#toggleTimeUI').hasClass('active')
to gate histogram rendering and time-filter toggle visibility.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… redundant jQuery positioning
Restored pieces lost during folder restructure:
- Click handler in init() and off handler in remove()
- Tippy tooltip for the time toggle button
- display:none when time is not enabled
- $('#toggleTimeUI').toggleClass('active') so TimeUI.js can check it
- $('#CoordinatesDiv > #toggleTimeUI').remove() on mobile
Removed jQuery CSS positioning from toggleTimeUI() since
BottomElementPositioner now reactively handles all bottom-anchored
element offsets via the Zustand store.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…Node Use the stored _reactRoot.unmount() instead, matching the React 18 createRoot pattern already used in make(). Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Tooltips: - Reduce Base UI Tooltip delay from 600ms (default) to 200ms - Restyle tooltip popup to match tippy blue theme (var(--color-c2)) - Add Tooltip wrappers to TopBar panel toggles (Viewer/Map/Globe) - Wrap Toggle with forwardRef so Tooltip render prop can attach ref - Remove title attrs that conflicted with custom tooltips Scale indicator: - Remove scalefactor-specific positioning from BottomElementPositioner (it moves naturally with .leaflet-bottom.leaflet-left container) - Position scalefactor to the left of compass at same bottom level Modal blur: - Call _applyBlur() immediately when marking modal as closing so blur clears during fade-out instead of persisting 500ms Help modal: - Add close (X) button in title bar matching other modal patterns Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…and mmgis.css
- Delete tools.css entirely (both selectors #CurtainToolList and
.searchToolSelect are unreferenced anywhere in the codebase)
- Remove from mmgisUI.css: .mmgisRadioBar3/4/Vertical (140 lines),
.mmgispureselect (104 lines), blink/condemned_blink_effect (38 lines),
.slidecontainer/.slider (41 lines), .ar_slider (91 lines),
.verticalSlider (91 lines), .mmgisMultirange_elev (19 lines),
.ui-corner-all/bottom/right/br (9 lines)
- Remove from mmgis.css: #nodeenv, empty #topBar{}, #topBarInfo,
#topBarHelp, #topBarFullscreen, #toggleUI, #logoGoBack
- Keep #topBarLink (used in BottomBarReact.jsx), #webgl-error-message
(used by vendored THREE.js)
- All selectors verified with repo-wide grep before removal
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Replace Base UI Tooltip with simple React portal tooltip (200ms delay, tippy-matching style) — fixes missing tooltips for toolbar/topbar/bottom buttons - Add cursor + hover highlight to vertical splitters (was missing because module CSS didn't inherit global .splitterV styles) - Add hover highlight to tool panel drag handle - Remove mdi-drag-vertical icon from tool panel drag - Add mobile toolbar horizontal layout via @media query overrides - Add 4 new color schemes: High Contrast (a11y), Dark Mars, Dark Midnight, Light Warm (total: 10 themes) - Previous fixes also included in working tree: - timeUI border moved to toolsWrapper border-bottom (conditional) - #toggleTimeUI button removed entirely - CoordinatesDiv: vertical centering, unified background, 12px right offset - barBottom padding-bottom: 8px Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Pre-existing bug: `TimeControl.enabled = true` was assigning instead of comparing. Changed to `TimeControl.enabled === true`. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…eWhenField - Added all 10 theme presets to dropdown (was missing Dark Mars, Dark Midnight, Light Warm, High Contrast) - Added 'Custom' option: skips preset theme, uses only color picker values - Moved Theming section directly under Rebranding - Nested 'Custom Color Options' under Theming with subdescription - Added enableWhenField support to Maker.js: disables color pickers unless theme is set to Custom - Renamed color options with clearer names and improved descriptions: Primary → Surface Color, Secondary → Deep Background Color, Tertiary → Text Color, Body → Page Body Color, Highlight → Feature Highlight - Stylize.js: skip setTheme() when theme is 'Custom' - Rebuilt configure page Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
1. Tooltip: Replaced custom React portal tooltip with tippy.js wrapper. Uses the existing tippy.js dependency and 'blue' theme for consistency. 2. Dropdown: Replaced Base UI Menu with native portal dropdown. Base UI's nested Menu.Trigger + BaseButton composition was swallowing click events, breaking userAvatar and menuBtn menus. New implementation uses simple state + createPortal with proper outside-click dismissal. 3. About modal: Professional redesign with centered MMGIS ASCII art header, proper GitHub SVG logo link, clean metadata section, centered link buttons, attributions section, and NASA-AMMOS footer. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ation The global .mmgisHelpButton styles (yellow color, compact 18x18px sizing, 0.7 opacity) were removed when Help.css was converted to Help.module.css. Since Help.getComponent() emits raw HTML strings for jQuery-rendered tool headers, it cannot use CSS Module scoped classes. Restored the base styles in mmgis.css alongside the related .mmgisToolHelpBtn definition. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… theme, Stylize.js, Default Tool config - Login: skip session.regenerate() for token-based re-auth (useToken:true) so reloading the main page no longer invalidates the configure page session - About modal: replace ASCII art with mmgis.png logo, rename Attributions to Map Layer Attributions, remove footer logo, link NASA-AMMOS to ammos.nasa.gov - High Contrast theme: change accent from #ffff00 to #ffd700 (gold) for better contrast ratios against dark backgrounds - Stylize.js: color overrides only apply when theme is Custom or unset, preventing preset themes from being clobbered by stale config values - Restore Default Tool config section in tab-ui-config.json (accidentally removed during Theming section reorganization) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The case was accidentally removed during the Configure page UI tab restructure (d7f96c5). Without it, the Default Tool dropdown in the Configure page rendered as nothing despite the config referencing it. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- High Contrast theme: tooltips now use black text on yellow background via --color-c2-text variable (white for all other themes) - About modal links use var(--color-f) for consistent theme text color - Panel toggle buttons: 11px uppercase with 600 weight for better visibility - Mapping scale button moved to bottom-right of compass (was top-left) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Viewer: dropdown selector at top-right edge, OSD buttons stacked vertically below it; settings panel opens to the left - Globe: home, exaggerate, observe, walk, link controls moved from TopLeft to TopRight corner via addControl 4th arg - Style consistency: OSD buttons and LithoSphere controls now match Leaflet zoom controls (var(--color-a) bg, var(--color-f) text, var(--color-mmgis) hover, 30px size, 3px border-radius) - Viewer settings sliders use var(--color-a3) instead of hardcoded #444444 - Az/el indicator stays at bottom center (exception per design) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Modal theming: - All modals now share consistent styling: backdrop-filter blur, semi-transparent background via --color-a-rgb, 10px border-radius, header divider line, box-shadow - Updated: loginModal, Help, ConfirmationModal, Settings, Hotkeys, About modals - Tool panel backgrounds changed from opaque var(--color-k) to transparent so the ToolPanel's existing backdrop-filter effect shows through - Legend tool header updated to match consistent 44px height with divider - applyTheme.js now auto-derives --color-a-rgb from theme's --color-a hex value - Modal service wrapper gets backdrop-filter: blur(12px) Session security (Devin Review fix): - Token re-auth now calls req.session.regenerate() with data preservation to prevent session fixation while maintaining multi-tab compatibility - Token is rotated via crypto.randomBytes on every re-auth (was being reused) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…n header 1. Viewer OSD settings moved to top of button stack, panel opens downward 2. Overlay buttons consistent 30x30px (OSD line-height fix, home button) 3. Menu/icon contrast improved: Dropdown items and IconButtons use --color-a5 (was --color-a3) with --color-f on hover for better dark theme legibility 4. CoordinatesDiv fixed to 30px height, pickLngLat button centered 5. Login modal now has a header bar with 'Log In' title and close X button; title toggles to 'Sign Up' when switching modes Also reverts session regeneration for token re-auth (Devin Review feedback): token-based re-auth now refreshes session data in-place without regeneration or token rotation, preserving multi-tab compatibility. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…icator, toast 1. Description nav popover: added z-index:9000 so menu appears above map panels 2. Separated tools: default color changed from accent to --color-f; fixed CSS selector from .toolButtonSep to .toolSep to match actual class names 3. Compass + mapping scale shifted left by 30px for better positioning 4. Map zoom/home controls: use --color-f instead of accent --color-c to reduce visual prominence; hover still highlights with accent color 5. Status indicators (reload/ws disconnect/layer update) moved from Leaflet control to TopBar with soft pulsing fade animation and tooltip on hover 6. WebSocket retry toast: rounded corners, glass background with backdrop-filter, border-left accent for failure state instead of solid red background Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Mirrors the reloadTimeLayers fix: a single failing TimeControl.reloadLayer call (e.g. unknown layer name throws inside asLayerUUID, network error, malformed config) no longer rejects the whole batch. The returned array preserves order and reports failed entries as false instead, matching the documented Promise<boolean[]> contract. Addresses second Devin Review finding on PR #78. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The implementation changed from sync to async in commit a096cfe (the function now uses await + Promise.allSettled internally to coordinate per-layer reloads), but the public API JSDoc and Main.md still documented the old synchronous return type. External consumers using the old synchronous return value would get a Promise instead of an array. Updates JSDoc on mmgisAPI.reloadTimeLayers to declare Promise<string[]>, and rewrites the Main.md example to use 'await'. Also fixes the previous example, which had a syntactically-malformed trailing tuple-style index. Addresses third Devin Review finding on PR #78. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Moves the two time-feature specs out of tests/e2e/map/ so the map suite
stays focused on map-UI behavior and the time/time-enabled-layer suite
can be run (and reasoned about) on its own:
- tests/e2e/map/time-control.spec.js
-> tests/e2e/time/time-control.spec.js
- tests/e2e/map/concurrent-layer-reload.spec.js
-> tests/e2e/time/concurrent-layer-reload.spec.js
Relative imports (../../helpers, ../../pages, ../../fixtures) are
unchanged because the new directory is the same depth. Playwright
picks the files up automatically via testDir './tests' +
testMatch '**/*.spec.js'.
Adds a matching 'npm run test:e2e:time' script and documents the new
suite in tests/README.md alongside the existing per-suite scripts.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
The previous `workers: process.env.CI ? 1 : undefined` resolved to Playwright's default (~half the CPU cores). On higher-core machines (e.g. 16 cores -> 8 workers) the dev server gets overloaded and the suite actually runs slower. CI behavior is unchanged (1 worker). Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…tating layer.url
The previous fix in Map_.refreshLayer temporarily swapped
`layerObj.url = resolvedUrl` during the `await makeLayer` call and
restored it in a finally block. Since `layerObj` is the shared
`L_.layers.data[name]` object, any concurrent code reading
`layer.url` during that async window could observe the resolved URL
instead of the template. Most importantly, a second
`TimeControl.reloadLayer()` call would then capture the resolved
URL as its 'template' and corrupt the placeholders for every
subsequent reload.
Surfaced by tests/e2e/time/concurrent-layer-reload.spec.js Test 5,
which after Promise.all of two reloads observed
`layer.url ==='geodatasets:...?from=...&to=...'` instead of the
expected `{starttime}/{endtime}` template.
Fix: thread `resolvedUrl` as an explicit parameter through
`Map_.refreshLayer` -> `makeLayer` -> `makeVectorLayer` ->
`captureVector` (via options.resolvedUrl). `captureVector` uses
`options.resolvedUrl` when provided and skips the
`{starttime}`/`{endtime}`/`{customtime.*}` regex replacement
(which TimeControl.reloadLayer already performed). `layer.url` is
NEVER mutated for the duration of the async operation, so the
template is preserved across overlapping reloads.
This also fully resolves the Devin Review #4 finding which flagged
the temporary URL swap as reintroducing the same race the PR was
meant to fix.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
In the previous fix, captureVector skips its time-replacement block when
the caller supplies options.resolvedUrl (because TimeControl.reloadLayer
already performed those replacements). However TimeControl.reloadLayer
was only replacing {starttime}/{endtime}/{customtime.*} on resolvedUrl,
not {time} — which captureVector previously handled at
LayerCapturer.js:97 by mapping {time} -> endTime. This caused vector
layers using the documented {time} placeholder (see
docs/pages/Configure/Layers/Tile/Tile.md:90 and
docs/pages/APIs/JavaScript/Main/Main.md:482) to fetch URLs containing
the literal text '{time}' on time-triggered reloads.
Mirror the existing captureVector behavior: replace {time} with the
formatted end-time value alongside {starttime}/{endtime}, before the
resolved URL is threaded through Map_.refreshLayer -> makeLayer ->
captureVector.
Addresses Devin Review finding on commit fcba810.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…tent)
The previous fix gated captureVector's time-placeholder replacement
block on `!hasResolvedUrl`, on the assumption that any caller passing
options.resolvedUrl had already done the replacement.
That assumption only holds for time.type === 'global' / 'requery' /
forceRequery. For other time types that still flow through
Map_.refreshLayer into captureVector (most importantly
time.type === 'local' with endProp == null per TimeControl.js:276-287),
TimeControl.reloadLayer's resolved-URL replacement block at lines 249-273
is skipped, so the resolvedUrl arrives at captureVector still containing
literal {starttime}/{endtime}/{time} placeholders. The fetch then goes
out with unreplaced placeholders.
Fix: drop the !hasResolvedUrl guard and always run the replacement,
reading the source from `layerUrl` (which is already either
options.resolvedUrl or layerObj.url per the choice above). The
.replace(/{starttime}/g, ...) chain is idempotent on URLs that have
already been resolved — the regexes simply don't match — so the
correct path is restored without re-introducing the mutate-in-place
bug fcba810 fixed.
Addresses Devin Review finding on commit ddb90db.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ndency) The test at tests/e2e/time/time-control.spec.js depended on gibs.earthdata.nasa.gov being reachable from the test environment and served only as a placeholder — it skips every run since the test infrastructure does not have external network access by policy. It provides no signal in CI or locally, so removing it reduces noise in the suite output without losing coverage. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…e-presence cases
Four new e2e tests in tests/e2e/time/concurrent-layer-reload.spec.js,
each targeting a gap the original suite missed:
Test 8 — {time} placeholder preservation. Mirrors Test 2/5 but uses
`{time}` instead of `{starttime}/{endtime}`. Catches the Devin
Review #5 regression where captureVector's gating on !hasResolvedUrl
silently dropped the {time} -> endTime replacement (the literal
'{time}' would have ended up in the fetch URL).
Test 9 — local + endProp==null path. Sets layer.time.type='local'
and layer.time.endProp=null to force the TimeControl.reloadLayer
branch (TimeControl.js:276-287) that bypasses the resolved-URL
placeholder block and falls through to the else clause. Inspects
outgoing /geodatasets/* requests via page.on('request', ...) and
asserts no literal {starttime}/{endtime}/{time} remain. Catches the
Devin Review #6 regression: when this branch hit captureVector with
hasResolvedUrl=true, the !hasResolvedUrl gate previously short-
circuited the only remaining replacement site.
Test 10 — 20-reload stress burst. Extends Test 4's two-reload check
to 20 concurrent reloadLayer() calls, capturing 'Cannot make layer'
warnings to verify the queue coalesces requests instead of silently
dropping them. Also re-asserts layer.url template integrity post-
burst.
Test 11 — Feature-presence after concurrent reload. Captures
L_.layers.layer[key].getLayers().length before and after a 5-reload
burst. Asserts the count is still > 0 afterwards — the user-visible
'gaps where dynamically-appearing data doesn't show up' symptom from
the original bug report.
All four tests skip gracefully when their fixture layer is absent or
the dataset returns no rows, to avoid spurious failures across
mission configurations.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Five additional tests in tests/e2e/time/concurrent-layer-reload.spec.js
covering paths adjacent to the URL-mutation fix that were not exercised
by tests 1-11. Each was chosen because the production code on that path
changed (or now has a different contract) and a regression would not be
caught by the original race-condition tests.
Test 12 — {customtime.N} placeholder preservation. TimeControl.reloadLayer's
customtime replacement loop was migrated from `layer.url = ...` to
`resolvedUrl = ...`. Seeds TimeControl.customTimes.times so the loop
actually runs, then asserts the {customtime.0} placeholder remains
literally on layer.url after reload.
Test 13 — mmgisAPI.reloadTimeLayers() returns a Promise. This is the
backward-incompatible behavior change documented in
docs/pages/APIs/JavaScript/Main/Main.md (previously synchronous).
Asserts the returned value is a thenable that resolves to an array,
pinning the new contract so it does not silently regress.
Test 14 — mmgisAPI.reloadLayers handles unknown layer names. The
Promise.allSettled change requires that a failing per-layer reload
surfaces as `false` at the same array position as its input name,
without throwing. Mixes a valid name + an unknown name + another
valid name to verify the order and the boolean mapping.
Test 15 — Reloading a time-DISABLED vector layer leaves layer.url
unchanged. Discovers a candidate layer from L_.layers.data at runtime
(skips if none exist), reloads it, and asserts the URL is byte-equal
afterwards. Catches any accidental URL mutation introduced for the
non-time code path.
Test 16 — mmgisAPI.reloadLayers handles empty array, null, undefined,
and string inputs without throwing — verifying the Array.isArray guard
at mmgisAPI.js:618. Returns `[]` in all cases.
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ways run
Address Devin Review finding: a thrown exception from any layer-type
builder (makeVectorLayer, makeVelocityLayer, etc.) inside makeLayer's
switch statement previously left lockRegistry[layerName] set to true
and skipped the queue drain entirely, since the release statement and
queue-drain block both lived AFTER the awaited dispatch.
Effect of the bug: any subsequent refreshLayer call for that layer
would queue against a permanently-locked entry that never drains.
The new queue mechanism inherits this pre-existing issue and makes
the failure mode worse — silent accumulation in _layerReloadQueue
instead of a visible 'Cannot make layer' warning.
Additional concern: the outer 'new Promise(async (resolve, reject) =>
{...})' is the async-executor anti-pattern. A throw inside the async
executor escapes to the unhandled-rejection handler instead of
rejecting the outer Promise — so the caller's 'await makeLayer(...)'
would hang indefinitely, compounding the lock-leak symptom.
Fix:
- Wrap the type-dispatch switch + Filtering.updateGeoJSON/
triggerFilter calls in a try/catch/finally.
- catch logs the error and tracks success via 'madeSuccessfully'.
- finally runs unconditionally: lockRegistry[layerName] = false,
drain L_._layerReloadQueue[layerObj.name] if present, then
resolve(madeSuccessfully). This ensures the lock release and
queue drain happen regardless of whether the inner builder
threw or completed normally.
Test (concurrent-layer-reload.spec.js):
Added probe-style test '_layersBeingMade lock is released after
single and concurrent reloads' that asserts the lock invariant:
1. After mmgisAPI.reloadLayer() resolves +
a 100ms drain window, _layersBeingMade[key] is false.
2. After 5 concurrent mmgisAPI.reloadLayer() calls resolve +
a 1000ms drain window, _layersBeingMade[key] is false.
3. _layerReloadQueue is empty afterwards (otherwise a future
reload would mistakenly trigger an immediate drain instead
of doing its own work).
This is a positive-invariant test — it catches accidental lock
retention even without force-triggering exceptions, which would
require monkey-patching webpack-internal module references that
aren't exposed on window.
Per user direction, NOT addressing Devin Review's separate finding
about the reloadTimeLayers sync->async breaking change at this
time (no version bump requested).
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…er-reload fix: serialize concurrent layer reloads and stop mutating layer.url
…ements Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…lopment Merges upstream/development (9966a84 - MMGIS 5.0.0) into JPL-Devin development branch. All upstream 5.0.0 changes were already incorporated into JPL-Devin via prior sync PRs (#65, #70). The 4 merge conflicts (configure/package.json, package.json, Map_.js, TimeControl.js) were resolved by keeping JPL-Devin's versions which include additional bug fixes: - Concurrent layer reload serialization (PR #78) - resolvedUrl threading to avoid layer.url mutation race - Promise.allSettled in reloadTimeLayers - try/finally lock release in makeLayer - Version kept at 5.0.8-20260513 (ahead of upstream 5.0.7) Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Merge upstream NASA-AMMOS/MMGIS development into JPL-Devin
…37-plugin-system-improvements
…tays visible Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ncies When a plugin tool/component/backend overrides a standard one by reusing the same directory name, only the override's deps should contribute to the aggregated plugin manifests. Previously, gatherDependencies() concatenated standard + plugin entries and fed both to mergeNpm/ mergePython, which could spuriously flag the same package as conflicting between the standard and override versions. Extract winnersByName() (mirroring API/updateTools.js + API/setups.js override behavior) and use it for all three plugin kinds. Add unit tests covering the override case and the spurious-conflict regression. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Plugin deps installed in the builder via `npm install --no-save --no-package-lock` aren't recorded in package.json/package-lock.json, so the runtime stage's `npm ci --only=production` would lose them. Frontend deps are bundled by webpack into ./build so they're fine, but backend plugins that `require()` their declared npm dependencies at runtime would crash with 'Cannot find module'. Copy plugin-package.json from the builder and re-run the same conditional install in the runtime stage so backend plugin deps land in the runtime image's node_modules. `--ignore-scripts` prevents the inner install from re-entering the root postinstall hook. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Add #operationsClock to BottomElementPositioner's reactive positioning so it shifts to bottom:58px when timeUIActive is true (TimeUI dock visible) and back to bottom:40px when closed. Avoids overlap with the bottom floating bar without adding new state to OperationsClock itself. Mobile path is unchanged — OperationsClock.setupMobilePositioning manages mobile positioning separately. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Revert the dynamic positioning in BottomElementPositioner and just hardcode bottom:58px in OperationsClock.css. Simpler, no cross-cutting state dependency. Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…improvements feat(plugins): per-plugin deps, lazy tool loading, validation & shared discovery
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
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.
With Devin: JPL-Devin#77
Purpose
End-to-end improvement of the MMGIS plugin system across four phases. The work is consolidated into one PR but each phase is independently reviewable in commit history. The end state is a plugin system that:
config.jsonat discovery time and skips/loudly logs invalid ones,Proposed Changes
Phase 3 — Plugin config validation + override warnings (API/pluginValidation.js, API/updateTools.js)
validatePluginConfig(config, name, type)validates required (name,paths) and optional fields, the shape ofpaths, thedependenciesblock, and warns on unknown top-level fields.Tool '<name>' overridden by <source>when a plugin replaces a previously-registered tool.updateTools()andupdateComponents()route every manifest throughregisterPlugin()which validates, skips invalid ones, and logs override warnings.Phase 2 — Shared
discoverPlugins()utility (API/pluginDiscovery.js, API/setups.js)discoverPlugins(basePath, patterns, configFile?, opts?)— single source of truth for plugin scanning. Supports exact-name patterns ("__exact:Tools") and substring patterns ("Plugin-Tools"), three loader modes (parse/require/none), and skips dot/underscore-prefixed dirs at both container and plugin levels.updateTools(),updateComponents(), andgetBackendSetups()all refactored on top of this helper. Backend discovery is now synchronous instead of nested-callback-with-counter.Phase 1 — Per-plugin dependency declaration + build-time aggregation (scripts/resolve-plugin-deps.js, scripts/build.js, Dockerfile, src/essence/Tools/Animation/config.json, .gitignore)
config.jsonmay include adependenciesblock:scripts/resolve-plugin-deps.jsaggregates every plugin's deps and writes three gitignored artifacts:plugin-package.json(npm)plugin-python-requirements.txt(pip)plugin-conda-deps.txt(conda)scripts/build.jscallsresolvePluginDeps()beforeupdateTools()/updateComponents().Dockerfilerunsnode scripts/resolve-plugin-deps.jsafterCOPY . .and then installs the aggregated npm deps (npm install --no-save --no-package-lock --ignore-scriptsso the root lockfile is untouched) and pip deps (inside themmgismicromamba env).src/essence/Tools/Animation/config.json. Left in rootpackage.jsonfor the transition so local dev keeps working withnpm install..gitignore: adds the three generatedplugin-*artifacts.Tests & docs
tests/fixtures/test-plugin-tools/{TestPlugin,InvalidPlugin,OverridePlugin}fixture plugins.tests/helpers/plugin-helpers.js— install/uninstall fixture plugins with optionalinstallAsrename.pluginValidation,pluginDiscovery,updateTools,resolvePluginDeps(57 tests total).CONTRIBUTING.md— new "Pluginconfig.jsonSchema", "Override Behavior", and "Plugin Dependencies" sections covering schema, build-time aggregation, conflict detection, Docker integration, and migration notes.docs/pages/Contributing/Contributing.md— mirrored documentation updates.Issues
None — feature work scoped to plugin system.
Testing
`PLAYWRIGHT_TEST_UNIT_ONLY=true npx playwright test tests/unit/pluginValidation.spec.js tests/unit/pluginDiscovery.spec.js tests/unit/updateTools.spec.js tests/unit/resolvePluginDeps.spec.js
node scripts/resolve-plugin-deps.jsproduces the expectedplugin-package.jsoncontaining the Animation tool's 5 npm deps.node --checkclean on every modified.jsfile (API/*,scripts/*,src/essence/Basics/ToolController_/ToolController_.js, …).Existing E2E test infrastructure (Playwright) untouched and will run on CI.