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

feat(react) Support for JSX Widgets in React #9278

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
9bbdc79
Export widget prop types
chrisgervang Oct 25, 2024
e55ab6f
Construct new props instead of mutating them
chrisgervang Oct 25, 2024
9803538
Add DeckGLContext
chrisgervang Oct 25, 2024
c526999
Add useWidget hook
chrisgervang Oct 25, 2024
51154e8
Create react components for each widget
chrisgervang Oct 25, 2024
e6a7a5f
Add widget to basic example
chrisgervang Oct 25, 2024
5549372
Fix bootstrap
chrisgervang Oct 25, 2024
b8aaef2
Merge branch 'master' into chr/jsx-widgets
chrisgervang Nov 22, 2024
5508ccd
Merge branch 'master' into chr/jsx-widgets
chrisgervang Dec 5, 2024
3ed7e27
Merge branch 'master' into chr/jsx-widgets
chrisgervang Dec 12, 2024
214f5e6
Deck should be defined in context for widgets
chrisgervang Dec 13, 2024
74c241b
WIP React widgets should warn when vanilla widgets are used
chrisgervang Dec 13, 2024
a0d0b6e
Add widget prop
chrisgervang Dec 13, 2024
26a95a1
Export useWidget
chrisgervang Dec 13, 2024
271561f
Demo react widget
chrisgervang Dec 13, 2024
1e62c65
Widgets should be removed when JSX unmounts
chrisgervang Dec 13, 2024
ec8d18f
Fix widget warning
chrisgervang Dec 13, 2024
7cd5d58
Demo toggleable zoom widget in react
chrisgervang Dec 13, 2024
b7a79ec
lint
chrisgervang Dec 13, 2024
82287da
clearer warning message
chrisgervang Dec 17, 2024
971da0f
Merge branch 'master' into chr/jsx-widgets
chrisgervang Dec 17, 2024
09b577a
Fix build
chrisgervang Dec 17, 2024
7e4c4fb
viewports possibly undefined
chrisgervang Dec 17, 2024
1e5545f
Merge branch 'master' into chr/jsx-widgets
chrisgervang Dec 19, 2024
91c0930
Merge branch 'master' into chr/jsx-widgets
chrisgervang Dec 23, 2024
ffe8362
basic jsx example
chrisgervang Dec 24, 2024
23a3210
lint
chrisgervang Dec 24, 2024
5b995d6
chore(react) all children are provided a DeckContext by default
chrisgervang Dec 31, 2024
3cfd798
chore(react) widgets should be reset when omitted in react
chrisgervang Dec 31, 2024
7dbf2cd
chore(react) widget prop warning shouldn't warn for empty array
chrisgervang Jan 4, 2025
8c822b4
chore(react) rename react widgets
chrisgervang Jan 4, 2025
d17b012
chore(main) export react components
chrisgervang Jan 4, 2025
aa9ae7b
chore(react) use consistent naming between react and purejs widgets
chrisgervang Jan 4, 2025
f144266
Merge branch 'master' into chr/jsx-widgets
chrisgervang Jan 4, 2025
323c020
Merge branch 'master' into chr/jsx-widgets
chrisgervang Jan 6, 2025
43168f2
Merge branch 'master' into chr/jsx-widgets
chrisgervang Jan 7, 2025
1ff736c
use named export
chrisgervang Jan 8, 2025
ea0cf0c
Should use @deck.gl/react for all widget imports
chrisgervang Jan 8, 2025
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
3 changes: 3 additions & 0 deletions examples/get-started/react/basic/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import React from 'react';
import {createRoot} from 'react-dom/client';
import DeckGL, {GeoJsonLayer, ArcLayer} from 'deck.gl';
import {CompassWidget} from '@deck.gl/react';
import '@deck.gl/widgets/stylesheet.css';

// source: Natural Earth http://www.naturalearthdata.com/ via geojson.xyz
const COUNTRIES =
Expand Down Expand Up @@ -62,6 +64,7 @@ function Root() {
getTargetColor={[200, 0, 80]}
getWidth={1}
/>
<CompassWidget />
</DeckGL>
);
}
Expand Down
2 changes: 1 addition & 1 deletion modules/main/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export {ScenegraphLayer, SimpleMeshLayer} from '@deck.gl/mesh-layers';
// REACT BINDINGS PACKAGE
//

export {default, DeckGL} from '@deck.gl/react';
export {default, DeckGL, useWidget} from '@deck.gl/react';

