Skip to content
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

Add widgets prop to Deck class #8023

Merged
merged 8 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions modules/core/src/lib/deck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import MapView from '../views/map-view';
import EffectManager from './effect-manager';
import DeckRenderer from './deck-renderer';
import DeckPicker from './deck-picker';
import {WidgetManager, Widget, WidgetPlacement} from './widget-manager';
import {WidgetManager, Widget} from './widget-manager';
import Tooltip from './tooltip';
import log from '../utils/log';
import {deepEqual} from '../utils/deep-equal';
Expand Down Expand Up @@ -181,6 +181,8 @@ export type DeckProps = {
_pickable?: boolean;
/** (Experimental) Fine-tune attribute memory usage. See documentation for details. */
_typedArrayManagerProps?: TypedArrayManagerOptions;
/** An array of Widget instances to be added to the parent element. */
widgets?: Widget[];
chrisgervang marked this conversation as resolved.
Show resolved Hide resolved

/** Called once the GPU Device has been initiated. */
onDeviceInitialized?: (device: Device) => void;
Expand Down Expand Up @@ -258,6 +260,7 @@ const defaultProps = {
_pickable: true,
_typedArrayManagerProps: {},
_customRender: null,
widgets: [],

onDeviceInitialized: noop,
onWebGLInitialized: noop,
Expand Down Expand Up @@ -485,6 +488,7 @@ export default class Deck {
this.effectManager.setProps(resolvedProps);
this.deckRenderer.setProps(resolvedProps);
this.deckPicker.setProps(resolvedProps);
this.widgetManager.setProps(resolvedProps);
}

this.stats.get('setProps Time').timeEnd();
Expand Down Expand Up @@ -988,7 +992,7 @@ export default class Deck {
deck: this,
parentElement: this.canvas?.parentElement
});
this.widgetManager.add(new Tooltip(), {placement: 'fill'});
this.widgetManager.addDefault(new Tooltip());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Is there any way to avoid addDefault()? Such "global" registration is usually best avoided if possible. Can the tooltip be added when it is actually used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd want to consider if changing it could break something since the 8.9 and prior always adds a tooltip to the container.


this.setProps(this.props);

Expand Down
7 changes: 6 additions & 1 deletion modules/core/src/lib/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
import {Widget} from './widget-manager';
import type {Widget, WidgetPlacement} from './widget-manager';
import type {PickingInfo} from './picking/pick-info';
import type Viewport from '../viewports/viewport';
import type Deck from './deck';
Expand Down Expand Up @@ -46,6 +46,9 @@
};

export default class Tooltip implements Widget {
id = 'default-tooltip';
placement: WidgetPlacement = 'fill';
props = {};
isVisible: boolean = false;
deck?: Deck;
element?: HTMLDivElement;
Expand All @@ -67,6 +70,8 @@
this.element = undefined;
}

setProps() {}

Check warning on line 73 in modules/core/src/lib/tooltip.ts

View workflow job for this annotation

GitHub Actions / test-node

Unexpected empty method 'setProps'

