-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(mapbox): Add widget support to MapboxOverlay via IControl adapter #9963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
90ca74b
c042e51
0d22779
6ac76e3
b3e8bd6
1bc5480
e793c90
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| // deck.gl | ||
| // SPDX-License-Identifier: MIT | ||
| // Copyright (c) vis.gl contributors | ||
|
|
||
| import type {Widget} from '@deck.gl/core'; | ||
| import type {IControl, ControlPosition, Map} from './types'; | ||
|
|
||
| /** | ||
| * Wraps a deck.gl Widget as a Mapbox/MapLibre IControl. | ||
| * | ||
| * This enables deck widgets to be positioned alongside native map controls | ||
| * in the same DOM container, preventing overlap issues. | ||
| * | ||
| * @internal Used by MapboxOverlay for widgets with `viewId: 'mapbox'`. | ||
| */ | ||
| export class DeckWidgetControl implements IControl { | ||
| private _widget: Widget; | ||
| private _container: HTMLDivElement | null = null; | ||
|
|
||
| constructor(widget: Widget) { | ||
| this._widget = widget; | ||
| } | ||
|
|
||
| /** | ||
| * Called when the control is added to the map. | ||
| * Creates a container element that will be positioned by Mapbox/MapLibre, | ||
| * and sets the widget's _container prop so WidgetManager appends the widget here. | ||
| */ | ||
| onAdd(map: Map): HTMLElement { | ||
| this._container = document.createElement('div'); | ||
| this._container.className = 'maplibregl-ctrl mapboxgl-ctrl deck-widget-ctrl'; | ||
|
|
||
| // Set _container so WidgetManager appends the widget's rootElement here | ||
| // instead of in its own overlay container | ||
| this._widget.props._container = this._container; | ||
|
|
||
| return this._container; | ||
| } | ||
|
|
||
| /** | ||
| * Called when the control is removed from the map. | ||
| */ | ||
| onRemove(): void { | ||
| // Clear the _container reference so widget doesn't try to append there | ||
| if (this._widget.props._container === this._container) { | ||
| this._widget.props._container = null; | ||
| } | ||
| this._container?.remove(); | ||
| this._container = null; | ||
| } | ||
|
|
||
| /** | ||
| * Returns the default position for this control. | ||
| * Uses the widget's placement, which conveniently matches Mapbox control positions. | ||
| * Note: 'fill' placement is not supported by Mapbox controls, defaults to 'top-left'. | ||
| */ | ||
| getDefaultPosition(): ControlPosition { | ||
| const placement = this._widget.placement; | ||
| // 'fill' is not a valid Mapbox control position | ||
| if (!placement || placement === 'fill') { | ||
| return 'top-left'; | ||
| } | ||
| return placement; | ||
| } | ||
|
|
||
| /** Returns the wrapped widget */ | ||
| get widget(): Widget { | ||
| return this._widget; | ||
| } | ||
|
|
||
| /** | ||
| * Updates the wrapped widget reference. | ||
| * Used when reusing this control for a new widget instance with the same id. | ||
| */ | ||
| setWidget(widget: Widget): void { | ||
| this._widget = widget; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,10 +11,11 @@ import { | |
| getDefaultParameters, | ||
| getProjection | ||
| } from './deck-utils'; | ||
| import {DeckWidgetControl} from './deck-widget-control'; | ||
|
|
||
| import type {Map, IControl, MapMouseEvent, ControlPosition} from './types'; | ||
| import type {MjolnirGestureEvent, MjolnirPointerEvent} from 'mjolnir.js'; | ||
| import type {DeckProps, LayersList} from '@deck.gl/core'; | ||
| import type {DeckProps, LayersList, Widget} from '@deck.gl/core'; | ||
|
|
||
| import {resolveLayers} from './resolve-layers'; | ||
| import {resolveLayerGroups} from './resolve-layer-groups'; | ||
|
|
@@ -55,6 +56,8 @@ export default class MapboxOverlay implements IControl { | |
| private _interleaved: boolean; | ||
| private _renderLayersInGroups: boolean; | ||
| private _lastMouseDownPoint?: {x: number; y: number; clientX: number; clientY: number}; | ||
| /** IControl wrappers for widgets with viewId: 'mapbox' */ | ||
| private _widgetControls: DeckWidgetControl[] = []; | ||
|
|
||
| constructor(props: MapboxOverlayProps) { | ||
| const {interleaved = false} = props; | ||
|
|
@@ -79,6 +82,12 @@ export default class MapboxOverlay implements IControl { | |
| this._resolveLayers(this._map, this._deck, this._props.layers, props.layers); | ||
| } | ||
|
|
||
| // Process widgets with viewId: 'mapbox' before updating props | ||
| // This must happen before deck.setProps so _container is set | ||
| if (props.widgets !== undefined) { | ||
| this._processWidgets(props.widgets); | ||
| } | ||
|
|
||
| Object.assign(this._props, this.filterProps(props)); | ||
|
|
||
| if (this._deck && this._map) { | ||
|
|
@@ -112,6 +121,10 @@ export default class MapboxOverlay implements IControl { | |
| }); | ||
| this._container = container; | ||
|
|
||
| // Process widgets with viewId: 'mapbox' BEFORE creating Deck | ||
| // so _container is set when WidgetManager initializes | ||
| this._processWidgets(this._props.widgets); | ||
|
|
||
| this._deck = new Deck<any>({ | ||
| ...this._props, | ||
| parent: container, | ||
|
|
@@ -143,6 +156,11 @@ export default class MapboxOverlay implements IControl { | |
| 'Incompatible basemap library. See: https://deck.gl/docs/api-reference/mapbox/overview#compatibility' | ||
| )(); | ||
| } | ||
|
|
||
| // Process widgets with viewId: 'mapbox' BEFORE creating Deck | ||
| // so _container is set when WidgetManager initializes | ||
| this._processWidgets(this._props.widgets); | ||
|
|
||
| this._deck = getDeckInstance({ | ||
| map, | ||
| deck: new Deck({ | ||
|
|
@@ -171,11 +189,73 @@ export default class MapboxOverlay implements IControl { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Process widgets and wrap those with viewId: 'mapbox' as IControls. | ||
| * This enables deck widgets to be positioned in Mapbox's control container | ||
| * alongside native map controls, preventing overlap. | ||
| * | ||
| * Matches widgets by id (like WidgetManager) to handle new instances with same id. | ||
| * Only recreates controls when placement changes to avoid orphaning the widget's | ||
| * rootElement when the container is removed from the DOM. | ||
| */ | ||
| private _processWidgets(widgets: Widget[] | undefined): void { | ||
| const map = this._map; | ||
| if (!map) return; | ||
|
|
||
| const mapboxWidgets = widgets?.filter(w => w && w.viewId === 'mapbox') ?? []; | ||
|
|
||
| // Build a map of existing controls by widget id | ||
| const existingControlsById = new Map<string, DeckWidgetControl>(); | ||
| for (const control of this._widgetControls) { | ||
| existingControlsById.set(control.widget.id, control); | ||
| } | ||
|
|
||
| const newControls: DeckWidgetControl[] = []; | ||
|
|
||
| for (const widget of mapboxWidgets) { | ||
| const existingControl = existingControlsById.get(widget.id); | ||
|
|
||
| if (existingControl && existingControl.widget.placement === widget.placement) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Placement change undetected for mutated widget objectsLow Severity The placement comparison |
||
| // Same id and placement - reuse existing control to preserve container | ||
| // Set _container on the new widget instance so WidgetManager uses it | ||
| widget.props._container = existingControl.widget.props._container; | ||
| // Update the control's widget reference to the new instance | ||
| existingControl.setWidget(widget); | ||
| newControls.push(existingControl); | ||
| existingControlsById.delete(widget.id); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } else { | ||
| // New widget or placement changed - need a new control | ||
| if (existingControl) { | ||
| // Placement changed - remove old control first | ||
| map.removeControl(existingControl); | ||
| existingControlsById.delete(widget.id); | ||
| } | ||
| const control = new DeckWidgetControl(widget); | ||
| // Add to map - this calls onAdd() synchronously, setting _container | ||
| map.addControl(control, control.getDefaultPosition()); | ||
| newControls.push(control); | ||
| } | ||
| } | ||
|
|
||
| // Remove controls for widgets that are no longer present | ||
| for (const control of existingControlsById.values()) { | ||
| map.removeControl(control); | ||
| } | ||
|
|
||
| this._widgetControls = newControls; | ||
| } | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** Called when the control is removed from a map */ | ||
| onRemove(): void { | ||
| const map = this._map; | ||
|
|
||
| if (map) { | ||
| // Remove widget controls | ||
| for (const control of this._widgetControls) { | ||
| map.removeControl(control); | ||
| } | ||
| this._widgetControls = []; | ||
|
|
||
| if (this._interleaved) { | ||
| this._onRemoveInterleaved(map); | ||
| } else { | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onRemove clears _container on wrong widget instance
Low Severity
After successive
setPropscalls with new widget instances of the same id,DeckWidgetControl._widgetreferences the latest new instance (viasetWidget), whileWidgetManagertracks the original old instance (viaoldWidget.setProps(widget.props)). WhenDeckWidgetControl.onRemoveruns, it clears_containeron the control's widget (the wrong instance), leaving a stale_containerreference pointing to a detached DOM element on the widget thatWidgetManageractually manages.Additional Locations (1)
modules/mapbox/src/mapbox-overlay.ts#L217-L225