Skip to content

Swipe mode fix ups round 2: secondary layer style sync and selection routing#1616

Draft
Copilot wants to merge 6 commits intomasterfrom
copilot/fix-swipe-mode-layer-style
Draft

Swipe mode fix ups round 2: secondary layer style sync and selection routing#1616
Copilot wants to merge 6 commits intomasterfrom
copilot/fix-swipe-mode-layer-style

Conversation

Copy link
Contributor

Copilot AI commented Mar 16, 2026

Two bugs in swipe mode: (1) layer style edits on the secondary map weren't reflected in the OL canvas until a map switch/re-activate cycle, and (2) clicking to select on the secondary side corrupted primary map selection state — and persisted after deactivating swipe.

Issue 1 — Secondary layer styles not applied live

useViewerSideEffects only watched useActiveMapLayers() (primary map). Redux updates to the secondary map's layer state never triggered layerManager.apply() for that map.

  • containers/hooks.ts: Added useNamedMapLayers(mapName) — same equality semantics as useActiveMapLayers but for an arbitrary map name.
  • containers/neo-map-viewer.tsx: When swipe is active, derive secondaryMapName from useMapSwipeInfo() and watch its layers via useNamedMapLayers. A new useEffect calls context.getLayerManager(secondaryMapName).apply(secondaryLayers) on change:
const secondaryMapName = isSwipeActive ? swipeInfo?.pair.secondaryMapName : undefined;
const secondaryLayers = useNamedMapLayers(secondaryMapName);
React.useEffect(() => {
    if (context.isReady() && secondaryMapName && secondaryLayers) {
        context.getLayerManager(secondaryMapName).apply(secondaryLayers);
    }
}, [context, secondaryMapName, secondaryLayers]);

Issue 2 — Selection routing when clicking secondary side

Two separate bugs compounding the problem:

  • queryWmsFeatures — wrong Redux target: addClientSelectedFeature was called without mapNameOverride, so WMS feature selections on the secondary side were stored under the primary map name in Redux. Fixed by passing mapName to the callback.

  • onMapClick — spurious primary query: When clicking the secondary side with no vector/WMS hit, onProviderMapClick(px) fired unconditionally. For MapGuide maps this dispatched a QUERYMAPFEATURES for the primary map using secondary-side pixel coordinates, producing a phantom selection. Added isClickOnPrimary guard:

const isClickOnPrimary = !this._swipeSecondaryMapName || effectiveMapName === this._state.mapName;
// ...
if (!madeSelection) {
    if (isClickOnPrimary) {
        this.onProviderMapClick(px);
    }
}

