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 6, 2025
1 parent ca4adc4 commit 338a5ed
Show file tree
Hide file tree
Showing 18 changed files with 2,723 additions and 2,480 deletions.
67 changes: 67 additions & 0 deletions adminSiteServer/testPageRouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import {
ColorSchemes,
GrapherProgrammaticInterface,
} from "@ourworldindata/grapher"
import { DATA_API_URL } from "../settings/clientSettings.js"

const IS_LIVE = ADMIN_BASE_URL === "https://owid.cloud"
const DEFAULT_COMPARISON_URL = "https://ourworldindata.org"
Expand Down Expand Up @@ -803,6 +804,72 @@ getPlainRouteWithROTransaction(
}
)

getPlainRouteWithROTransaction(
testPageRouter,
"/experiment",
async (req, res, trx) => {

Check warning on line 810 in adminSiteServer/testPageRouter.tsx

View workflow job for this annotation

GitHub Actions / eslint

'trx' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 810 in adminSiteServer/testPageRouter.tsx

View workflow job for this annotation

GitHub Actions / eslint

'trx' is defined but never used. Allowed unused args must match /^_/u

Check warning on line 810 in adminSiteServer/testPageRouter.tsx

View workflow job for this annotation

GitHub Actions / eslint

'trx' is defined but never used. Allowed unused args must match /^_/u
const style = `
.row {
padding: 10px;
margin: 0;
border-bottom: 1px solid #ddd;
}
.side-by-side {
display: flex;
align-items: center;
justify-content: space-around;
width: 100%;
}
.side-by-side + .side-by-side {
margin-top: 64px;
}
`
const script = `
const grapher = new FetchingGrapher();
const container = document.getElementById('graphers');
const root = ReactDOM.createRoot(container);
const grapher1Props = {
configUrl: "https://ourworldindata.org/grapher/life-expectancy.config.json",
dataApiUrl: "${DATA_API_URL}"
};
root.render(
React.createElement(
React.Fragment,
null,
React.createElement(FetchingGrapher, grapher1Props),
React.createElement(FetchingGrapher, grapher1Props)
)
);
`
const html = (
<html>
<Head
canonicalUrl=""
pageTitle="Experiment"
baseUrl={BAKED_BASE_URL}
>
<style dangerouslySetInnerHTML={{ __html: style }} />
</Head>
<body>
<div className="row">
<div id="graphers" className="side-by-side"></div>
</div>
<script src={`${BAKED_BASE_URL}/assets/embedCharts.js`} />
<script
dangerouslySetInnerHTML={{ __html: script }}
></script>
</body>
</html>
)

res.send(renderToHtmlPage(html))
}
)

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,
} from "@ourworldindata/grapher"
import {
Bounds,
Expand Down Expand Up @@ -194,15 +195,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 @@ -329,7 +338,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 @@ -350,7 +359,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 @@ -428,7 +437,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 @@ -440,31 +449,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 @@ -474,7 +486,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 @@ -572,9 +584,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 @@ -736,9 +748,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 @@ -769,9 +781,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 @@ -805,7 +817,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 @@ -1026,16 +1038,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 @@ -1076,7 +1079,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 @@ -1191,6 +1194,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

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 338a5ed

Please sign in to comment.