Replaces notification with panel button #1358
Conversation
Replaces the dedicated notification component with a more general-purpose panel button. This allows for more flexible integration of notification features.
Addresses minor formatting inconsistencies in the code. Enhances the MapPanelButton component for better control management and code readability.
| class NotificationControl { | ||
| constructor(eventHandler) { | ||
| class PanelButton { | ||
| constructor(eventHandler, iconClass) { |
There was a problem hiding this comment.
Should we just pass two icons instead of iconClass function?
There was a problem hiding this comment.
So, set the svg directly in component parameter ?
Or just set the name of the class and I build .maplibre-ctrl-[className]-on/off in component, sound better ( But we keep the css import in this case )
There was a problem hiding this comment.
Actually let's keep the function. I see that it's probably cleaner.
src/main/MainMap.jsx
Outdated
| <MapPanelButton | ||
| enabled={eventsAvailable} | ||
| > | ||
| iconClass={(status) => `maplibre-ctrl-notification maplibre-ctrl-notification-${status}`} |
There was a problem hiding this comment.
We don't need maplibre-ctrl-notification here, right?
There was a problem hiding this comment.
Yes you're right, don't know why I put it
|
I rework it, let me know if I push : // MapButton.js import {useEffect, useMemo, useCallback} from "react";
import {map} from "../core/MapView";
import "./map-button.css";
const VALID_TYPES = ["overlay", "notification"];
class Button {
constructor(eventHandler, type) {
if (type && !VALID_TYPES.includes(type)) {
console.warn(
`Invalid button type: ${type}. Valid types are: ${VALID_TYPES.join(
", "
)}`
);
}
this.eventHandler = eventHandler;
this.type = type;
}
static getDefaultPosition() {
return "top-right";
}
onAdd() {
this.button = document.createElement("button");
this.button.className = this.buildButtonClasses();
this.button.type = "button";
this.button.onclick = () => this.eventHandler(this);
this.container = document.createElement("div");
this.container.className = "maplibregl-ctrl maplibregl-ctrl-group";
this.container.appendChild(this.button);
return this.container;
}
onRemove() {
this.container.parentNode.removeChild(this.container);
}
setEnabled(enabled) {
this.button.className = this.buildButtonClasses(enabled);
}
buildButtonClasses(enabled = false) {
const classes = ["maplibre-ctrl-icon"];
if (this.type && VALID_TYPES.includes(this.type)) {
classes.push(`maplibre-ctrl-${this.type}-${enabled ? "on" : "off"}`);
}
return classes.join(" ");
}
}
const MapButton = ({enabled, onClick, type}) => {
const importCSS = useCallback(async (type) => {
if (!type || !VALID_TYPES.includes(type)) {
return;
}
try {
await import(`../${type}/${type}.css`);
} catch (error) {
console.error(`Failed to load CSS for ${type}:`, error);
}
}, []);
const control = useMemo(() => new Button(onClick, type), [onClick, type]);
useEffect(() => {
importCSS(type);
}, [type, importCSS]);
useEffect(() => {
map.addControl(control);
return () => map.removeControl(control);
}, [control]);
useEffect(() => {
control.setEnabled(enabled);
}, [enabled, control]);
return null;
};
export default MapButton;// MainMap.jsx <MapButton
enabled={overlayEnabled}
onClick={onOverlayButtonClick}
type="overlay"
/>
...
<MapButton
enabled={eventsAvailable}
onClick={onEventsClick}
type="notification"
/>CSS is dynamically imported into the component, but must respect the file architecture. So the two CSS files are no longer imported into MainMap.jsx. |
|
I don't think we need types validation, but overall I think it's fine to work like that. |
|
Actually I don't like the dynamic import. I think we should stick with the original |
Renames and refactors the map panel button to a more generic map button component. This change aims to improve code reusability and clarity by providing a single, configurable button component for various map controls, instead of having a specific button tailored for panels.
|
Will rebase and use this implementation on the other branch when merged |
Replaces the dedicated notification component with a more general-purpose panel button.
This allows for more flexible integration of notification features and other possible features in the future.