Skip to content

Commit

Permalink
✨ Refactor grapher and extract most of the state into GrapherState
Browse files Browse the repository at this point in the history
  • Loading branch information
danyx23 committed Jan 10, 2025
1 parent 13c6dc0 commit 2348bca
Show file tree
Hide file tree
Showing 18 changed files with 2,598 additions and 2,466 deletions.
5 changes: 2 additions & 3 deletions adminSiteServer/testPageRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ async function propsFromQueryParams(
const page = params.page
? expectInt(params.page)
: params.random
? Math.floor(1 + Math.random() * 180) // Sample one of 180 pages. Some charts won't ever get picked but good enough.
: 1
? Math.floor(1 + Math.random() * 180) // Sample one of 180 pages. Some charts won't ever get picked but good enough.
: 1
const perPage = parseIntOrUndefined(params.perPage) ?? 20
const ids = parseIntArrayOrUndefined(params.ids)
const datasetIds = parseIntArrayOrUndefined(params.datasetIds)
Expand Down Expand Up @@ -802,7 +802,6 @@ getPlainRouteWithROTransaction(
res.send(svg)
}
)

testPageRouter.get("/explorers", async (req, res) => {
let explorers = await explorerAdminServer.getAllPublishedExplorers()
const viewProps = getViewPropsFromQueryParams(req.query)
Expand Down
26 changes: 14 additions & 12 deletions packages/@ourworldindata/explorer/src/Explorer.jsdom.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,28 @@ describe(Explorer, () => {

explorer.onChangeChoice("Gas")("All GHGs (CO₂eq)")

if (explorer.grapher) explorer.grapher.tab = GRAPHER_TAB_OPTIONS.table
if (explorer.grapher?.grapherState)
explorer.grapher.grapherState.tab = GRAPHER_TAB_OPTIONS.table
else throw Error("where's the grapher?")
expect(explorer.queryParams.tab).toEqual("table")

explorer.onChangeChoice("Gas")("CO₂")
expect(explorer.queryParams.tab).toEqual("table")

explorer.grapher.tab = GRAPHER_TAB_OPTIONS.chart
explorer.grapher.grapherState.tab = GRAPHER_TAB_OPTIONS.chart
})

it("switches to first tab if current tab does not exist in new view", () => {
const explorer = element.instance() as Explorer
expect(explorer.queryParams.tab).toBeUndefined()
if (explorer.grapher) explorer.grapher.tab = GRAPHER_TAB_OPTIONS.map
if (explorer.grapher?.grapherState)
explorer.grapher.grapherState.tab = GRAPHER_TAB_OPTIONS.map
else throw Error("where's the grapher?")
expect(explorer.queryParams.tab).toEqual("map")

explorer.onChangeChoice("Gas")("All GHGs (CO₂eq)")

expect(explorer.grapher.tab).toEqual("chart")
expect(explorer.grapher?.grapherState.tab).toEqual("chart")
expect(explorer.queryParams.tab).toEqual(undefined)
})

Expand Down Expand Up @@ -85,20 +87,20 @@ describe("inline data explorer", () => {
expect(explorer.queryParams).toMatchObject({
Test: "Scatter",
})
expect(explorer.grapher?.xSlug).toEqual("x")
expect(explorer.grapher?.ySlugs).toEqual("y")
expect(explorer.grapher?.colorSlug).toEqual("color")
expect(explorer.grapher?.sizeSlug).toEqual("size")
expect(explorer.grapher?.grapherState?.xSlug).toEqual("x")
expect(explorer.grapher?.grapherState?.ySlugs).toEqual("y")
expect(explorer.grapher?.grapherState?.colorSlug).toEqual("color")
expect(explorer.grapher?.grapherState?.sizeSlug).toEqual("size")
})

it("clears column slugs that don't exist in current row", () => {
explorer.onChangeChoice("Test")("Line")
expect(explorer.queryParams).toMatchObject({
Test: "Line",
})
expect(explorer.grapher?.xSlug).toEqual(undefined)
expect(explorer.grapher?.ySlugs).toEqual("y")
expect(explorer.grapher?.colorSlug).toEqual(undefined)
expect(explorer.grapher?.sizeSlug).toEqual(undefined)
expect(explorer.grapher?.grapherState?.xSlug).toEqual(undefined)
expect(explorer.grapher?.grapherState?.ySlugs).toEqual("y")
expect(explorer.grapher?.grapherState?.colorSlug).toEqual(undefined)
expect(explorer.grapher?.grapherState?.sizeSlug).toEqual(undefined)
})
})
73 changes: 38 additions & 35 deletions packages/@ourworldindata/explorer/src/Explorer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
SlideShowManager,
DEFAULT_GRAPHER_ENTITY_TYPE,
GrapherAnalytics,
GrapherState,
FocusArray,
} from "@ourworldindata/grapher"
import {
Expand Down Expand Up @@ -195,15 +196,23 @@ export class Explorer
GrapherManager
{
analytics = new GrapherAnalytics()
grapherState: GrapherState

constructor(props: ExplorerProps) {
super(props)
this.explorerProgram = ExplorerProgram.fromJson(
props
).initDecisionMatrix(this.initialQueryParams)
this.grapher = new Grapher({
bounds: props.bounds,
this.grapherState = new GrapherState({
staticBounds: props.staticBounds,
bounds: props.bounds,
enableKeyboardShortcuts: true,
manager: this,
isEmbeddedInAnOwidPage: this.props.isEmbeddedInAnOwidPage,
adminBaseUrl: this.adminBaseUrl,
})
this.grapher = new Grapher({
grapherState: this.grapherState,
})
}
// caution: do a ctrl+f to find untyped usages
Expand Down Expand Up @@ -332,7 +341,7 @@ export class Explorer

if (this.props.isInStandalonePage) this.setCanonicalUrl()

this.grapher?.populateFromQueryParams(url.queryParams)
this.grapher?.grapherState?.populateFromQueryParams(url.queryParams)

exposeInstanceOnWindow(this, "explorer")
this.setUpIntersectionObserver()
Expand All @@ -353,7 +362,7 @@ export class Explorer
this.explorerProgram.indexViewsSeparately &&
document.location.search
) {
document.title = `${this.grapher.displayTitle} - Our World in Data`
document.title = `${this.grapher?.grapherState.displayTitle} - Our World in Data`
}
}

Expand Down Expand Up @@ -431,7 +440,7 @@ export class Explorer
return // todo: can we remove this?
this.initSlideshow()

const oldGrapherParams = this.grapher.changedParams
const oldGrapherParams = this.grapher?.grapherState.changedParams
this.persistedGrapherQueryParamsBySelectedRow.set(
oldSelectedRow,
oldGrapherParams
Expand All @@ -443,31 +452,34 @@ export class Explorer
),
country: oldGrapherParams.country,
region: oldGrapherParams.region,
time: this.grapher.timeParam,
time: this.grapher?.grapherState.timeParam,
}

const previousTab = this.grapher.activeTab
const previousTab = this.grapher?.grapherState.activeTab

this.updateGrapherFromExplorer()

if (this.grapher.availableTabs.includes(previousTab)) {
if (this.grapher?.grapherState.availableTabs.includes(previousTab)) {
// preserve the previous tab if that's still available in the new view
newGrapherParams.tab =
this.grapher.mapGrapherTabToQueryParam(previousTab)
} else if (this.grapher.validChartTypes.length > 0) {
this.grapher?.grapherState.mapGrapherTabToQueryParam(
previousTab
)
} else if (this.grapher?.grapherState.validChartTypes.length > 0) {
// otherwise, switch to the first chart tab
newGrapherParams.tab = this.grapher.mapGrapherTabToQueryParam(
this.grapher.validChartTypes[0]
)
} else if (this.grapher.hasMapTab) {
newGrapherParams.tab =
this.grapher?.grapherState.mapGrapherTabToQueryParam(
this.grapher?.grapherState.validChartTypes[0]
)
} else if (this.grapher?.grapherState.hasMapTab) {
// or switch to the map, if there is one
newGrapherParams.tab = GRAPHER_TAB_QUERY_PARAMS.map
} else {
// if everything fails, switch to the table tab that is always available
newGrapherParams.tab = GRAPHER_TAB_QUERY_PARAMS.table
}

this.grapher.populateFromQueryParams(newGrapherParams)
this.grapher?.grapherState.populateFromQueryParams(newGrapherParams)

this.analytics.logExplorerView(
this.explorerProgram.slug,
Expand All @@ -477,7 +489,7 @@ export class Explorer

@action.bound private setGrapherTable(table: OwidTable) {
if (this.grapher) {
this.grapher.inputTable = table
this.grapher.grapherState.inputTable = table
this.grapher.appendNewEntitySelectionOptions()
}
}
Expand Down Expand Up @@ -575,9 +587,9 @@ export class Explorer
config.selectedEntityNames = this.selection.selectedEntityNames
}

grapher.setAuthoredVersion(config)
grapher?.grapherState.setAuthoredVersion(config)
grapher.reset()
grapher.updateFromObject(config)
grapher?.grapherState.updateFromObject(config)
// grapher.downloadData()
}

Expand Down Expand Up @@ -739,9 +751,9 @@ export class Explorer
return table
}

