-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
base: master
Are you sure you want to change the base?
Changes from 33 commits
9bbdc79
e55ab6f
9803538
c526999
51154e8
e6a7a5f
5549372
b8aaef2
5508ccd
3ed7e27
214f5e6
74c241b
a0d0b6e
26a95a1
271561f
1e62c65
ec8d18f
7cd5d58
b7a79ec
82287da
971da0f
09b577a
7e4c4fb
1e5545f
91c0930
ffe8362
23a3210
5b995d6
3cfd798
7dbf2cd
8c822b4
d17b012
aa9ae7b
f144266
323c020
43168f2
1ff736c
ea0cf0c
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,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>(); |
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; |
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; | ||
}; |
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; | ||
}; |
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; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
"outDir": "dist" | ||
}, | ||
"references": [ | ||
{"path": "../core"} | ||
{"path": "../core"}, | ||
{"path": "../widgets"} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
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. @Pessimistress I'm noticing that the widget's Vanilla widgets work. Any idea what's different? 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. The cause is this guard is hit before the viewports have been assigned to the widget. 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: 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. 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 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. See proposed fix in #9303 |
||
id?: string; | ||
placement?: WidgetPlacement; | ||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -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'); | ||
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. 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(); | ||
} | ||
}); | ||
} | ||
}); | ||
}); |
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.
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.