After deactivation _swipeSecondaryMapName is undefined, so isClickOnPrimary is always true and normal click behaviour is fully restored.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.0/x64/bin/node /opt/hostedtoolcache/node/24.14.0/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/.bin/storybook build -c .storybook c/components/tooltips/selected-features.ts c/components/tooltips/feature.ts /home/REDACTED/work/_temp/copilot-developer-action-main/ebpf/in-path/mkcert (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>Swipe mode fix ups round 2</issue_title>
<issue_description>Some new issues discovered with the recently introduced swipe mode:

  1. In split mode, changing the style of any layer on the secondary map is not reflected on the secondary map. See attached screenshot were I was able to change the layer style of the layer on the primary map, but doing this to the layer on the secondary map (by changing color to purple-pink) does not apply. It still looks bright green.

Image

However, if after this I map switch to secondary and the back to primary and re-activate swipe mode, the secondary map will have the correct style that I applied, so there must be some "update OL layer's internal style with the latest settings from its layer style model" code path that not being it if I were applying layer style changes on the secondary map in swipe mode.

  1. Also in swipe mode, if I select on the secondary map, I lose the ability to select on the primary map. Hovering still happens on the primary map. Deactivating swipe mode does not fix this.</issue_description>

Comments on the Issue (you are @copilot in this section)


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix layer style updates in swipe mode Swipe mode fix ups round 2: secondary layer style sync and selection routing Mar 16, 2026
Copilot AI requested a review from jumpinjackie March 16, 2026 05:13
@jumpinjackie
Copy link
Owner

@copilot Some observations.

  1. Secondary styles are now applied live. 👍

  2. Selection still broken, but now it seems only on the visual side. The model backend the selection panel, seems to acknowledge selections made on the primary map after making selections on the secondary map, but the OL vector selection overlay on the primary side is not updating.

  3. We are now getting a script error like this:

Uncaught Error: Duplicate item added to a unique collection
    at Collection.assertUnique_ (Collection.js:320:1)
    at Collection.insertAt (Collection.js:203:1)
    at MgLayerSetGroup.attach (layer-set-group-base.ts:176:16)
    at MapGuideMapProviderContext.setProviderState (mapguide.ts:680:25)
    at MapViewer (neo-map-viewer.tsx:131:13)

Caused by the following sequence.

Activate swipe mode
Open Layer Manager UI
Ensure primary map is selected. Add layer to it.
Ensure secondary map is selected. Add layer to it.
Select a boundary on the primary map. Get selection.
Select a boundary on the secondary map. Boundary on primary is deselected. Get selection on secondary.
Select a boundary on the primary map again.
Expect: Boundary is selected on primary. Boundary on secondary is deselected.
Get:Get no selection on primary. Boundary on secondary is deselected.
Now switch to the secondary map. Get the above script error.

@coveralls
Copy link

coveralls commented Mar 16, 2026

Coverage Status

coverage: 40.35% (+0.001%) from 40.349%
when pulling bd2c2c2 on copilot/fix-swipe-mode-layer-style
into 9aaaa7f on master.

…ss-map feature hit detection

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 16, 2026

@copilot Some observations.

  1. Secondary styles are now applied live. 👍
    ...

Fixed both remaining issues in commit 0e60b0b:

Duplicate item error on map switch: The root cause was my secondary-layer-sync useEffect calling layerManager.apply() on the secondary layer set. apply() was unconditionally adding the secondary layer set's helper layers (scratchLayer, wmsSelOverlayLayer, hoverHighlightLayer) to the OL map if they weren't at the top — even though they'd never been attach()'d. deactivateMapSwipe didn't know about them, so they remained on the OL map, causing attach() to throw "Duplicate item" when inserting them again. Fixed by adding an Array.includes guard so those layers are only repositioned if they are already on the OL map.

OL vector selection overlay not updating: OL's forEachFeatureAtPixel does not respect canvas clip regions. Secondary map layers sit higher in the OL layer stack and have features at the same map coordinates as primary layers (just visually clipped to the right). Clicking on the primary side was finding secondary features first, so the primary visual selection never updated. Fixed by adding ownsCustomLayer(layer) to LayerSetGroupBase and using it in onMapClick to restrict forEachFeatureAtPixel results to layers owned by the map that owns the click pixel.

@jumpinjackie
Copy link
Owner

@copilot Another follow up.

The duplicate item script error is gone 👍 but we still do not have the ability to select after selecting different maps in swipe mode. To assist, here's screenshots of the whole sequence after adding layers to both.

Selecting a boundary on the primary map works and shows its feature popup. As an aside the swipe mode splitter is being drawn above the popup, when the popup should really be above it.

Screenshot_20260316_193501

Then selecting a boundary on the secondary map deselects from the primary and selects the boundary on the secondary map and shows its feature popup.

Screenshot_20260316_193510

If I select a boundary on the primary map again, the secondary map deselects, but I get no selected boundary or feature popup, only the hover selection state.

Screenshot_20260316_193522

If I were to switch to just the secondary map, selecting boundaries does not give me a selected boundary or feature popup, only the hover selection state. Browser console chatter suggests that the redux state is being updated.

Screenshot_20260316_193552

If I were to switch to just the primary map, selecting boundaries does not give me a selected boundary or feature popup, only the hover selection state. Browser console chatter suggests that the redux state is being updated.

Screenshot_20260316_193608

Copy link
Contributor Author

Copilot AI commented Mar 16, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.0/x64/bin/node /opt/hostedtoolcache/node/24.14.0/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/.bin/storybook build -c .storybook (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jumpinjackie
Copy link
Owner

@copilot I think we're on the home stretch. Selection works on both sides in swipe-mode and also before/after toggling swipe mode. The only issue is that selection in general (swipe mode or not) does not seem to reliably visually update if there is a hover selection overlay currently over the boundary I just clicked in select mode.

For example, in this screenshot, when I click the boundary with the red outline (from the hover selection), there is redux actions dispatched and the selection panel shows the properties of the boundary I selected, but the boundary itself does not visually become selected and there is no feature popup.

Screenshot_20260316_215201

Copilot stopped work on behalf of jumpinjackie due to an error March 16, 2026 13:36
@jumpinjackie
Copy link
Owner

@copilot Coveralls reported a drop in coverage. Pls add/update test to ensure any new or changed code has been covered.

…amedMapLayers()

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 16, 2026

@copilot Coveralls reported a drop in coverage. Pls add/update test to ensure any new or changed code has been covered.

Added tests in commit fcc0df3:

test/api/layer-set-group-swipe.spec.ts — 7 new tests covering:

  • ownsCustomLayer(): false for unrelated layers, true for addLayer()'d layers, false after transferLayerOut(), true after transferLayerIn(), false for internal helper layers (scratchLayer etc.)
  • apply() helper-layer includes guard: verifies scratchLayer/wmsSelOverlayLayer/hoverHighlightLayer are NOT added to the OL map when the layer set has not been attach()'d (the secondary-swipe scenario that was causing the "Duplicate item" error)

test/containers/hooks.spec.tsx — new file, 6 tests covering useNamedMapLayers(): undefined when mapName is undefined, undefined when map absent from state, undefined when layers is undefined, correct layers when map exists, reactivity to store changes (dispatch SET_LAYER_VISIBILITY → hook reflects updated visibility), and correct layers when mapName changes between two maps.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • storybook.js.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.0/x64/bin/node /opt/hostedtoolcache/node/24.14.0/x64/bin/node /home/REDACTED/work/mapguide-react-layout/mapguide-react-layout/node_modules/.bin/storybook build -c .storybook (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@jumpinjackie
Copy link
Owner

@copilot Your most recent change has errors. Pls fix:

ERROR in /home/runner/work/mapguide-react-layout/mapguide-react-layout/test/containers/hooks.spec.tsx
137:27-140:13
[tsl] ERROR in /home/runner/work/mapguide-react-layout/mapguide-react-layout/test/containers/hooks.spec.tsx(137,28)
      TS2345: Argument of type '{ type: ActionType; payload: { mapName: string; layerName: string; visible: boolean; }; }' is not assignable to parameter of type 'never'.

…r instead of raw dispatch

Co-authored-by: jumpinjackie <563860+jumpinjackie@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 16, 2026

@copilot Your most recent change has errors. Pls fix:

...

Fixed in commit e782d60. The raw store.dispatch({ type: ActionType.SET_LAYER_VISIBILITY, ... }) clashed with the store's strict dispatch type. Replaced with the typed action creator setMapLayerVisibility("Sherbrooke", "roads", false) instead.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swipe mode fix ups round 2

3 participants