grapher.setAuthoredVersion(config)
grapher?.grapherState.setAuthoredVersion(config)
grapher.reset()
grapher.updateFromObject(config)
grapher?.grapherState.updateFromObject(config)
if (dimensions.length === 0) {
// If dimensions are empty, explicitly set the table to an empty table
// so we don't end up confusingly showing stale data from a previous chart
Expand Down Expand Up @@ -772,9 +784,9 @@ export class Explorer
config.selectedEntityNames = this.selection.selectedEntityNames
}

grapher.setAuthoredVersion(config)
grapher?.grapherState.setAuthoredVersion(config)
grapher.reset()
grapher.updateFromObject(config)
grapher?.grapherState.updateFromObject(config)

// Clear any error messages, they are likely to be related to dataset loading.
this.grapher?.clearErrors()
Expand Down Expand Up @@ -808,7 +820,7 @@ export class Explorer

let url = Url.fromQueryParams(
omitUndefinedValues({
...this.grapher.changedParams,
...this.grapher?.grapherState.changedParams,
pickerSort: this.entityPickerSort,
pickerMetric: this.entityPickerMetric,
hideControls: this.initialQueryParams.hideControls || undefined,
Expand Down Expand Up @@ -1029,16 +1041,7 @@ export class Explorer
this.isNarrow &&
this.mobileCustomizeButton}
<div className="ExplorerFigure" ref={this.grapherContainerRef}>
<Grapher
adminBaseUrl={this.adminBaseUrl}
bounds={this.grapherBounds}
enableKeyboardShortcuts={true}
manager={this}
ref={this.grapherRef}
isEmbeddedInAnOwidPage={
this.props.isEmbeddedInAnOwidPage
}
/>
<Grapher grapherState={this.grapherState} />
</div>
</div>
)
Expand Down Expand Up @@ -1079,7 +1082,7 @@ export class Explorer
}

