Skip to content

Commit 9426c8c

Browse files
chrisgervangclaude
andcommitted
fix(mapbox): Preserve widget container on setProps to prevent orphaned rootElement
When setProps was called with widgets, _processWidgets would destroy and recreate all DeckWidgetControls. This removed the container from the DOM, orphaning the widget's rootElement since WidgetManager wouldn't re-attach it (same widget id means no change detected). Now matches widgets by id (like WidgetManager) and only recreates controls when the widget is new or placement changes. For existing widgets with same id and placement, the container is preserved and copied to the new widget instance to support React patterns where widgets are recreated on render. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
1 parent d9c3679 commit 9426c8c

File tree

2 files changed

+99
-9
lines changed

2 files changed

+99
-9
lines changed

modules/mapbox/src/mapbox-overlay.ts

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -193,29 +193,54 @@ export default class MapboxOverlay implements IControl {
193193
* Process widgets and wrap those with viewId: 'mapbox' as IControls.
194194
* This enables deck widgets to be positioned in Mapbox's control container
195195
* alongside native map controls, preventing overlap.
196+
*
197+
* Matches widgets by id (like WidgetManager) to handle new instances with same id.
198+
* Only recreates controls when placement changes to avoid orphaning the widget's
199+
* rootElement when the container is removed from the DOM.
196200
*/
197201
private _processWidgets(widgets: Widget[] | undefined): void {
198202
const map = this._map;
199203
if (!map) return;
200204

201-
// Remove old widget controls
205+
const mapboxWidgets = widgets?.filter(w => w.viewId === 'mapbox') ?? [];
206+
207+
// Build a map of existing controls by widget id
208+
const existingControlsById = new Map<string, DeckWidgetControl>();
202209
for (const control of this._widgetControls) {
203-
map.removeControl(control);
210+
existingControlsById.set(control.widget.id, control);
204211
}
205-
this._widgetControls = [];
206212

207-
if (!widgets) return;
213+
const newControls: DeckWidgetControl[] = [];
214+
215+
for (const widget of mapboxWidgets) {
216+
const existingControl = existingControlsById.get(widget.id);
208217

209-
// Wrap widgets with viewId: 'mapbox' as IControls
210-
for (const widget of widgets) {
211-
if (widget.viewId === 'mapbox') {
218+
if (existingControl && existingControl.widget.placement === widget.placement) {
219+
// Same id and placement - reuse existing control to preserve container
220+
// Set _container on the new widget instance so WidgetManager uses it
221+
widget.props._container = existingControl.widget.props._container;
222+
newControls.push(existingControl);
223+
existingControlsById.delete(widget.id);
224+
} else {
225+
// New widget or placement changed - need a new control
226+
if (existingControl) {
227+
// Placement changed - remove old control first
228+
map.removeControl(existingControl);
229+
existingControlsById.delete(widget.id);
230+
}
212231
const control = new DeckWidgetControl(widget);
213232
// Add to map - this calls onAdd() synchronously, setting _container
214-
// Use control.getDefaultPosition() which handles 'fill' -> 'top-left' conversion
215233
map.addControl(control, control.getDefaultPosition());
216-
this._widgetControls.push(control);
234+
newControls.push(control);
217235
}
218236
}
237+
238+
// Remove controls for widgets that are no longer present
239+
for (const control of existingControlsById.values()) {
240+
map.removeControl(control);
241+
}
242+
243+
this._widgetControls = newControls;
219244
}
220245

221246
/** Called when the control is removed from a map */

test/modules/mapbox/mapbox-overlay.spec.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,71 @@ test('MapboxOverlay#widgets - setProps updates widget controls', t => {
629629
t.end();
630630
});
631631

632+
test('MapboxOverlay#widgets - setProps preserves container for same widget instance', t => {
633+
const map = new MockMapboxMap({
634+
center: {lng: -122.45, lat: 37.78},
635+
zoom: 14
636+
});
637+
638+
const widget = new TestWidget({id: 'widget1', viewId: 'mapbox', placement: 'top-right'});
639+
const overlay = new MapboxOverlay({
640+
device: overlaidTestDevice,
641+
layers: [new ScatterplotLayer()],
642+
widgets: [widget]
643+
});
644+
645+
map.addControl(overlay);
646+
t.is(overlay._widgetControls.length, 1, 'Widget control created');
647+
const originalContainer = widget.props._container;
648+
t.ok(originalContainer, 'Widget _container is set');
649+
const originalControl = overlay._widgetControls[0];
650+
651+
// Call setProps with the same widget instance
652+
overlay.setProps({
653+
widgets: [widget]
654+
});
655+
656+
t.is(overlay._widgetControls.length, 1, 'Still one widget control');
657+
t.is(overlay._widgetControls[0], originalControl, 'Same control instance preserved');
658+
t.is(widget.props._container, originalContainer, 'Container preserved - not recreated');
659+
660+
map.removeControl(overlay);
661+
t.end();
662+
});
663+
664+
test('MapboxOverlay#widgets - setProps preserves container for new widget instance with same id', t => {
665+
const map = new MockMapboxMap({
666+
center: {lng: -122.45, lat: 37.78},
667+
zoom: 14
668+
});
669+
670+
const widget1 = new TestWidget({id: 'my-widget', viewId: 'mapbox', placement: 'top-right'});
671+
const overlay = new MapboxOverlay({
672+
device: overlaidTestDevice,
673+
layers: [new ScatterplotLayer()],
674+
widgets: [widget1]
675+
});
676+
677+
map.addControl(overlay);
678+
t.is(overlay._widgetControls.length, 1, 'Widget control created');
679+
const originalContainer = widget1.props._container;
680+
t.ok(originalContainer, 'Widget _container is set');
681+
const originalControl = overlay._widgetControls[0];
682+
683+
// Call setProps with a NEW widget instance but same id and placement (React pattern)
684+
const widget2 = new TestWidget({id: 'my-widget', viewId: 'mapbox', placement: 'top-right'});
685+
overlay.setProps({
686+
widgets: [widget2]
687+
});
688+
689+
t.is(overlay._widgetControls.length, 1, 'Still one widget control');
690+
t.is(overlay._widgetControls[0], originalControl, 'Same control instance preserved');
691+
t.is(widget2.props._container, originalContainer, 'New widget gets existing container');
692+
693+
map.removeControl(overlay);
694+
t.end();
695+
});
696+
632697
test('MapboxOverlay#widgets - interleaved mode', t => {
633698
const map = new MockMapboxMap({
634699
center: {lng: -122.45, lat: 37.78},

0 commit comments

Comments
 (0)