-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Cache JSON when using pydeck local data #7113
Changes from 8 commits
c148941
bfb51f6
9facb03
e4e6f44
15999f6
a6270ef
69d30c4
af51b14
c2eed5c
ac78cf9
2cc5f97
426d73d
a08deb9
d0dafef
bfecde4
498a117
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 |
---|---|---|
|
@@ -39,8 +39,6 @@ import { registerLoaders } from "@loaders.gl/core" | |
import withFullScreenWrapper from "@streamlit/lib/src/hocs/withFullScreenWrapper" | ||
import withMapboxToken from "@streamlit/lib/src/hocs/withMapboxToken" | ||
|
||
import { notNullOrUndefined } from "@streamlit/lib/src/util/utils" | ||
|
||
import { DeckGlJsonChart as DeckGlJsonChartProto } from "@streamlit/lib/src/proto" | ||
import { | ||
StyledDeckGlChart, | ||
|
@@ -83,10 +81,14 @@ export interface PropsWithHeight extends Props { | |
height?: number | ||
} | ||
|
||
interface State { | ||
export interface State { | ||
viewState: Record<string, unknown> | ||
initialized: boolean | ||
initialViewState: Record<string, unknown> | ||
elementId: string | undefined | ||
pydeckJson: any | ||
isFullScreen: boolean | ||
isLightTheme: boolean | ||
} | ||
|
||
export const DEFAULT_DECK_GL_HEIGHT = 500 | ||
|
@@ -100,6 +102,10 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> { | |
}, | ||
initialized: false, | ||
initialViewState: {}, | ||
elementId: undefined, | ||
pydeckJson: undefined, | ||
isFullScreen: false, | ||
isLightTheme: hasLightBackgroundColor(this.props.theme), | ||
} | ||
|
||
componentDidMount = (): void => { | ||
|
@@ -115,7 +121,7 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> { | |
props: Readonly<PropsWithHeight>, | ||
state: Partial<State> | ||
): Partial<State> | null { | ||
const deck = DeckGlJsonChart.getDeckObject(props) | ||
const deck = DeckGlJsonChart.getDeckObject(props, state) | ||
|
||
// If the ViewState on the server has changed, apply the diff to the current state | ||
if (!isEqual(deck.initialViewState, state.initialViewState)) { | ||
|
@@ -144,35 +150,51 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> { | |
return null | ||
} | ||
|
||
static getDeckObject = (props: PropsWithHeight): DeckObject => { | ||
static getDeckObject = ( | ||
props: PropsWithHeight, | ||
state: Partial<State> | ||
): DeckObject => { | ||
const { element, width, height, theme } = props | ||
const json = JSON.parse(element.json) | ||
const { elementId } = state | ||
|
||
const isFullScreen = Boolean(height) | ||
willhuang1997 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Only parse JSON when not transitioning to/from fullscreen, the element id changes, or theme changes | ||
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. maybe i should remove this comment since it's not necessary? 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. You could also add one more sentence here why we need to reparse with these changes |
||
if ( | ||
element.elementId !== elementId || | ||
state.isFullScreen !== isFullScreen || | ||
state.isLightTheme !== hasLightBackgroundColor(theme) | ||
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. Why do we need to reparse this when layout changes? Aren't we always adapting the mapStyle below anyways based on the theme? |
||
) { | ||
state.pydeckJson = JSON.parse(element.json) | ||
state.elementId = element.elementId | ||
} | ||
|
||
// If unset, use either the Mapbox light or dark style based on Streamlit's theme | ||
// For Mapbox styles, see https://docs.mapbox.com/api/maps/styles/#mapbox-styles | ||
if (!notNullOrUndefined(json.mapStyle)) { | ||
const mapTheme = hasLightBackgroundColor(theme) ? "light" : "dark" | ||
json.mapStyle = `mapbox://styles/mapbox/${mapTheme}-v9` | ||
if (!state.pydeckJson?.mapStyle) { | ||
state.pydeckJson.mapStyle = `mapbox://styles/mapbox/${ | ||
hasLightBackgroundColor(theme) ? "light" : "dark" | ||
}-v9` | ||
} | ||
|
||
// The graph dimensions could be set from props ( like withFullscreen ) or | ||
// from the generated element object | ||
if (height) { | ||
json.initialViewState.height = height | ||
json.initialViewState.width = width | ||
// Set width and height based on the fullscreen state | ||
if (isFullScreen) { | ||
Object.assign(state.pydeckJson?.initialViewState, { width, height }) | ||
} else { | ||
if (!json.initialViewState.height) { | ||
json.initialViewState.height = DEFAULT_DECK_GL_HEIGHT | ||
if (!state.pydeckJson?.initialViewState?.height) { | ||
state.pydeckJson.initialViewState.height = DEFAULT_DECK_GL_HEIGHT | ||
} | ||
|
||
if (element.useContainerWidth) { | ||
json.initialViewState.width = width | ||
state.pydeckJson.initialViewState.width = width | ||
} | ||
} | ||
|
||
delete json.views // We are not using views. This avoids a console warning. | ||
state.isFullScreen = isFullScreen | ||
state.isLightTheme = hasLightBackgroundColor(theme) | ||
|
||
delete state.pydeckJson?.views // We are not using views. This avoids a console warning. | ||
|
||
return jsonConverter.convert(json) | ||
return jsonConverter.convert(state.pydeckJson) | ||
} | ||
|
||
createTooltip = (info: PickingInfo): Record<string, unknown> | boolean => { | ||
|
@@ -213,9 +235,8 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> { | |
} | ||
|
||
render(): ReactNode { | ||
const deck = DeckGlJsonChart.getDeckObject(this.props) | ||
const deck = DeckGlJsonChart.getDeckObject(this.props, this.state) | ||
const { viewState } = this.state | ||
|
||
return ( | ||
<StyledDeckGlChart | ||
className="stDeckGlJsonChart" | ||
|
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.
just moved this to top of file so I can reuse later in the tests