@computed get grapherTable() {
return this.grapher?.tableAfterAuthorTimelineFilter
return this.grapher?.grapherState?.tableAfterAuthorTimelineFilter
}

@observable entityPickerMetric? = this.initialQueryParams.pickerMetric
Expand Down Expand Up @@ -1194,6 +1197,6 @@ export class Explorer
}

@computed get requiredColumnSlugs() {
return this.grapher?.newSlugs ?? []
return this.grapher?.grapherState?.newSlugs ?? []
}
}
14 changes: 7 additions & 7 deletions packages/@ourworldindata/grapher/src/chart/DimensionSlot.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
// todo: remove

import { Grapher } from "../core/Grapher"
import { Grapher, GrapherState } from "../core/Grapher"

Check warning on line 3 in packages/@ourworldindata/grapher/src/chart/DimensionSlot.ts

View workflow job for this annotation

GitHub Actions / eslint

'Grapher' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 3 in packages/@ourworldindata/grapher/src/chart/DimensionSlot.ts

View workflow job for this annotation

GitHub Actions / eslint

'Grapher' is defined but never used. Allowed unused vars must match /^_/u
import { computed } from "mobx"
import { ChartDimension } from "./ChartDimension"
import { DimensionProperty } from "@ourworldindata/utils"

export class DimensionSlot {
private grapher: Grapher
private grapherState: GrapherState
property: DimensionProperty
constructor(grapher: Grapher, property: DimensionProperty) {
this.grapher = grapher
constructor(grapher: GrapherState, property: DimensionProperty) {
this.grapherState = grapher
this.property = property
}

@computed get name(): string {
const names = {
y: this.grapher.isDiscreteBar ? "X axis" : "Y axis",
y: this.grapherState.isDiscreteBar ? "X axis" : "Y axis",
x: "X axis",
size: "Size",
color: "Color",
Expand All @@ -28,7 +28,7 @@ export class DimensionSlot {
@computed get allowMultiple(): boolean {
return (
this.property === DimensionProperty.y &&
this.grapher.supportsMultipleYColumns
this.grapherState.supportsMultipleYColumns
)
}

Expand All @@ -37,7 +37,7 @@ export class DimensionSlot {
}

@computed get dimensions(): ChartDimension[] {
return this.grapher.dimensions.filter(
return this.grapherState.dimensions.filter(
(d) => d.property === this.property
)
}
Expand Down
Loading

0 comments on commit 2348bca

Please sign in to comment.