From cfd07add082ad73818108511e9bcc7eb2f81b201 Mon Sep 17 00:00:00 2001
From: Dan Leech <dan.leech@defra.gov.uk>
Date: Tue, 5 May 2026 17:07:00 +0100
Subject: [PATCH] Selected state on style chnage fix

---
 .../interact/src/hooks/useHighlightSync.js    | 14 +++++++++----
 .../src/hooks/useHighlightSync.test.js        | 21 +++++++++++++------
 src/services/eventBus.js                      | 15 +++++++++++++
 src/services/eventBus.test.js                 | 11 ++++++++++
 4 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/plugins/interact/src/hooks/useHighlightSync.js b/plugins/interact/src/hooks/useHighlightSync.js
index 29878f1c..5b6450fa 100755
--- a/plugins/interact/src/hooks/useHighlightSync.js
+++ b/plugins/interact/src/hooks/useHighlightSync.js
@@ -53,12 +53,18 @@ export const useHighlightSync = ({
     updateHighlightedFeatures()
 
     // Re-apply after style reload — highlight layers are removed when style reloads.
-    // MAP_DATA_CHANGE is NOT used here because addLayer/moveLayer fire styledata,
-    // which would create an infinite update loop via MAP_DATA_CHANGE.
-    eventBus.on(events.MAP_STYLE_CHANGE, updateHighlightedFeatures)
+    // MAP_STYLE_CHANGE fires as soon as the new style is ready, before any plugin has
+    // re-added its layers, so we register a one-time MAP_DATA_CHANGE listener instead.
+    // MAP_DATA_CHANGE is debounced and fires once all layer additions from every plugin
+    // have settled, giving us a single well-timed trigger. Using once() avoids the
+    // infinite loop a permanent listener would cause (layer moves re-trigger MAP_DATA_CHANGE).
+    const handleStyleChange = () => {
+      eventBus.once(events.MAP_DATA_CHANGE, updateHighlightedFeatures)
+    }
+    eventBus.on(events.MAP_STYLE_CHANGE, handleStyleChange)
 
     return () => {
-      eventBus.off(events.MAP_STYLE_CHANGE, updateHighlightedFeatures)
+      eventBus.off(events.MAP_STYLE_CHANGE, handleStyleChange)
     }
   }, [selectedFeatures, listboxActiveItem, mapProvider, stylesMap, eventBus])
 }
diff --git a/plugins/interact/src/hooks/useHighlightSync.test.js b/plugins/interact/src/hooks/useHighlightSync.test.js
index 9570030b..24e56c9e 100644
--- a/plugins/interact/src/hooks/useHighlightSync.test.js
+++ b/plugins/interact/src/hooks/useHighlightSync.test.js
@@ -7,16 +7,19 @@ jest.mock('../utils/buildStylesMap.js', () => ({
 }))
 
 const STYLE_CHANGE = 'map:stylechange'
+const DATA_CHANGE = 'map:datachange'
 
 let mockDeps
-let capturedEventHandler
+let capturedStyleChangeHandler
+let capturedDataChangeHandler
 
 const render = (overrides = {}) =>
   renderHook(() => useHighlightSync({ ...mockDeps, ...overrides }))
 
 beforeEach(() => {
   jest.clearAllMocks()
-  capturedEventHandler = null
+  capturedStyleChangeHandler = null
+  capturedDataChangeHandler = null
 
   mockDeps = {
     mapProvider: {
@@ -28,11 +31,16 @@ beforeEach(() => {
     },
     selectedFeatures: [],
     dispatch: jest.fn(),
-    events: { MAP_STYLE_CHANGE: STYLE_CHANGE },
+    events: { MAP_STYLE_CHANGE: STYLE_CHANGE, MAP_DATA_CHANGE: DATA_CHANGE },
     eventBus: {
       on: jest.fn((event, handler) => {
         if (event === STYLE_CHANGE) {
-          capturedEventHandler = handler
+          capturedStyleChangeHandler = handler
+        }
+      }),
+      once: jest.fn((event, handler) => {
+        if (event === DATA_CHANGE) {
+          capturedDataChangeHandler = handler
         }
       }),
       off: jest.fn()
@@ -107,13 +115,14 @@ describe('useHighlightSync — styles memoization', () => {
 // ─── useHighlightSync — style-change event ───────────────────────────────────
 
 describe('useHighlightSync — style-change event', () => {
-  it('refreshes highlights on MAP_STYLE_CHANGE', () => {
+  it('refreshes highlights after MAP_STYLE_CHANGE then MAP_DATA_CHANGE', () => {
     mockDeps.selectedFeatures = [{ featureId: 'F1', layerId: 'layer1' }]
 
     render()
     mockDeps.mapProvider.updateHighlightedFeatures.mockClear()
 
-    act(() => capturedEventHandler())
+    act(() => capturedStyleChangeHandler())
+    act(() => capturedDataChangeHandler())
 
     expect(mockDeps.mapProvider.updateHighlightedFeatures).toHaveBeenCalled()
   })
diff --git a/src/services/eventBus.js b/src/services/eventBus.js
index 489c3b47..09f062fc 100755
--- a/src/services/eventBus.js
+++ b/src/services/eventBus.js
@@ -31,6 +31,21 @@ class EventBus {
     return this
   }
 
+  /**
+   * Register a one-time handler that removes itself after the first call.
+   *
+   * @param {string} eventName
+   * @param {Function} handler
+   * @returns {this}
+   */
+  once (eventName, handler) {
+    const wrapper = (...args) => {
+      this.off(eventName, wrapper)
+      handler(...args)
+    }
+    return this.on(eventName, wrapper)
+  }
+
   /**
    * Remove a handler for an event. Omit `handler` to remove all handlers.
    *
diff --git a/src/services/eventBus.test.js b/src/services/eventBus.test.js
index 27e6ac2b..874b5cf5 100755
--- a/src/services/eventBus.test.js
+++ b/src/services/eventBus.test.js
@@ -82,6 +82,17 @@ describe('EventBus singleton', () => {
     console.error.mockRestore()
   })
 
+  it('once fires the handler exactly once', () => {
+    const handler = jest.fn()
+    eventBus.once('single', handler)
+
+    eventBus.emit('single', 'a')
+    eventBus.emit('single', 'b')
+
+    expect(handler).toHaveBeenCalledTimes(1)
+    expect(handler).toHaveBeenCalledWith('a')
+  })
+
   it('destroys all events', () => {
     const handler = jest.fn()
     eventBus.on('destroyMe', handler)