From d237a3a1c57d29df6848cabe54f6ffe192eafaff Mon Sep 17 00:00:00 2001 From: Havard Bjerke Date: Tue, 4 Apr 2023 09:23:42 +0200 Subject: [PATCH 1/2] Honor changing view controller type. When replacing existing Views with changed Controller types, if the new Views happen to have the same ids, then the new Controller types are not honored, and instead the old, possibly incompatible Controllers are used instead. --- modules/core/src/lib/view-manager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/core/src/lib/view-manager.ts b/modules/core/src/lib/view-manager.ts index f304ac74d7c..a8b57e12bef 100644 --- a/modules/core/src/lib/view-manager.ts +++ b/modules/core/src/lib/view-manager.ts @@ -326,8 +326,9 @@ export default class ViewManager { height: viewport.height }; - // TODO - check if view / controller type has changed, and replace the controller - if (!controller) { + // Create controller if not already existing or if the type of the + // controller has changed. + if (!controller || (controller.constructor !== view.ControllerType)) { controller = this._createController(view, resolvedProps); } if (controller) { From b90835d1988c829f216cc89956d451dc538c1477 Mon Sep 17 00:00:00 2001 From: Havard Bjerke Date: Wed, 5 Apr 2023 18:01:49 +0200 Subject: [PATCH 2/2] Added test to verify controller update. --- modules/core/src/lib/view-manager.ts | 2 +- test/modules/core/views/view-manager.spec.js | 32 +++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/modules/core/src/lib/view-manager.ts b/modules/core/src/lib/view-manager.ts index a8b57e12bef..cf43e12e7d0 100644 --- a/modules/core/src/lib/view-manager.ts +++ b/modules/core/src/lib/view-manager.ts @@ -328,7 +328,7 @@ export default class ViewManager { // Create controller if not already existing or if the type of the // controller has changed. - if (!controller || (controller.constructor !== view.ControllerType)) { + if (!controller || controller.constructor !== view.ControllerType) { controller = this._createController(view, resolvedProps); } if (controller) { diff --git a/test/modules/core/views/view-manager.spec.js b/test/modules/core/views/view-manager.spec.js index 46fff15b20d..869b73d8c94 100644 --- a/test/modules/core/views/view-manager.spec.js +++ b/test/modules/core/views/view-manager.spec.js @@ -1,6 +1,6 @@ import test from 'tape-promise/tape'; import ViewManager from '@deck.gl/core/lib/view-manager'; -import {MapView, Viewport} from 'deck.gl'; +import {OrbitController, OrbitView, MapController, MapView, Viewport} from 'deck.gl'; const INITIAL_VIEW_STATE = {latitude: 0, longitude: 0, zoom: 1}; @@ -75,3 +75,33 @@ test('ViewManager#needsRedraw', t => { t.end(); }); + +test('ViewManager#updateController', t => { + const viewManager = new ViewManager({}); + + const mapView = new MapView({id: 'test', height: '100%', controller: MapController}); + viewManager.setProps({ + views: [mapView], + viewState: INITIAL_VIEW_STATE, + width: 100, + height: 100 + }); + + const mapController = viewManager.controllers['test']; + t.equals(mapController.constructor, MapController, 'Correct controller type'); + + // Replace the MapView with a new OrbitView, given the same id. + const orbitView = new OrbitView({id: 'test', height: '100%', controller: OrbitController}); + viewManager.setProps({ + views: [orbitView], + viewState: INITIAL_VIEW_STATE, + width: 100, + height: 100 + }); + + // Verify that the new view has the correct controller. + const orbitController = viewManager.controllers['test']; + t.equals(orbitController.constructor, OrbitController, 'Correct controller type'); + + t.end(); +});