Skip to content

Commit

Permalink
✨ Move remaining computed props into grapherstate. This fixes a few b…
Browse files Browse the repository at this point in the history
…ugs.

The main channel through which this led to undesired behavior is that we used to pass
the full Grapher as manager into components like the Footer; we now only pass the
GrapherState. But because the interfaces that outline managers (like that of the footer)
often use optional properties, the type system didn't flag that some of the computed
properties were missing now on GrapherState. This commit moves the remaining computed
state props onto GrapherState.
  • Loading branch information
danyx23 committed Feb 2, 2025
1 parent 48e0203 commit 2d53d5a
Showing 1 changed file with 63 additions and 75 deletions.
138 changes: 63 additions & 75 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3121,32 +3121,10 @@ export class GrapherState {
}

@observable isShareMenuActive = false
}

export interface GrapherProps {
grapherState: GrapherState
}

@observer
export class Grapher extends React.Component<GrapherProps> {
@computed get grapherState(): GrapherState {
return this.props.grapherState
}

// #region Observable props not in any interface

analytics = new GrapherAnalytics(
this.props.grapherState.initialOptions.env ?? ""
)

// stored on Grapher so state is preserved when switching to full-screen mode

@observable
private legacyVariableDataJson?: MultipleOwidVariableDataDimensionsMap
private hasLoggedGAViewEvent = false
@observable private hasBeenVisible = false
@observable private uncaughtError?: Error
@observable slideShow?: SlideShowController<any>
// Computed props that are not required by any inteface but can be optional in interfaces
// and could then lead to changed behavior vs previously if now only GrapherState is passed into
// a component.

/**
* Whether the chart is rendered in an Admin context (e.g. on owid.cloud).
Expand All @@ -3162,49 +3140,41 @@ export class Grapher extends React.Component<GrapherProps> {

@computed get shouldLinkToOwid(): boolean {
if (
this.grapherState.isEmbeddedInAnOwidPage ||
this.grapherState.isExportingToSvgOrPng ||
!this.grapherState.isInIFrame
this.isEmbeddedInAnOwidPage ||
this.isExportingToSvgOrPng ||
!this.isInIFrame
)
return false

return true
}

@computed.struct private get variableIds(): number[] {
return uniq(this.grapherState.dimensions.map((d) => d.variableId))
return uniq(this.dimensions.map((d) => d.variableId))
}

@computed get hasOWIDLogo(): boolean {
return (
!this.grapherState.hideLogo &&
(this.grapherState.logo === undefined ||
this.grapherState.logo === "owid")
!this.hideLogo && (this.logo === undefined || this.logo === "owid")
)
}

@computed get xScaleType(): ScaleType | undefined {
return this.grapherState.xAxis.scaleType
return this.xAxis.scaleType
}

@computed get hasYDimension(): boolean {
return this.grapherState.dimensions.some(
(d) => d.property === DimensionProperty.y
)
}

@computed get defaultBounds(): Bounds {
return new Bounds(0, 0, DEFAULT_GRAPHER_WIDTH, DEFAULT_GRAPHER_HEIGHT)
return this.dimensions.some((d) => d.property === DimensionProperty.y)
}

@computed get cacheTag(): string {
return this.grapherState.version.toString()
return this.version.toString()
}

// Filter data to what can be display on the map (across all times)
@computed get mappableData(): OwidVariableRow<any>[] {
return this.grapherState.inputTable
.get(this.grapherState.mapColumnSlug)
return this.inputTable
.get(this.mapColumnSlug)
.owidRows.filter((row) => isOnTheMap(row.entityName))
}

Expand All @@ -3213,44 +3183,40 @@ export class Grapher extends React.Component<GrapherProps> {
}

@computed get hideFullScreenButton(): boolean {
if (this.grapherState.isInFullScreenMode) return false
if (this.isInFullScreenMode) return false
// hide the full screen button if the full screen height
// is barely larger than the current chart height
const fullScreenHeight = this.grapherState.windowInnerHeight!
return fullScreenHeight < this.grapherState.frameBounds.height + 80
const fullScreenHeight = this.windowInnerHeight!
return fullScreenHeight < this.frameBounds.height + 80
}

@computed get sidePanelBounds(): Bounds | undefined {
if (!this.grapherState.isEntitySelectorPanelActive) return
if (!this.isEntitySelectorPanelActive) return

return new Bounds(
0, // not in use; intentionally set to zero
0, // not in use; intentionally set to zero
this.grapherState.frameBounds.width -
this.grapherState.captionedChartBounds.width,
this.grapherState.captionedChartBounds.height
this.frameBounds.width - this.captionedChartBounds.width,
this.captionedChartBounds.height
)
}
@computed get containerElement(): HTMLDivElement | undefined {
return this.grapherState.base.current || undefined
return this.base.current || undefined
}

// the header and footer don't rely on the base font size unless explicitly specified
@computed get useBaseFontSize(): boolean {
return (
this.props.grapherState.initialOptions.baseFontSize !== undefined ||
this.grapherState.isStatic
)
return this.initialOptions.baseFontSize !== undefined || this.isStatic
}

@computed get hasRelatedQuestion(): boolean {
if (
this.grapherState.hideRelatedQuestion ||
!this.grapherState.relatedQuestions ||
!this.grapherState.relatedQuestions.length
this.hideRelatedQuestion ||
!this.relatedQuestions ||
!this.relatedQuestions.length
)
return false
const question = this.grapherState.relatedQuestions[0]
const question = this.relatedQuestions[0]
return !!question && !!question.text && !!question.url
}

Expand All @@ -3262,8 +3228,7 @@ export class Grapher extends React.Component<GrapherProps> {
// "ourworldindata.org" and yet should still yield a match.
// - Note that this won't work on production previews (where the
// path is /admin/posts/preview/ID)
const { relatedQuestions = [] } = this.grapherState
const { hasRelatedQuestion } = this
const { relatedQuestions = [], hasRelatedQuestion } = this
const relatedQuestion = relatedQuestions[0]
return (
hasRelatedQuestion &&
Expand All @@ -3275,31 +3240,54 @@ export class Grapher extends React.Component<GrapherProps> {

@computed get showRelatedQuestion(): boolean {
return (
!!this.grapherState.relatedQuestions &&
!!this.relatedQuestions &&
!!this.hasRelatedQuestion &&
!!this.isRelatedQuestionTargetDifferentFromCurrentPage
)
}

@computed get isOnCanonicalUrl(): boolean {
if (!this.grapherState.canonicalUrl) return false
if (!this.canonicalUrl) return false
return (
getWindowUrl().pathname ===
Url.fromURL(this.grapherState.canonicalUrl).pathname
getWindowUrl().pathname === Url.fromURL(this.canonicalUrl).pathname
)
}

@computed get showEntitySelectionToggle(): boolean {
return (
!this.grapherState.hideEntityControls &&
this.grapherState.canChangeAddOrHighlightEntities &&
this.grapherState.isOnChartTab &&
(this.grapherState.showEntitySelectorAs ===
GrapherWindowType.modal ||
this.grapherState.showEntitySelectorAs ===
GrapherWindowType.drawer)
!this.hideEntityControls &&
this.canChangeAddOrHighlightEntities &&
this.isOnChartTab &&
(this.showEntitySelectorAs === GrapherWindowType.modal ||
this.showEntitySelectorAs === GrapherWindowType.drawer)
)
}
}

export interface GrapherProps {
grapherState: GrapherState
}

@observer
export class Grapher extends React.Component<GrapherProps> {
@computed get grapherState(): GrapherState {
return this.props.grapherState
}

// #region Observable props not in any interface

analytics = new GrapherAnalytics(
this.props.grapherState.initialOptions.env ?? ""
)

// stored on Grapher so state is preserved when switching to full-screen mode

@observable
private legacyVariableDataJson?: MultipleOwidVariableDataDimensionsMap
private hasLoggedGAViewEvent = false
@observable private hasBeenVisible = false
@observable private uncaughtError?: Error
@observable slideShow?: SlideShowController<any>

constructor(props: { grapherState: GrapherState }) {
super(props)
Expand Down Expand Up @@ -3740,8 +3728,8 @@ export class Grapher extends React.Component<GrapherProps> {
{/* captioned chart and entity selector */}
<div className="CaptionedChartAndSidePanel">
<CaptionedChart manager={this.grapherState} />
{this.sidePanelBounds && (
<SidePanel bounds={this.sidePanelBounds}>
{this.grapherState.sidePanelBounds && (
<SidePanel bounds={this.grapherState.sidePanelBounds}>
<EntitySelector manager={this.grapherState} />
</SidePanel>
)}
Expand Down Expand Up @@ -3847,7 +3835,7 @@ export class Grapher extends React.Component<GrapherProps> {
},
{ threshold: [0, 0.66] }
)
observer.observe(this.containerElement!)
observer.observe(this.grapherState.containerElement!)
this.grapherState.disposers.push(() => observer.disconnect())
} else {
// IntersectionObserver not available; we may be in a Node environment, just render
Expand Down

0 comments on commit 2d53d5a

Please sign in to comment.