onViewportChange(viewport: Viewport) {
if (this.isVisible && viewport.id === this.lastViewport?.id && viewport !== this.lastViewport) {
// Camera has moved, clear tooltip
Expand Down
132 changes: 98 additions & 34 deletions modules/core/src/lib/widget-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
import type Layer from './layer';

import {EVENTS} from './constants';
import {deepEqual} from '../utils/deep-equal';

export interface Widget<PropsT = any> {
id: string;
chrisgervang marked this conversation as resolved.
Show resolved Hide resolved
props: PropsT;
viewId?: string | null;
placement?: WidgetPlacement;

// Populated by core when mounted
_element?: HTMLDivElement | null;
_viewId?: string | null;

// Lifecycle hooks
/** Called when the widget is added to a Deck instance.
Expand All @@ -23,7 +28,7 @@
/** Called when the widget is removed */
onRemove: () => void;
/** Called to update widget options */
setProps?: (props: Partial<PropsT>) => void;
setProps: (props: Partial<PropsT>) => void;

// Optional event hooks
/** Called when the containing view is changed */
Expand All @@ -49,6 +54,7 @@
'bottom-right': {bottom: 0, right: 0},
fill: {top: 0, left: 0, bottom: 0, right: 0}
} as const;
const DEFAULT_PLACEMENT = 'top-left';

export type WidgetPlacement = keyof typeof PLACEMENTS;

Expand All @@ -57,61 +63,119 @@
export class WidgetManager {
deck: Deck;
parentElement?: HTMLElement | null;
containers: {[id: string]: HTMLDivElement} = {};
widgets: Widget[] = [];
lastViewports: {[id: string]: Viewport} = {};

/** Widgets added via the imperative API */
private defaultWidgets: Widget[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the names could be more reflective of the comments - imperativeWidgets / declarativeWidgets / allWidgets etc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes maybe.. there's ongoing discussion in a new RFC on what our imperative API could be.

/** Widgets received from the declarative API */
private widgets: Widget[] = [];
/** Resolved widgets from both imperative and declarative APIs */
private resolvedWidgets: Widget[] = [];
private containers: {[id: string]: HTMLDivElement} = {};
private lastViewports: {[id: string]: Viewport} = {};
chrisgervang marked this conversation as resolved.
Show resolved Hide resolved

constructor({deck, parentElement}: {deck: Deck; parentElement?: HTMLElement | null}) {
this.deck = deck;
this.parentElement = parentElement;
}

getWidgets(): Widget[] {
return this.resolvedWidgets;
}

/** Declarative API to configure widgets */
setProps(props: {widgets?: Widget[]}) {
if (props.widgets && !deepEqual(props.widgets, this.widgets, 1)) {
this._setWidgets(props.widgets);
}
}

finalize() {
for (const widget of this.widgets) {
this.remove(widget);
for (const widget of this.getWidgets()) {
this._remove(widget);
Pessimistress marked this conversation as resolved.
Show resolved Hide resolved
}
this.defaultWidgets.length = 0;
this.resolvedWidgets.length = 0;
for (const id in this.containers) {
this.containers[id].remove();
}
}

add(
widget: Widget,
opts: {
viewId?: string | null;
placement?: WidgetPlacement;
} = {}
) {
if (this.widgets.includes(widget)) {
// widget already added
return;
/** Imperative API. Widgets added this way are not affected by the declarative prop. */
addDefault(widget: Widget) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to suggest a different name, but I guess this was renamed to addDefault based on previous comments.

if (!this.defaultWidgets.find(w => w.id === widget.id)) {
this._add(widget);
this.defaultWidgets.push(widget);
// Update widget list
this._setWidgets(this.widgets);
}
}

/** Resolve widgets from the declarative prop */
private _setWidgets(nextWidgets: Widget[]) {
const oldWidgetMap: Record<string, Widget | null> = {};

for (const widget of this.resolvedWidgets) {
oldWidgetMap[widget.id] = widget;
}
// Clear and rebuild the list
this.resolvedWidgets.length = 0;

const {placement = 'top-left', viewId = null} = opts;
// Add all default widgets
for (const widget of this.defaultWidgets) {
oldWidgetMap[widget.id] = null;
this.resolvedWidgets.push(widget);
}

for (let widget of nextWidgets) {
const oldWidget = oldWidgetMap[widget.id];
if (!oldWidget) {
// Widget is new
this._add(widget);
} else if (
// Widget placement changed
oldWidget.viewId !== widget.viewId ||
oldWidget.placement !== widget.placement
) {
this._remove(oldWidget);
this._add(widget);
} else if (widget !== oldWidget) {
// Widget props changed
oldWidget.setProps(widget.props);
widget = oldWidget;
}

// mark as matched
oldWidgetMap[widget.id] = null;
this.resolvedWidgets.push(widget);
}

for (const id in oldWidgetMap) {
const oldWidget = oldWidgetMap[id];
if (oldWidget) {
// No longer exists
this._remove(oldWidget);
}
}
this.widgets = nextWidgets;
}

private _add(widget: Widget) {
const {viewId = null, placement = DEFAULT_PLACEMENT} = widget;
const element = widget.onAdd({deck: this.deck, viewId});

if (element) {
this._getContainer(viewId, placement).append(element);
}
widget._viewId = viewId;
widget._element = element;
this.widgets.push(widget);
}

remove(widget: Widget) {
const i = this.widgets.indexOf(widget);
if (i < 0) {
// widget not found
return;
}
this.widgets.splice(i, 1);
private _remove(widget: Widget) {
widget.onRemove();

if (widget._element) {
widget._element.remove();
}
widget._element = undefined;
widget._viewId = undefined;
}

/* global document */
Expand Down Expand Up @@ -165,8 +229,8 @@
}, {});
const {lastViewports} = this;

for (const widget of this.widgets) {
const viewId = widget._viewId;
for (const widget of this.getWidgets()) {
const {viewId} = widget;
if (viewId) {
// Attached to a specific view
const viewport = viewportsById[viewId];
Expand All @@ -180,7 +244,7 @@
// Not attached to a specific view
if (widget.onViewportChange) {
for (const viewport of viewports) {
if (!viewport.equals(lastViewports[viewport.id])) {

Check warning on line 247 in modules/core/src/lib/widget-manager.ts

View workflow job for this annotation

GitHub Actions / test-node

Blocks are nested too deeply (5). Maximum allowed is 4
widget.onViewportChange(viewport);
}
}
Expand All @@ -193,8 +257,8 @@
}

onHover(info: PickingInfo, event: MjolnirPointerEvent) {
for (const widget of this.widgets) {
const viewId = widget._viewId;
for (const widget of this.getWidgets()) {
const {viewId} = widget;
if (!viewId || viewId === info.viewport?.id) {
widget.onHover?.(info, event);
}
Expand All @@ -206,8 +270,8 @@
if (!eventOptions) {
return;
}
for (const widget of this.widgets) {
const viewId = widget._viewId;
for (const widget of this.getWidgets()) {
const {viewId} = widget;
if (!viewId || viewId === info.viewport?.id) {
widget[eventOptions.handler]?.(info, event);
}
Expand Down
5 changes: 2 additions & 3 deletions test/modules/core/lib/tooltip.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function setupTest() {
const container = document.createElement('div');
const widgetManager = new WidgetManager({parentElement: container});
const tooltip = new Tooltip();
widgetManager.add(tooltip);
widgetManager.addDefault(tooltip);
return {tooltip, widgetManager, container};
}

Expand Down Expand Up @@ -90,13 +90,12 @@ test('Tooltip#remove', t => {
const {widgetManager, tooltip, container} = setupTest();

t.equals(container.querySelectorAll('.deck-tooltip').length, 1, 'Tooltip element present');
widgetManager.remove(tooltip);
widgetManager.finalize();
t.equals(
container.querySelectorAll('.deck-tooltip').length,
0,
'Tooltip element successfully removed'
);

widgetManager.finalize();
t.end();
});
Loading
Loading