Skip to content

fix: closing popups when removing interactive map filters - #2135

Open
NiclasNorin wants to merge 1 commit into
mainfrom
fix/closing-popups-when-removing-interactive-map-filters
Open

fix: closing popups when removing interactive map filters#2135
NiclasNorin wants to merge 1 commit into
mainfrom
fix/closing-popups-when-removing-interactive-map-filters

Conversation

@NiclasNorin

Copy link
Copy Markdown
Contributor

No description provided.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Fix: close map popups when removing InteractiveMap button filters

🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Close any open map popup when a button filter is deactivated.
• Prevent stale popups from persisting after filtered layers are removed.
Diagram

graph TD
  UI["Filter button"] --> F["LayerGroupWithButtonFilter"] --> MAP["Map instance"] --> POP["Popup overlay"]
  F --> LG["Layer group"]
  subgraph Legend
    direction LR
    _ui["UI"] ~~~ _comp["Component"] ~~~ _map["Map"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Close only popups tied to removed layers
  • ➕ Avoids closing unrelated popups that might remain meaningful across filters
  • ➕ More precise behavior if multiple overlay types exist
  • ➖ Requires tracking popup ownership/feature source, increasing implementation complexity
  • ➖ Higher regression risk vs. closing the active popup unconditionally
2. Centralize popup lifecycle in a map-level 'filter changed' handler
  • ➕ Single place to manage popup cleanup for all filter types
  • ➕ Reduces duplicated cleanup logic across filter components
  • ➖ Requires additional event wiring and conventions across filter implementations
  • ➖ May be overkill for a localized bug in one filter type

Recommendation: The PR’s approach (close the active popup as part of filter deactivation) is a pragmatic, low-risk UX fix. Consider a more targeted or centralized popup lifecycle only if future requirements involve multiple concurrent popups or filter types needing consistent cleanup.

Files changed (1) +1 / -0

Bug fix (1) +1 / -0
layerGroupWithButtonFilter.tsClose active popup when deactivating button-based layer filter +1/-0

Close active popup when deactivating button-based layer filter

• Adds a call to close the currently open map popup during filter deactivation (removeActive). This prevents popups from lingering after the associated layer group is removed from the map.

Modularity/source/php/Module/InteractiveMap/assets/map/filtering/layerGroupWithButtonFilter.ts

@sonarqubecloud

Copy link
Copy Markdown

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

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.

1 participant