Open
Conversation
Introduces a button to toggle the overlay's visibility. The overlay preference persists across sessions. Memoize MapOverlay to avoid unnecessary re-renders
TheYakuzo
changed the title
Refactors the map overlay component to improve code readability and maintainability. - Updates the overlay control to use static getDefaultPosition method. - Updates the overlay styling to use single quotes.
tananaev
reviewed
Apr 5, 2025
src/map/overlay/MapOverlayButton.js
Outdated
|
|
||
| const statusClass = (status) => `maplibregl-ctrl-icon maplibre-ctrl-overlay maplibre-ctrl-overlay-${status}`; | ||
|
|
||
| class OverlayControl { |
Member
There was a problem hiding this comment.
Is this basically a copy of the alarm button? I think we need to figure out how to reuse a button instead of copying.
Author
There was a problem hiding this comment.
Yes, it's true, I'll see tomorrow to redo these two buttons.
Do you usually put them in src/common/components?
Or maybe src/map and I create a new components folder in it
Member
There was a problem hiding this comment.
Map components should be under map folder.
Refactors the map notification and overlay buttons into a generic panel button component. This change improves code reusability and simplifies the map interface by using a consistent button style for different map controls.
Simplifies map control button styling by removing the redundant `maplibre-ctrl-background-icon` class. This change improves code readability and maintainability by consolidating the icon styling under a single `maplibre-ctrl-icon` class, making it easier to manage button appearance.
Member
|
Can we split into 2 PRs:
Also I think the icon shouldn't have a color. |
Author
|
Done : #1358 ( refactoring ) |
Updates the overlay icon colors to improve visual clarity. The "on" state icon is now black, and the "off" state icon is now gray.
Author
|
color is now black for enabled and gray for disabled |
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
2 participants
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.
Introduces a button to toggle the overlay's visibility. The overlay preference persists across sessions.
Memoize MapOverlay to avoid unnecessary re-renders
Move activeOverlay management from MapOverlay.js to MainMap.jsx to avoid code duplication on MapOverlay.js and MapOverlayButton.js [ new component ]
The button is not rendered when no map overlay is indicated (hence the displacement of the activeOverlay logic).