From df7e5457d8269d793622edb137e02728f5e203d4 Mon Sep 17 00:00:00 2001 From: Sophia Mersmann Date: Fri, 3 May 2024 10:07:39 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20(entity=20selector)=20render=20into?= =?UTF-8?q?=20panel,=20drawer=20or=20modal=20(#3373)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Cycle 2024.2: Entity selector](https://github.com/owid/owid-grapher/issues/3349) | [Designs](https://www.figma.com/file/X5mOEX8zULS6qyHocUYdmh/Grapher-UI?type=design&node-id=2523%3A6266&mode=design&t=7edFp79OOjz6RENz-1) ## Summary Renders the entity selector either into... - a side panel next to the chart - a drawer that slides in from the right - or a modal The entity selector itself hasn't been touched but will be redesigned in subsequent PRs.
Screenshots

![Screenshot 2024-03-21 at 10 10 14](https://github.com/owid/owid-grapher/assets/12461810/de7bc544-15c8-4d61-a92b-718b4d3431f6) ![Screenshot 2024-03-21 at 10 10 42](https://github.com/owid/owid-grapher/assets/12461810/59829930-401c-47e3-b5fc-4b7ede6fbc58) ![Screenshot 2024-03-21 at 10 11 25](https://github.com/owid/owid-grapher/assets/12461810/75a810e8-7e5d-4e95-b03f-858759d4394d)

## Details - Bounds (see screenshot below) - `frameBounds` are the bounds of the whole Grapher frame (including the side panel if it exists) - `captionedChartBounds` are the bounds of the `` component that renders the chart itself and its header and footer - `sidePanelBounds` are the bounds of the side panel (if present) - New components - `EntitySelector` has been broken out of `EntitySelectorModal` so that we can render it into different spaces - `SidePanel` and `SlideInDrawer` are both utility components that don't know anything about the content they render - `SlideInDrawer` renders a drawer outside of Grapher that slides in on request (it works exactly like the slide-in drawer for the settings menu used to work) - `SidePanel` renders a panel to the right of the chart
Screenshot

![Screenshot 2024-03-21 at 10 56 40](https://github.com/owid/owid-grapher/assets/12461810/f2dff3c6-2e9b-4bb9-9ee2-0954826fb9b5)

## Caveats - Ideal bounds: - Grapher uses ideal bounds on Grapher pages - If the side panel is visible, then the ideal bounds should apply to the captioned chart, not the whole frame, since we care about the aspect ratio of the chart - Making it so that the `captionedChartBounds` are ideal (rather than the `frameBounds`) makes it more difficult to mirror that behaviour in CSS - Since making this work is not trivial, and in theory the design suggests that Grapher should be rendered into a 12-column grid anyway, I decided to look into sizing on Grapher pages at the end of the project ## Notes for the reviewer - It looks like a big PR, but most of it comes down to moving code around - I didn't spend any time making the current (old) design work when rendered into the side panel or drawer since it will be redesigned in subsequent PRs - There is no need to thoroughly review the CSS in particular (it's mostly just copy-pasted from other places or will change in the near future) - I removed the behaviour where clicking on an entity name opened the entity selector (it was difficult to discover for users and also inconsistent across charts (this only worked for line charts and stacked area charts)) - That's also the reason that the SVG tester fails for all line charts and all stacked area charts --- adminSiteClient/ChartEditorPage.tsx | 2 +- baker/GrapherImageBaker.tsx | 4 +- devTools/svgTester/utils.ts | 2 +- .../grapher/src/bodyPortal/.eslintrc.yaml | 3 + .../grapher/src/bodyPortal/BodyPortal.tsx | 32 ++ .../src/captionedChart/CaptionedChart.scss | 5 +- .../captionedChart/CaptionedChart.stories.tsx | 2 +- .../src/captionedChart/CaptionedChart.tsx | 88 +++-- .../grapher/src/chart/ChartManager.ts | 2 - .../src/controls/EntitySelectionToggle.tsx | 47 +-- .../grapher/src/controls/SettingsMenu.tsx | 16 +- .../grapher/src/core/Grapher.tsx | 311 ++++++++++++------ .../grapher/src/core/GrapherConstants.ts | 2 + .../grapher/src/core/grapher.scss | 9 +- .../grapher/src/dataTable/DataTable.tsx | 6 +- .../src/entitySelector/EntitySelector.scss | 175 ++++++++++ .../src/entitySelector/EntitySelector.tsx | 221 +++++++++++++ .../grapher/src/fullScreen/FullScreen.tsx | 4 + .../grapher/src/header/HeaderManager.ts | 2 +- packages/@ourworldindata/grapher/src/index.ts | 1 + .../grapher/src/lineCharts/LineChart.tsx | 4 - .../grapher/src/lineLegend/LineLegend.tsx | 11 +- .../grapher/src/modal/DownloadModal.tsx | 39 ++- .../grapher/src/modal/EmbedModal.tsx | 10 +- .../src/modal/EntitySelectorModal.scss | 179 ---------- .../grapher/src/modal/EntitySelectorModal.tsx | 253 ++------------ .../grapher/src/modal/SourcesModal.tsx | 10 +- .../src/noDataModal/NoDataModal.stories.tsx | 2 +- .../grapher/src/noDataModal/NoDataModal.tsx | 4 +- .../grapher/src/sidePanel/SidePanel.scss | 15 + .../grapher/src/sidePanel/SidePanel.tsx | 32 ++ .../src/slideInDrawer/SlideInDrawer.scss | 137 ++++++++ .../src/slideInDrawer/SlideInDrawer.tsx | 114 +++++++ .../src/stackedCharts/StackedAreaChart.tsx | 4 - .../types/src/grapherTypes/GrapherTypes.ts | 6 + packages/@ourworldindata/types/src/index.ts | 1 + site/DataPageV2.tsx | 2 + site/GrapherPage.tsx | 2 + 38 files changed, 1127 insertions(+), 632 deletions(-) create mode 100644 packages/@ourworldindata/grapher/src/bodyPortal/.eslintrc.yaml create mode 100644 packages/@ourworldindata/grapher/src/bodyPortal/BodyPortal.tsx create mode 100644 packages/@ourworldindata/grapher/src/entitySelector/EntitySelector.scss create mode 100644 packages/@ourworldindata/grapher/src/entitySelector/EntitySelector.tsx delete mode 100644 packages/@ourworldindata/grapher/src/modal/EntitySelectorModal.scss create mode 100644 packages/@ourworldindata/grapher/src/sidePanel/SidePanel.scss create mode 100644 packages/@ourworldindata/grapher/src/sidePanel/SidePanel.tsx create mode 100644 packages/@ourworldindata/grapher/src/slideInDrawer/SlideInDrawer.scss create mode 100644 packages/@ourworldindata/grapher/src/slideInDrawer/SlideInDrawer.tsx diff --git a/adminSiteClient/ChartEditorPage.tsx b/adminSiteClient/ChartEditorPage.tsx index 502df473cd8..4b35d41382d 100644 --- a/adminSiteClient/ChartEditorPage.tsx +++ b/adminSiteClient/ChartEditorPage.tsx @@ -249,7 +249,7 @@ export class ChartEditorPage @computed private get bounds(): Bounds { return this.isMobilePreview ? new Bounds(0, 0, 380, 525) - : this.grapher.idealBounds + : this.grapher.defaultBounds } @computed private get staticFormat(): GrapherStaticFormat { diff --git a/baker/GrapherImageBaker.tsx b/baker/GrapherImageBaker.tsx index a8232510b88..173ae4449df 100644 --- a/baker/GrapherImageBaker.tsx +++ b/baker/GrapherImageBaker.tsx @@ -46,7 +46,7 @@ export async function bakeGraphersToPngs( .then(() => console.log(`${outPath}.svg`)), sharp(Buffer.from(grapher.staticSVG), { density: 144 }) .png() - .resize(grapher.idealBounds.width, grapher.idealBounds.height) + .resize(grapher.defaultBounds.width, grapher.defaultBounds.height) .flatten({ background: "#ffffff" }) .toFile(`${outPath}.png`), ]) @@ -102,7 +102,7 @@ export async function bakeGrapherToSvg( verbose = true ) { const grapher = initGrapherForSvgExport(jsonConfig, queryStr) - const { width, height } = grapher.idealBounds + const { width, height } = grapher.defaultBounds const outPath = buildSvgOutFilepath( outDir, { diff --git a/devTools/svgTester/utils.ts b/devTools/svgTester/utils.ts index 26c43e473c7..83207632ef8 100644 --- a/devTools/svgTester/utils.ts +++ b/devTools/svgTester/utils.ts @@ -401,7 +401,7 @@ export async function renderSvg( }, queryStr ) - const { width, height } = grapher.idealBounds + const { width, height } = grapher.defaultBounds const outFilename = buildSvgOutFilename( { slug: configAndData.config.slug!, diff --git a/packages/@ourworldindata/grapher/src/bodyPortal/.eslintrc.yaml b/packages/@ourworldindata/grapher/src/bodyPortal/.eslintrc.yaml new file mode 100644 index 00000000000..2972fb8a9d9 --- /dev/null +++ b/packages/@ourworldindata/grapher/src/bodyPortal/.eslintrc.yaml @@ -0,0 +1,3 @@ +rules: + "@typescript-eslint/explicit-function-return-type": "warn" + "@typescript-eslint/explicit-module-boundary-types": "warn" diff --git a/packages/@ourworldindata/grapher/src/bodyPortal/BodyPortal.tsx b/packages/@ourworldindata/grapher/src/bodyPortal/BodyPortal.tsx new file mode 100644 index 00000000000..8153c14a07f --- /dev/null +++ b/packages/@ourworldindata/grapher/src/bodyPortal/BodyPortal.tsx @@ -0,0 +1,32 @@ +import React from "react" +import ReactDOM from "react-dom" + +interface BodyPortalProps { + id?: string + tagName?: string // default: "div" + children: React.ReactNode +} + +// Render a component on the Body instead of inside the current Tree. +// https://reactjs.org/docs/portals.html +export class BodyPortal extends React.Component { + el: HTMLElement + + constructor(props: BodyPortalProps) { + super(props) + this.el = document.createElement(props.tagName || "div") + if (props.id) this.el.id = props.id + } + + componentDidMount(): void { + document.body.appendChild(this.el) + } + + componentWillUnmount(): void { + document.body.removeChild(this.el) + } + + render(): any { + return ReactDOM.createPortal(this.props.children, this.el) + } +} diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.scss b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.scss index b44d634451b..06bd9ea88a6 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.scss +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.scss @@ -1,6 +1,10 @@ // keep in sync with constant values in CaptionedChart.tsx $controlRowHeight: 32px; // keep in sync with CONTROLS_ROW_HEIGHT +.CaptionedChart { + width: 100%; +} + .HeaderHTML, .SourcesFooterHTML { font-family: $sans-serif-font-stack; @@ -22,7 +26,6 @@ $controlRowHeight: 32px; // keep in sync with CONTROLS_ROW_HEIGHT align-items: center; border-top: 1px solid $frame-color; position: absolute; - width: 100%; bottom: 0; color: $dark-text; font-weight: 700; diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx index c0522eda421..b832d6b9ae3 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.stories.tsx @@ -21,7 +21,7 @@ export default { const table = SynthesizeGDPTable({ entityCount: 5 }) const manager: CaptionedChartManager = { - tabBounds: DEFAULT_BOUNDS, + captionedChartBounds: DEFAULT_BOUNDS, table, selection: table.availableEntityNames, currentTitle: "This is the Title", diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx index c73851bcb8b..7a05b4a79be 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx @@ -45,7 +45,6 @@ import { GrapherTabOption, RelatedQuestionsConfig, } from "@ourworldindata/types" -import { AxisConfig } from "../axis/AxisConfig" import { DataTable, DataTableManager } from "../dataTable/DataTable" import { ContentSwitchers, @@ -68,33 +67,42 @@ export interface CaptionedChartManager MapProjectionMenuManager, SettingsMenuManager { containerElement?: HTMLDivElement - tabBounds?: Bounds + bakedGrapherURL?: string + isReady?: boolean + whatAreWeWaitingFor?: string + + // bounds + captionedChartBounds?: Bounds + sidePanelBounds?: Bounds staticBounds?: Bounds staticBoundsWithDetails?: Bounds + + // layout + isSmall?: boolean + isMedium?: boolean + framePaddingHorizontal?: number + framePaddingVertical?: number fontSize?: number - bakedGrapherURL?: string + + // state tab?: GrapherTabOption - type: ChartTypeName - yAxis: AxisConfig - xAxis: AxisConfig - typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart?: ChartTypeName - isReady?: boolean - whatAreWeWaitingFor?: string - entityType?: string - entityTypePlural?: string - shouldIncludeDetailsInStaticExport?: boolean - detailRenderers: MarkdownTextWrap[] isOnMapTab?: boolean isOnTableTab?: boolean + type: ChartTypeName + typeExceptWhenLineChartAndSingleTimeThenWillBeBarChart?: ChartTypeName + showEntitySelectionToggle?: boolean + + // timeline hasTimeline?: boolean timelineController?: TimelineController - hasRelatedQuestion?: boolean - isRelatedQuestionTargetDifferentFromCurrentPage?: boolean + + // details on demand + shouldIncludeDetailsInStaticExport?: boolean + detailRenderers: MarkdownTextWrap[] + + // related question relatedQuestions?: RelatedQuestionsConfig[] - isSmall?: boolean - isMedium?: boolean - framePaddingHorizontal?: number - framePaddingVertical?: number + showRelatedQuestion?: boolean } interface CaptionedChartProps { @@ -144,10 +152,6 @@ export class CaptionedChart extends React.Component { return this.manager.isMedium ? 8 : 16 } - @computed protected get relatedQuestionHeight(): number { - return this.manager.isMedium ? 24 : 28 - } - @computed protected get header(): Header { return new Header({ manager: this.manager, @@ -181,7 +185,9 @@ export class CaptionedChart extends React.Component { @computed protected get bounds(): Bounds { const bounds = - this.props.bounds ?? this.manager.tabBounds ?? DEFAULT_BOUNDS + this.props.bounds ?? + this.manager.captionedChartBounds ?? + DEFAULT_BOUNDS // the padding ensures grapher's frame is not cut off return bounds.padRight(2).padBottom(2) } @@ -258,28 +264,36 @@ export class CaptionedChart extends React.Component { return this.manager.selection } - @computed get showRelatedQuestion(): boolean { - return ( - !!this.manager.relatedQuestions && - !!this.manager.hasRelatedQuestion && - !!this.manager.isRelatedQuestionTargetDifferentFromCurrentPage - ) + @computed private get showRelatedQuestion(): boolean { + return !!this.manager.showRelatedQuestion + } + + @computed get relatedQuestionHeight(): number { + if (!this.showRelatedQuestion) return 0 + return this.manager.isMedium ? 24 : 28 + } + + @computed private get sidePanelWidth(): number { + return this.manager.sidePanelBounds?.width ?? 0 } private renderControlsRow(): JSX.Element { - const { showContentSwitchers } = this + const { showEntitySelectionToggle } = this.manager return (