-
Notifications
You must be signed in to change notification settings - Fork 2.2k
chore(widgets): Encapsulate the hacky / complex setViewState() #9596
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -4,20 +4,17 @@ | |
|
||
import {Widget, WidgetProps} from '@deck.gl/core'; | ||
import type {WidgetPlacement, Viewport} from '@deck.gl/core'; | ||
import {FlyToInterpolator, LinearInterpolator} from '@deck.gl/core'; | ||
import {render} from 'preact'; | ||
|
||
/** @todo - is the the best we can do? */ | ||
type ViewState = Record<string, unknown>; | ||
|
||
/** Properties for the GeolocateWidget */ | ||
export type GeolocateWidgetProps = WidgetProps & { | ||
viewId?: string; | ||
/** Widget positioning within the view. Default 'top-left'. */ | ||
placement?: WidgetPlacement; | ||
/** Tooltip message */ | ||
label?: string; | ||
transitionDuration?: number; | ||
/** Animated transition duration. Set to 0 to disable transitions */ | ||
transitionDurationMs?: number; | ||
}; | ||
|
||
/** | ||
|
@@ -32,7 +29,7 @@ export class GeolocateWidget extends Widget<GeolocateWidgetProps> { | |
viewId: undefined!, | ||
placement: 'top-left', | ||
label: 'Geolocate', | ||
transitionDuration: 200 | ||
transitionDurationMs: 200 | ||
}; | ||
|
||
className = 'deck-widget-geolocate'; | ||
|
@@ -95,31 +92,6 @@ export class GeolocateWidget extends Widget<GeolocateWidgetProps> { | |
handleCoordinates = coordinates => { | ||
this.setViewState(coordinates); | ||
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. This is broken |
||
}; | ||
|
||
// TODO - MOVE TO WIDGETIMPL? | ||
|
||
setViewState(viewState: ViewState) { | ||
const viewId = this.props.viewId || (viewState?.id as string) || 'default-view'; | ||
const viewport = this.viewports[viewId] || {}; | ||
Comment on lines
-102
to
-103
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. This is necessary in widget that set view state |
||
const nextViewState: ViewState = { | ||
...viewport, | ||
...viewState | ||
}; | ||
if (this.props.transitionDuration > 0) { | ||
nextViewState.transitionDuration = this.props.transitionDuration; | ||
nextViewState.transitionInterpolator = | ||
'latitude' in nextViewState ? new FlyToInterpolator() : new LinearInterpolator(); | ||
} | ||
|
||
// @ts-ignore Using private method temporary until there's a public one | ||
this.deck._onViewStateChange({viewId, viewState: nextViewState, interactionState: {}}); | ||
} | ||
|
||
onViewportChange(viewport: Viewport) { | ||
this.viewports[viewport.id] = viewport; | ||
} | ||
|
||
viewports: Record<string, Viewport> = {}; | ||
Comment on lines
-118
to
-122
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. Keep |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,9 @@ export type ResetViewWidgetProps = WidgetProps & { | |
/** The initial view state to reset the view to. Defaults to deck.props.initialViewState */ | ||
initialViewState?: ViewState; | ||
/** View to interact with. Required when using multiple views. */ | ||
viewId?: string | null; | ||
viewId?: string; | ||
/** Set to 0 to disable transitions */ | ||
transitionDurationMs?: number; | ||
}; | ||
|
||
/** | ||
|
@@ -32,7 +34,8 @@ export class ResetViewWidget extends Widget<ResetViewWidgetProps> { | |
placement: 'top-left', | ||
label: 'Reset View', | ||
initialViewState: undefined!, | ||
viewId: undefined! | ||
viewId: undefined!, | ||
transitionDurationMs: 200, | ||
}; | ||
|
||
className = 'deck-widget-reset-view'; | ||
|
@@ -61,18 +64,10 @@ export class ResetViewWidget extends Widget<ResetViewWidgetProps> { | |
|
||
handleClick() { | ||
const initialViewState = this.props.initialViewState || this.deck?.props.initialViewState; | ||
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. We really ought to combine what this widget does and what compass widget does so that they can be used in either mode (initialViewState/viewState) A user managing viewState would be able to use this widget too if we made In compass, we don't need to expose a prop but just check if |
||
this.setViewState(initialViewState); | ||
} | ||
|
||
setViewState(viewState: ViewState) { | ||
const viewId = this.props.viewId || viewState?.id || 'default-view'; | ||
const nextViewState = { | ||
...viewState | ||
// only works for geospatial? | ||
// transitionDuration: this.props.transitionDuration, | ||
// transitionInterpolator: new FlyToInterpolator() | ||
}; | ||
// @ts-ignore Using private method temporary until there's a public one | ||
this.deck._onViewStateChange({viewId, viewState: nextViewState, interactionState: {}}); | ||
this.setViewState({ | ||
viewState: initialViewState, | ||
viewId: this.props.viewId, | ||
transitionDurationMs: this.props.transitionDurationMs | ||
}); | ||
} | ||
} |
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.
Make protected?