Bug/minor fixes

demo/js/index.js

@@ -34,8 +34,7 @@ var interactPlugin = createInteractPlugin({
 	}],
 	interactionMode: 'select', // 'auto', 'select', 'marker' // defaults to 'marker'
 	multiSelect: true,
-	contiguous: true,
-	// excludeModes: ['draw']
+	contiguous: true
 })
 
 var drawPlugin = createDrawPlugin({

plugins/interact/src/InteractInit.jsx

@@ -50,7 +50,7 @@ export const InteractInit = ({
 
   useEffect(() => {
     if (!pluginState.enabled) {
-      return
+      return undefined // Explicit return
     }
 
     const cleanupEvents = attachEvents({

plugins/interact/src/events.js

@@ -1,3 +1,16 @@
+// Helper for feature toggling logic
+const createFeatureHandler = (mapState, pluginState) => (args, addToExisting) => {
+  mapState.markers.remove('location')
+  pluginState.dispatch({
+    type: 'TOGGLE_SELECTED_FEATURES',
+    payload: {
+      multiSelect: pluginState.multiSelect,
+      addToExisting,
+      ...args
+    }
+  })
+}
+
 export function attachEvents ({
   appState,
   mapState,
@@ -8,41 +21,24 @@ export function attachEvents ({
   handleInteraction,
   closeApp
 }) {
-  const {
-    selectDone,
-    selectAtTarget,
-    selectCancel
-  } = buttonConfig
-
+  const { selectDone, selectAtTarget, selectCancel } = buttonConfig
   const { viewportRef } = appState.layoutRefs
 
+  // Keyboard Logic
   let enterOnViewport = false
-  const handleKeydown = (e) => {
-    enterOnViewport = e.key === 'Enter' && viewportRef.current === e.target
-  }
-  document.addEventListener('keydown', handleKeydown)
-
+  const handleKeydown = (e) => { enterOnViewport = e.key === 'Enter' && viewportRef.current === e.target }
   const handleKeyup = (e) => {
-    if (e.key !== 'Enter' || !enterOnViewport) {
-      return
+    if (e.key === 'Enter' && enterOnViewport) {
+      e.preventDefault()
+      handleSelectAtTarget()
     }
-    e.preventDefault()
-    handleSelectAtTarget()
   }
-  document.addEventListener('keyup', handleKeyup)
 
-  // Allow tapping on touch devices as well as accurate placement
-  const handleMapClick = (e) => {
-    handleInteraction(e)
-  }
-  eventBus.on(events.MAP_CLICK, handleMapClick)
+  // Interaction Handlers
+  const handleMapClick = (e) => handleInteraction(e)
+  const handleSelectAtTarget = () => handleInteraction(mapState.crossHair.getDetail())
 
-  const handleSelectAtTarget = () => {
-    handleInteraction(mapState.crossHair.getDetail())
-  }
-  selectAtTarget.onClick = handleSelectAtTarget
-
-  const handleSelectDone = (e) => {
+  const handleSelectDone = () => {
     const marker = mapState.markers.getMarker('location')
     const { coords } = marker || {}
     const { selectionBounds, selectedFeatures } = pluginState
@@ -57,43 +53,32 @@ export function attachEvents ({
       ...(!coords && selectionBounds && { selectionBounds })
     })
 
-    if (!(pluginState.closeOnAction ?? true)) {
-      return
+    if (pluginState.closeOnAction ?? true) {
+      closeApp()
     }
-
-    closeApp()
   }
-  selectDone.onClick = handleSelectDone
 
   const handleSelectCancel = () => {
     eventBus.emit('interact:cancel')
-
-    if (!(pluginState.closeOnAction ?? true)) {
-      return
+    if (pluginState.closeOnAction ?? true) {
+      closeApp()
     }
-
-    closeApp()
   }
-  selectCancel.onClick = handleSelectCancel
 
-  const handleToggleFeature = (args, addToExisting) => {
-    mapState.markers.remove('location')
+  const toggleFeature = createFeatureHandler(mapState, pluginState)
+  const handleSelect = (args) => toggleFeature(args, true)
+  const handleUnselect = (args) => toggleFeature(args, false)
 
-    pluginState.dispatch({
-      type: 'TOGGLE_SELECTED_FEATURES',
-      payload: {
-        multiSelect: pluginState.multiSelect,
-        addToExisting,
-        ...args
-      }
-    })
-  }
-  const handleSelect = (args) => handleToggleFeature(args, true)
-  const handleUnselect = (args) => handleToggleFeature(args, false)
+  // Attach Listeners
+  document.addEventListener('keydown', handleKeydown)
+  document.addEventListener('keyup', handleKeyup)
+  eventBus.on(events.MAP_CLICK, handleMapClick)
   eventBus.on('interact:selectFeature', handleSelect)
   eventBus.on('interact:unselectFeature', handleUnselect)
+  selectAtTarget.onClick = handleSelectAtTarget
+  selectDone.onClick = handleSelectDone
+  selectCancel.onClick = handleSelectCancel
 
-  // Return cleanup function
   return () => {
     selectDone.onClick = null
     selectAtTarget.onClick = null

plugins/interact/src/hooks/useHighlightSync.js

@@ -32,7 +32,7 @@ export const useHighlightSync = ({
 
   useEffect(() => {
     if (!mapProvider || !selectedFeatures || !stylesMap) {
-      return
+      return undefined // Explicit return to match the cleanup function return below
     }
 
     // Update updateHighlightedFeatures on interaction

plugins/interact/src/hooks/useInteractionHandlers.js

@@ -2,6 +2,34 @@ import { useCallback, useEffect, useRef } from 'react'
 import { isContiguousWithAny, canSplitFeatures, areAllContiguous } from '../utils/spatial.js'
 import { getFeaturesAtPoint, findMatchingFeature, buildLayerConfigMap } from '../utils/featureQueries.js'
 
+const useSelectionChangeEmitter = (eventBus, selectedFeatures, selectionBounds) => {
+  const lastEmittedSelectionChange = useRef(null)
+
+  useEffect(() => {
+    // Skip if features exist but bounds not yet calculated
+    const awaitingBounds = selectedFeatures.length > 0 && !selectionBounds
+    if (awaitingBounds) {
+      return
+    }
+
+    // Skip if selection was already empty and remains empty
+    const prev = lastEmittedSelectionChange.current
+    const wasEmpty = prev === null || prev.length === 0
+    if (wasEmpty && selectedFeatures.length === 0) {
+      return
+    }
+
+    eventBus.emit('interact:selectionchange', {
+      selectedFeatures,
+      selectionBounds,
+      canMerge: areAllContiguous(selectedFeatures),
+      canSplit: canSplitFeatures(selectedFeatures)
+    })
+
+    lastEmittedSelectionChange.current = selectedFeatures
+  }, [selectedFeatures, selectionBounds])
+}
+
 export const useInteractionHandlers = ({
   mapState,
   pluginState,
@@ -11,83 +39,73 @@ export const useInteractionHandlers = ({
   const { markers } = mapState
   const { dispatch, dataLayers, interactionMode, multiSelect, contiguous, markerColor, tolerance, selectedFeatures, selectionBounds } = pluginState
   const { eventBus } = services
-  const lastEmittedSelectionChange = useRef(null)
   const layerConfigMap = buildLayerConfigMap(dataLayers)
 
   const handleInteraction = useCallback(({ point, coords }) => {
     const allFeatures = getFeaturesAtPoint(mapProvider, point, { radius: tolerance })
     const hasDataLayers = dataLayers.length > 0
 
-    // Debug option to inspect the map style data
     if (pluginState?.debug) {
-      console.log(`--- Features at ${coords} ---`)
-      console.log(allFeatures)
+      console.log(`--- Features at ${coords} ---`, allFeatures)
     }
 
-    const canMatchFeature = hasDataLayers && (interactionMode === 'select' || interactionMode === 'auto')
-    const match = canMatchFeature ? findMatchingFeature(allFeatures, layerConfigMap) : null
+    const canMatch = hasDataLayers && (interactionMode === 'select' || interactionMode === 'auto')
+    const match = canMatch ? findMatchingFeature(allFeatures, layerConfigMap) : null
 
+    // 1. Handle Feature Match
     if (match) {
-      markers.remove('location')
-      const { feature, config } = match
-
-      const isNewFeatureContiguous = contiguous && isContiguousWithAny(feature, selectedFeatures)
-
-      const featureId = config.idProperty
-        ? feature.properties?.[config.idProperty]
-        : feature.id
-
-      if (featureId) {
-        dispatch({
-          type: 'TOGGLE_SELECTED_FEATURES',
-          payload: {
-            featureId,
-            multiSelect,
-            layerId: config.layerId,
-            idProperty: config.idProperty,
-            properties: feature.properties,
-            geometry: feature.geometry,
-            replaceAll: contiguous && !isNewFeatureContiguous
-          },
-        })
-      }
-
+      processFeatureMatch(match)
       return
     }
 
-    // Marker mode
-    if (interactionMode === 'marker' || (interactionMode === 'auto' && hasDataLayers)) {
+    // 2. Handle Marker Mode (Fallback)
+    const isMarkerMode = interactionMode === 'marker' || (interactionMode === 'auto' && hasDataLayers)
+    if (isMarkerMode) {
       dispatch({ type: 'CLEAR_SELECTED_FEATURES' })
       markers.add('location', coords, { color: markerColor })
-
       eventBus.emit('interact:markerchange', { coords })
     }
-  }, [mapProvider, dataLayers, interactionMode, multiSelect, eventBus, dispatch, markers])
-
-  // Emit event when selectedFeatures change
-  useEffect(() => {
-    // Skip if features exist but bounds not yet calculated
-    const awaitingBounds = selectedFeatures.length > 0 && !selectionBounds
-    if (awaitingBounds) {
-      return
-    }
 
-    // Skip if selection was already empty and remains empty
-    const prev = lastEmittedSelectionChange.current
-    const wasEmpty = prev === null || prev.length === 0
-    if (wasEmpty && selectedFeatures.length === 0) {
-      return
-    }
+    // Internal helper to keep complexity low
+    function processFeatureMatch({ feature, config }) {
+      markers.remove('location')
+      const isNewContiguous = contiguous && isContiguousWithAny(feature, selectedFeatures)
+      const featureId = feature.properties?.[config.idProperty] ?? feature.id
 
-    eventBus.emit('interact:selectionchange', {
-      selectedFeatures,
-      selectionBounds,
-      canMerge: areAllContiguous(selectedFeatures),
-      canSplit: canSplitFeatures(selectedFeatures)
-    })
+      if (!featureId) {
+        return
+      }
 
-    lastEmittedSelectionChange.current = selectedFeatures
-  }, [selectedFeatures, selectionBounds])
+      dispatch({
+        type: 'TOGGLE_SELECTED_FEATURES',
+        payload: {
+          featureId,
+          multiSelect,
+          layerId: config.layerId,
+          idProperty: config.idProperty,
+          properties: feature.properties,
+          geometry: feature.geometry,
+          replaceAll: contiguous && !isNewContiguous
+        }
+      })
+    }
+  }, [
+    mapProvider,
+    dataLayers,
+    interactionMode,
+    multiSelect,
+    eventBus,
+    dispatch,
+    markers,
+    contiguous,
+    selectedFeatures,
+    layerConfigMap,
+    pluginState?.debug,
+    tolerance,
+    markerColor
+  ])
+
+  useSelectionChangeEmitter(eventBus, selectedFeatures, selectionBounds)
 
   return {
     handleInteraction

plugins/interact/src/reducer.js

@@ -33,48 +33,40 @@ const disable = (state) => {
  */
 const toggleSelectedFeatures = (state, payload) => {
   const { featureId, multiSelect, layerId, idProperty, addToExisting = true, replaceAll = false, properties, geometry } = payload
-  const selected = Array.isArray(state.selectedFeatures) ? [...state.selectedFeatures] : []
-
-  const existingIndex = selected.findIndex(
+  const currentSelected = Array.isArray(state.selectedFeatures) ? state.selectedFeatures : []
+  
+  const existingIndex = currentSelected.findIndex(
     f => f.featureId === featureId && f.layerId === layerId
   )
 
-  // Handle explicit unselect
+  // 1. Handle explicit unselect
   if (addToExisting === false) {
-    if (existingIndex !== -1) {
-      selected.splice(existingIndex, 1) // remove the feature
-    }
-    return { ...state, selectedFeatures: selected, selectionBounds: null }
+    const filtered = currentSelected.filter((_, i) => i !== existingIndex)
+    return { ...state, selectedFeatures: filtered, selectionBounds: null }
   }
 
-  // Replace all selected features if flag is true
-  if (replaceAll) {
-    // Toggle off if clicking the same already-selected feature
-    if (existingIndex !== -1 && selected.length === 1) {
-      return { ...state, selectedFeatures: [], selectionBounds: null }
-    }
-    return {
-      ...state,
-      selectedFeatures: [{ featureId, layerId, idProperty, properties, geometry }],
-      selectionBounds: null
-    }
-  }
+  // Define the feature object once to avoid repetition
+  const featureObj = { featureId, layerId, idProperty, properties, geometry }
+  let nextSelected
 
-  // Multi-select mode (add to selection)
-  if (multiSelect) {
+  // 2. Determine New State 
+  // We combine 'replaceAll' and 'single-select' because they share the same logic
+  if (multiSelect && !replaceAll) {
+    const selectedCopy = [...currentSelected]
     if (existingIndex === -1) {
-      selected.push({ featureId, layerId, idProperty, properties, geometry })
+      selectedCopy.push(featureObj)
     } else {
-      selected.splice(existingIndex, 1)
+      selectedCopy.splice(existingIndex, 1)
     }
-    return { ...state, selectedFeatures: selected, selectionBounds: null }
+    nextSelected = selectedCopy
+  } else {
+    // Both 'replaceAll' and single-select mode logic:
+    // If same feature is already the only one, toggle off; otherwise return just this feature.
+    const isSameSingle = existingIndex !== -1 && currentSelected.length === 1
+    nextSelected = isSameSingle ? [] : [featureObj]
   }
 
-  // Single-select mode
-  const isSameSingle = existingIndex !== -1 && selected.length === 1
-  const newSelected = isSameSingle ? [] : [{ featureId, layerId, idProperty, properties, geometry }]
-
-  return { ...state, selectedFeatures: newSelected, selectionBounds: null }
+  return { ...state, selectedFeatures: nextSelected, selectionBounds: null }
 }
 
 // Update bounds (called from useEffect after map provider calculates them)