diff --git a/adminSiteClient/AbstractChartEditor.ts b/adminSiteClient/AbstractChartEditor.ts index 4db1c70e3d8..c22baa71c8a 100644 --- a/adminSiteClient/AbstractChartEditor.ts +++ b/adminSiteClient/AbstractChartEditor.ts @@ -5,6 +5,8 @@ import { diffGrapherConfigs, mergeGrapherConfigs, PostReference, + SeriesName, + difference, } from "@ourworldindata/utils" import { action, computed, observable, when } from "mobx" import { EditorFeatures } from "./EditorFeatures.js" @@ -163,6 +165,24 @@ export abstract class AbstractChartEditor< return Object.hasOwn(this.activeParentConfig, property) } + @computed get invalidFocusedSeriesNames(): SeriesName[] { + const { grapher } = this + + // if focusing is not supported, then all focused series are invalid + if (!this.features.canHighlightSeries) { + return grapher.focusArray.seriesNames + } + + // find invalid focused series + const availableSeriesNames = grapher.chartSeriesNames + const focusedSeriesNames = grapher.focusArray.seriesNames + return difference(focusedSeriesNames, availableSeriesNames) + } + + @action.bound removeInvalidFocusedSeriesNames(): void { + this.grapher.focusArray.remove(...this.invalidFocusedSeriesNames) + } + abstract get isNewGrapher(): boolean abstract get availableTabs(): EditorTab[] diff --git a/adminSiteClient/ChartEditorTypes.ts b/adminSiteClient/ChartEditorTypes.ts index d1d6f7ffd12..966741c5861 100644 --- a/adminSiteClient/ChartEditorTypes.ts +++ b/adminSiteClient/ChartEditorTypes.ts @@ -6,13 +6,8 @@ export type FieldWithDetailReferences = | "axisLabelX" | "axisLabelY" -export interface DimensionErrorMessage { - displayName?: string -} +type ErrorMessageFieldName = FieldWithDetailReferences | "focusedSeriesNames" -export type ErrorMessages = Partial> +export type ErrorMessages = Partial> -export type ErrorMessagesForDimensions = Record< - DimensionProperty, - DimensionErrorMessage[] -> +export type ErrorMessagesForDimensions = Record diff --git a/adminSiteClient/ChartEditorView.tsx b/adminSiteClient/ChartEditorView.tsx index 3a23203cf83..2cd3005e101 100644 --- a/adminSiteClient/ChartEditorView.tsx +++ b/adminSiteClient/ChartEditorView.tsx @@ -265,6 +265,14 @@ export class ChartEditorView< } ) + // add an error message if any focused series names are invalid + const { invalidFocusedSeriesNames = [] } = this.editor ?? {} + if (invalidFocusedSeriesNames.length > 0) { + const invalidNames = invalidFocusedSeriesNames.join(", ") + const message = `Invalid focus state. The following entities/indicators are not plotted: ${invalidNames}` + errorMessages.focusedSeriesNames = message + } + return errorMessages } @@ -287,9 +295,8 @@ export class ChartEditorView< // add error message if details are referenced in the display name if (hasDetailsInDisplayName) { - errorMessages[slot.property][dimensionIndex] = { - displayName: "Detail syntax is not supported", - } + errorMessages[slot.property][dimensionIndex] = + `Detail syntax is not supported for display names of indicators: ${dimension.display.name}` } }) }) diff --git a/adminSiteClient/DimensionCard.tsx b/adminSiteClient/DimensionCard.tsx index 74ff1cdad01..4053be981d3 100644 --- a/adminSiteClient/DimensionCard.tsx +++ b/adminSiteClient/DimensionCard.tsx @@ -4,7 +4,6 @@ import { observer } from "mobx-react" import { ChartDimension } from "@ourworldindata/grapher" import { OwidColumnDef, OwidVariableRoundingMode } from "@ourworldindata/types" import { startCase } from "@ourworldindata/utils" -import { DimensionErrorMessage } from "./ChartEditorTypes.js" import { Toggle, BindAutoString, @@ -35,7 +34,7 @@ export class DimensionCard< onChange: (dimension: ChartDimension) => void onEdit?: () => void onRemove?: () => void - errorMessage?: DimensionErrorMessage + errorMessage?: string }> { @observable.ref isExpanded: boolean = false @@ -171,7 +170,7 @@ export class DimensionCard< store={dimension.display} auto={column.displayName} onBlur={this.onChange} - errorMessage={this.props.errorMessage?.displayName} + errorMessage={this.props.errorMessage} /> this.grapher.validChartTypes, () => { this.updateDefaultSelection() - this.validateFocusedSeriesNames() + this.editor.removeInvalidFocusedSeriesNames() } ), reaction( () => this.grapher.yColumnsFromDimensions.length, () => { this.updateDefaultSelection() - this.validateFocusedSeriesNames() + this.editor.removeInvalidFocusedSeriesNames() } ) ) @@ -531,12 +518,3 @@ function IndicatorChartInfo(props: { editor: IndicatorChartEditor }) { ) } - -export function findInvalidFocusedSeriesNames(grapher: Grapher): SeriesName[] { - const availableSeriesNames = new Set(grapher.chartSeriesNames) - const focusedSeriesNames = grapher.focusArray.seriesNameSet - - return Array.from( - differenceOfSets([focusedSeriesNames, availableSeriesNames]) - ) -} diff --git a/adminSiteClient/EditorDataTab.tsx b/adminSiteClient/EditorDataTab.tsx index c508ac8fdeb..84c15f868b6 100644 --- a/adminSiteClient/EditorDataTab.tsx +++ b/adminSiteClient/EditorDataTab.tsx @@ -30,7 +30,6 @@ import { DropResult, } from "react-beautiful-dnd" import { AbstractChartEditor } from "./AbstractChartEditor.js" -import { findInvalidFocusedSeriesNames } from "./EditorBasicTab.js" interface EntityListItemProps extends React.HTMLProps { grapher: Grapher @@ -143,12 +142,12 @@ export class EntitySelectionSection extends React.Component<{ @action.bound onAddKey(entityName: EntityName) { this.editor.grapher.selection.selectEntity(entityName) - this.syncFocusedSeriesNames() + this.editor.removeInvalidFocusedSeriesNames() } @action.bound onRemoveKey(entityName: EntityName) { this.editor.grapher.selection.deselectEntity(entityName) - this.syncFocusedSeriesNames() + this.editor.removeInvalidFocusedSeriesNames() } @action.bound onDragEnd(result: DropResult) { @@ -171,12 +170,7 @@ export class EntitySelectionSection extends React.Component<{ grapher.selection.setSelectedEntities( activeParentConfig.selectedEntityNames ) - } - - @action.bound private syncFocusedSeriesNames(): void { - const { grapher } = this.props.editor - const invalidFocusedSeriesNames = findInvalidFocusedSeriesNames(grapher) - grapher.focusArray.remove(...invalidFocusedSeriesNames) + this.editor.removeInvalidFocusedSeriesNames() } render() { @@ -287,10 +281,23 @@ export class FocusSection extends React.Component<{ this.editor.grapher.focusArray.remove(seriesName) } + @action.bound setFocusedSeriesNamesToParentValue() { + const { grapher, activeParentConfig } = this.editor + if (!activeParentConfig || !activeParentConfig.focusedSeriesNames) + return + grapher.focusArray.clearAllAndAdd( + ...activeParentConfig.focusedSeriesNames + ) + this.editor.removeInvalidFocusedSeriesNames() + } + render() { const { editor } = this const { grapher } = editor + const isFocusInherited = + editor.isPropertyInherited("focusedSeriesNames") + const focusedSeriesNameSet = grapher.focusArray.seriesNameSet const focusedSeriesNames = grapher.focusArray.seriesNames @@ -300,13 +307,19 @@ export class FocusSection extends React.Component<{ seriesNameSet, focusedSeriesNameSet, ]) + + // focusing only makes sense for two or more plotted series + if (focusedSeriesNameSet.size === 0 && availableSeriesNameSet.size < 2) + return null + const availableSeriesNames: SeriesName[] = sortBy( Array.from(availableSeriesNameSet) ) - const invalidFocusedSeriesNames = new Set( - findInvalidFocusedSeriesNames(grapher) - ) + const invalidFocusedSeriesNames = differenceOfSets([ + focusedSeriesNameSet, + seriesNameSet, + ]) return (
@@ -318,6 +331,20 @@ export class FocusSection extends React.Component<{ .concat(availableSeriesNames) .map((key) => ({ value: key }))} /> + {editor.couldPropertyBeInherited("focusedSeriesNames") && ( + + )} {focusedSeriesNames.map((seriesName) => (
- + {features.canHighlightSeries && ( + + )} {features.canSpecifyMissingDataStrategy && ( )} diff --git a/adminSiteClient/EditorExportTab.tsx b/adminSiteClient/EditorExportTab.tsx index dc2d47a8cb6..84fa7bac40a 100644 --- a/adminSiteClient/EditorExportTab.tsx +++ b/adminSiteClient/EditorExportTab.tsx @@ -211,6 +211,7 @@ export class EditorExportTab< staticFormat: format, selectedEntityNames: this.grapher.selection.selectedEntityNames, + focusedSeriesNames: this.grapher.focusedSeriesNames, isSocialMediaExport, }) } diff --git a/adminSiteClient/EditorFeatures.tsx b/adminSiteClient/EditorFeatures.tsx index 0fefaa9c693..243f0a84ffb 100644 --- a/adminSiteClient/EditorFeatures.tsx +++ b/adminSiteClient/EditorFeatures.tsx @@ -156,4 +156,11 @@ export class EditorFeatures { ) ) } + + @computed get canHighlightSeries() { + return ( + (this.grapher.hasLineChart || this.grapher.hasSlopeChart) && + this.grapher.isOnChartTab + ) + } } diff --git a/adminSiteClient/SaveButtons.tsx b/adminSiteClient/SaveButtons.tsx index fe43cbd95f9..dbb9ed71714 100644 --- a/adminSiteClient/SaveButtons.tsx +++ b/adminSiteClient/SaveButtons.tsx @@ -2,7 +2,7 @@ import React from "react" import { ChartEditor, isChartEditorInstance } from "./ChartEditor.js" import { action, computed } from "mobx" import { observer } from "mobx-react" -import { isEmpty, omit } from "@ourworldindata/utils" +import { excludeUndefined, omit } from "@ourworldindata/utils" import { IndicatorChartEditor, isIndicatorChartEditorInstance, @@ -73,22 +73,20 @@ class SaveButtonsForChart extends React.Component<{ else this.props.editor.publishGrapher() } - @computed get hasEditingErrors(): boolean { + @computed get editingErrors(): string[] { const { errorMessages, errorMessagesForDimensions } = this.props - - if (!isEmpty(errorMessages)) return true - - const allErrorMessagesForDimensions = Object.values( - errorMessagesForDimensions - ).flat() - return allErrorMessagesForDimensions.some((error) => error) + return excludeUndefined([ + ...Object.values(errorMessages), + ...Object.values(errorMessagesForDimensions).flat(), + ]) } render() { - const { hasEditingErrors } = this + const { editingErrors } = this const { editor } = this.props const { grapher } = editor + const hasEditingErrors = editingErrors.length > 0 const isSavingDisabled = grapher.hasFatalErrors || hasEditingErrors return ( @@ -125,11 +123,17 @@ class SaveButtonsForChart extends React.Component<{ )} + {editingErrors.map((error, i) => ( +
+ {error} +
+ ))} ) } @@ -145,23 +149,21 @@ class SaveButtonsForIndicatorChart extends React.Component<{ void this.props.editor.saveGrapher() } - @computed get hasEditingErrors(): boolean { + @computed get editingErrors(): string[] { const { errorMessages, errorMessagesForDimensions } = this.props - - if (!isEmpty(errorMessages)) return true - - const allErrorMessagesForDimensions = Object.values( - errorMessagesForDimensions - ).flat() - return allErrorMessagesForDimensions.some((error) => error) + return excludeUndefined([ + ...Object.values(errorMessages), + ...Object.values(errorMessagesForDimensions).flat(), + ]) } render() { - const { hasEditingErrors } = this + const { editingErrors } = this const { editor } = this.props const { grapher } = editor const isTrivial = editor.isNewGrapher && !editor.isModified + const hasEditingErrors = editingErrors.length > 0 const isSavingDisabled = grapher.hasFatalErrors || hasEditingErrors || isTrivial @@ -176,6 +178,11 @@ class SaveButtonsForIndicatorChart extends React.Component<{ ? "Create indicator chart" : "Update indicator chart"} + {editingErrors.map((error, i) => ( +
+ {error} +
+ ))} ) } @@ -191,22 +198,20 @@ class SaveButtonsForChartView extends React.Component<{ void this.props.editor.saveGrapher() } - @computed get hasEditingErrors(): boolean { + @computed get editingErrors(): string[] { const { errorMessages, errorMessagesForDimensions } = this.props - - if (!isEmpty(errorMessages)) return true - - const allErrorMessagesForDimensions = Object.values( - errorMessagesForDimensions - ).flat() - return allErrorMessagesForDimensions.some((error) => error) + return excludeUndefined([ + ...Object.values(errorMessages), + ...Object.values(errorMessagesForDimensions).flat(), + ]) } render() { - const { hasEditingErrors } = this + const { editingErrors } = this const { editor } = this.props const { grapher } = editor + const hasEditingErrors = editingErrors.length > 0 const isSavingDisabled = grapher.hasFatalErrors || hasEditingErrors return ( @@ -226,6 +231,11 @@ class SaveButtonsForChartView extends React.Component<{ > Go to parent chart + {editingErrors.map((error, i) => ( +
+ {error} +
+ ))} ) } diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx index df10e98a56a..babbce9753e 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx @@ -34,7 +34,6 @@ import { SelectionArray } from "../selection/SelectionArray" import { EntityName, GRAPHER_CHART_TYPES, - FacetStrategy, RelatedQuestionsConfig, Color, GrapherTabName, @@ -83,6 +82,7 @@ export interface CaptionedChartManager isOnMapTab?: boolean isOnTableTab?: boolean activeChartType?: GrapherChartType + isFaceted?: boolean isLineChartThatTurnedIntoDiscreteBarActive?: boolean showEntitySelectionToggle?: boolean isExportingForSocialMedia?: boolean @@ -185,13 +185,6 @@ export class CaptionedChart extends React.Component { ) } - @computed get isFaceted(): boolean { - const hasStrategy = - !!this.manager.facetStrategy && - this.manager.facetStrategy !== FacetStrategy.none - return !this.manager.isOnMapTab && hasStrategy - } - @computed get activeChartOrMapType(): GrapherChartOrMapType | undefined { const { manager } = this if (manager.isOnTableTab) return undefined @@ -211,6 +204,7 @@ export class CaptionedChart extends React.Component { activeChartOrMapType, containerElement, } = this + const { isFaceted } = manager if (!activeChartOrMapType) return @@ -219,7 +213,7 @@ export class CaptionedChart extends React.Component { activeChartOrMapType !== GRAPHER_MAP_TYPE ? activeChartOrMapType : undefined - if (this.isFaceted && activeChartType) + if (isFaceted && activeChartType) return ( + chartInstance.series.map((series) => series.seriesName) + ) + ) + } + return this.chartInstance.series.map((series) => series.seriesName) } @@ -2805,6 +2817,11 @@ export class Grapher this.selectedFacetStrategy = facet } + @computed get isFaceted(): boolean { + const hasFacetStrategy = this.facetStrategy !== FacetStrategy.none + return !this.isOnMapTab && hasFacetStrategy + } + @action.bound randomSelection(num: number): void { // Continent, Population, GDP PC, GDP, PopDens, UN, Language, etc. this.clearErrors() diff --git a/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx b/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx index d63b7cf9743..457ef06f20e 100644 --- a/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx +++ b/packages/@ourworldindata/grapher/src/facetChart/FacetChart.tsx @@ -347,7 +347,7 @@ export class FacetChart }) } - @computed private get intermediateChartInstances(): ChartInterface[] { + @computed get intermediateChartInstances(): ChartInterface[] { return this.intermediatePlacedSeries.map(({ bounds, manager }) => { const ChartClass = ChartComponentClassMap.get(this.chartTypeName) ??