//
// WIDGETS PACKAGE
Expand Down
1 change: 1 addition & 0 deletions modules/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"scripts": {},
"peerDependencies": {
"@deck.gl/core": "^9.1.0-beta",
"@deck.gl/widgets": "^9.1.0-beta",
"react": ">=16.3.0",
"react-dom": ">=16.3.0"
},
Expand Down
3 changes: 2 additions & 1 deletion modules/react/src/deckgl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import extractJSXLayers, {DeckGLRenderCallback} from './utils/extract-jsx-layers
import positionChildrenUnderViews from './utils/position-children-under-views';
import extractStyles from './utils/extract-styles';

import type {DeckGLContextValue} from './utils/position-children-under-views';
import type {DeckGLContextValue} from './utils/deckgl-context';
import type {DeckProps, View, Viewport} from '@deck.gl/core';

export type ViewOrViews = View | View[] | null;
Expand Down Expand Up @@ -157,6 +157,7 @@ function DeckGLWithRef<ViewsT extends ViewOrViews = null>(
// Needs to be called both from initial mount, and when new props are received
const deckProps = useMemo(() => {
const forwardProps: DeckProps<ViewsT> = {
widgets: [],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Similar to jsxProps.layers, the widgets array needs to reset to an empty array in the react component so that widgets unmount when the prop is removed.

This sets the behavior when a user goes from <DeckGL widget={[...]}/> to <DeckGL/>.

I believe unmounting in this case is the desired behavior. This behavior was noticeable in the deck.gl playground (which uses the react deck component) when removing the entire widgets section from the json string didn't remove them from the map.

...props,
// Override user styling props. We will set the canvas style in render()
style: null,
Expand Down
8 changes: 7 additions & 1 deletion modules/react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@
export {default as DeckGL} from './deckgl';
export {default} from './deckgl';

// Widgets
export {CompassWidget} from './widgets/compass-widget';
felixpalmer marked this conversation as resolved.
Show resolved Hide resolved
export {FullscreenWidget} from './widgets/fullscreen-widget';
export {ZoomWidget} from './widgets/zoom-widget';
chrisgervang marked this conversation as resolved.
Show resolved Hide resolved
export {default as useWidget} from './utils/use-widget';

// Types
export type {DeckGLContextValue} from './utils/position-children-under-views';
export type {DeckGLContextValue} from './utils/deckgl-context';
export type {DeckGLRef, DeckGLProps} from './deckgl';
15 changes: 15 additions & 0 deletions modules/react/src/utils/deckgl-context.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import {createContext} from 'react';
import type {EventManager} from 'mjolnir.js';
import type {Deck, DeckProps, Viewport, Widget} from '@deck.gl/core';

export type DeckGLContextValue = {
viewport: Viewport;
container: HTMLElement;
eventManager: EventManager;
onViewStateChange: DeckProps['onViewStateChange'];
deck?: Deck<any>;
widgets?: Widget[];
};

// @ts-ignore
export const DeckGlContext = createContext<DeckGLContextValue>();
46 changes: 19 additions & 27 deletions modules/react/src/utils/position-children-under-views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,15 @@ import {inheritsFrom} from './inherits-from';
import evaluateChildren, {isComponent} from './evaluate-children';

import type {ViewOrViews} from '../deckgl';
import type {Deck, DeckProps, Viewport} from '@deck.gl/core';
import type {EventManager} from 'mjolnir.js';

export type DeckGLContextValue = {
viewport: Viewport;
container: HTMLElement;
eventManager: EventManager;
onViewStateChange: DeckProps['onViewStateChange'];
};
import type {Deck, Viewport} from '@deck.gl/core';
import {DeckGlContext, type DeckGLContextValue} from './deckgl-context';

// Iterate over views and reposition children associated with views
// TODO - Can we supply a similar function for the non-React case?
export default function positionChildrenUnderViews<ViewsT extends ViewOrViews>({
children,
deck,
ContextProvider
ContextProvider = DeckGlContext.Provider
chrisgervang marked this conversation as resolved.
Show resolved Hide resolved
}: {
children: React.ReactNode[];
deck?: Deck<ViewsT>;
Expand Down Expand Up @@ -101,22 +94,21 @@ export default function positionChildrenUnderViews<ViewsT extends ViewOrViews>({
// a key" warning. Sending each child as separate arguments removes this requirement.
const viewElement = createElement('div', {key, id: key, style}, ...viewChildren);

if (ContextProvider) {
const contextValue: DeckGLContextValue = {
viewport,
// @ts-expect-error accessing protected property
container: deck.canvas.offsetParent,
// @ts-expect-error accessing protected property
eventManager: deck.eventManager,
onViewStateChange: params => {
params.viewId = viewId;
// @ts-expect-error accessing protected method
deck._onViewStateChange(params);
}
};
return createElement(ContextProvider, {key, value: contextValue}, viewElement);
}

return viewElement;
const contextValue: DeckGLContextValue = {
deck,
viewport,
// @ts-expect-error accessing protected property
container: deck.canvas.offsetParent,
// @ts-expect-error accessing protected property
eventManager: deck.eventManager,
onViewStateChange: params => {
params.viewId = viewId;
// @ts-expect-error accessing protected method
deck._onViewStateChange(params);
},
widgets: []
};
const providerKey = `view-${viewId}-context`;
return createElement(ContextProvider, {key: providerKey, value: contextValue}, viewElement);
});
}
41 changes: 41 additions & 0 deletions modules/react/src/utils/use-widget.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {useContext, useMemo, useEffect} from 'react';
import {DeckGlContext} from './deckgl-context';
import {log, type Widget, _deepEqual as deepEqual} from '@deck.gl/core';

function useWidget<T extends Widget, PropsT extends {}>(
WidgetClass: {new (props: PropsT): T},
props: PropsT
): T {
const context = useContext(DeckGlContext);
const {widgets, deck} = context;
useEffect(() => {
// warn if the user supplied a vanilla widget, since it will be ignored
// NOTE: This effect runs once per widget. Context widgets and deck widget props are synced after first effect runs.
const internalWidgets = deck?.props.widgets;
if (widgets?.length && internalWidgets?.length && !deepEqual(internalWidgets, widgets, 1)) {
log.warn('"widgets" prop will be ignored because React widgets are in use.')();
}

return () => {
// Remove widget from context when it is unmounted
const index = widgets?.indexOf(widget);
if (index && index !== -1) {
widgets?.splice(index, 1);
deck?.setProps({widgets});
}
};
}, []);
const widget = useMemo(() => new WidgetClass(props), [WidgetClass]);

// Hook rebuilds widgets on every render: [] then [FirstWidget] then [FirstWidget, SecondWidget]
widgets?.push(widget);
widget.setProps(props);

useEffect(() => {
deck?.setProps({widgets});
felixpalmer marked this conversation as resolved.
Show resolved Hide resolved
}, [widgets]);

return widget;
}

export default useWidget;
8 changes: 8 additions & 0 deletions modules/react/src/widgets/compass-widget.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {CompassWidget as _CompassWidget} from '@deck.gl/widgets';
import type {CompassWidgetProps} from '@deck.gl/widgets';
import useWidget from '../utils/use-widget';

export const CompassWidget = (props: CompassWidgetProps = {}) => {
const widget = useWidget(_CompassWidget, props);
return null;
};
8 changes: 8 additions & 0 deletions modules/react/src/widgets/fullscreen-widget.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {FullscreenWidget as _FullscreenWidget} from '@deck.gl/widgets';
import type {FullscreenWidgetProps} from '@deck.gl/widgets';
import useWidget from '../utils/use-widget';

export const FullscreenWidget = (props: FullscreenWidgetProps = {}) => {
const widget = useWidget(_FullscreenWidget, props);
return null;
};
8 changes: 8 additions & 0 deletions modules/react/src/widgets/zoom-widget.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {ZoomWidget as _ZoomWidget} from '@deck.gl/widgets';
import type {ZoomWidgetProps} from '@deck.gl/widgets';
import useWidget from '../utils/use-widget';

export const ZoomWidget = (props: ZoomWidgetProps = {}) => {
const widget = useWidget(_ZoomWidget, props);
return null;
};
3 changes: 2 additions & 1 deletion modules/react/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"outDir": "dist"
},
"references": [
{"path": "../core"}
{"path": "../core"},
{"path": "../widgets"}
]
}
2 changes: 1 addition & 1 deletion modules/widgets/src/compass-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';

interface CompassWidgetProps {
export interface CompassWidgetProps {
id?: string;
placement?: WidgetPlacement;
/**
Expand Down
2 changes: 1 addition & 1 deletion modules/widgets/src/fullscreen-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type {Deck, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';
import {IconButton} from './components';

interface FullscreenWidgetProps {
export interface FullscreenWidgetProps {
id?: string;
placement?: WidgetPlacement;
/**
Expand Down
4 changes: 4 additions & 0 deletions modules/widgets/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ export {FullscreenWidget} from './fullscreen-widget';
export {CompassWidget} from './compass-widget';
export {ZoomWidget} from './zoom-widget';

export type {FullscreenWidgetProps} from './fullscreen-widget';
export type {CompassWidgetProps} from './compass-widget';
export type {ZoomWidgetProps} from './zoom-widget';

export * from './themes';
2 changes: 1 addition & 1 deletion modules/widgets/src/zoom-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';
import {ButtonGroup, GroupedIconButton} from './components';

interface ZoomWidgetProps {
export interface ZoomWidgetProps {
Copy link
Collaborator Author

@chrisgervang chrisgervang Dec 13, 2024

Choose a reason for hiding this comment

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

@Pessimistress I'm noticing that the widget's onViewportChange isn't called on initialization. The user needs to interact with deck before a viewport is set.

Vanilla widgets work. Any idea what's different?

Copy link
Collaborator Author

@chrisgervang chrisgervang Dec 13, 2024

Choose a reason for hiding this comment

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

The cause is this guard is hit before the viewports have been assigned to the widget. lastViewports was set on the first time props are set (when widgets: [] and it initialized the tooltip).
Screenshot 2024-12-12 at 5 18 07 PM

The goal of this code is to cache the current viewport so that the widget can use it as the starting point of its view modification.

The hook adds widgets in one-by-one: [] then [FirstWidget] then [FirstWidget, SecondWidget]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess we could push this check down into each widget to implement since the manager isn't aware of how widgets store viewports, or we could add viewports: {} to class Widget and have the manager check that instead? Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See proposed fix in #9303

id?: string;
placement?: WidgetPlacement;
/**
Expand Down
45 changes: 45 additions & 0 deletions test/modules/core/lib/deck.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import test from 'tape-promise/tape';
import {Deck, log, MapView} from '@deck.gl/core';
import {ScatterplotLayer} from '@deck.gl/layers';
import {FullscreenWidget} from '@deck.gl/widgets';
import {device} from '@deck.gl/test-utils';
import {sleep} from './async-iterator-test-utils';

Expand Down Expand Up @@ -280,3 +281,47 @@ test('Deck#resourceManager', async t => {
deck.finalize();
t.end();
});

test('Deck#props omitted are unchanged', async t => {
const layer = new ScatterplotLayer({
id: 'scatterplot-global-data',
data: 'deck://pins',
getPosition: d => d.position
});

const widget = new FullscreenWidget({});

// Initialize with widgets and layers.
const deck = new Deck({
device,
width: 1,
height: 1,

viewState: {
longitude: 0,
latitude: 0,
zoom: 0
},

layers: [layer],
widgets: [widget],

onLoad: () => {
const {widgets, layers} = deck.props;
t.is(widgets && Array.isArray(widgets) && widgets.length, 1, 'Widgets is set');
t.is(layers && Array.isArray(layers) && layers.length, 1, 'Layers is set');

// Render deck a second time without changing widget or layer props.
deck.setProps({
onAfterRender: () => {
const {widgets, layers} = deck.props;
t.is(widgets && Array.isArray(widgets) && widgets.length, 1, 'Widgets remain set');
t.is(layers && Array.isArray(layers) && layers.length, 1, 'Layers remain set');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See below, this works a little differently between react and pure-js. I believe this is intentional and just wasn't covered in our tests.


deck.finalize();
t.end();
}
});
}
});
});
16 changes: 15 additions & 1 deletion test/modules/core/lib/widget-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,24 @@ test('WidgetManager#setProps', t => {
t.notOk(widgetB.isVisible, 'widget.onRemove is called');
t.ok(widgetB2.isVisible, 'widget.onAdd is called');

widgetManager.setProps({widgets: []});
t.is(widgetManager.getWidgets().length, 0, 'all widgets are removed');
t.notOk(widgetB2.isVisible, 'widget.onRemove is called');

t.end();
});

test('WidgetManager#finalize', t => {
const container = document.createElement('div');
const widgetManager = new WidgetManager({deck: mockDeckInstance, parentElement: container});

const widgetA = new TestWidget({id: 'A'});
widgetManager.setProps({widgets: [widgetA]});

widgetManager.finalize();
t.is(widgetManager.getWidgets().length, 0, 'all widgets are removed');
t.is(container.childElementCount, 0, 'all widget containers are removed');
t.notOk(widgetB2.isVisible, 'widget.onRemove is called');
t.notOk(widgetA.isVisible, 'widget.onRemove is called');

t.end();
});
Expand Down
Loading
Loading