From 756ec13380df5ede69baf8ac4154e191ac386ac0 Mon Sep 17 00:00:00 2001 From: sophiamersmann Date: Thu, 23 May 2024 18:31:29 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20(grapher)=20prevent=20controls=20fr?= =?UTF-8?q?om=20overlapping?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/captionedChart/CaptionedChart.tsx | 85 ++------ .../src/controls/ContentSwitchers.scss | 16 +- .../grapher/src/controls/ContentSwitchers.tsx | 42 +++- .../grapher/src/controls/LabeledSwitch.scss | 3 + .../src/controls/MapProjectionMenu.scss | 5 +- .../src/controls/MapProjectionMenu.tsx | 11 +- .../grapher/src/controls/SettingsMenu.tsx | 183 +++++++++++------- .../src/controls/controlsRow/ControlsRow.tsx | 117 +++++++++++ .../controls/settings/TableFilterToggle.tsx | 20 +- 9 files changed, 334 insertions(+), 148 deletions(-) create mode 100644 packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx diff --git a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx index 4e7fdc58476..b312886a215 100644 --- a/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx +++ b/packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx @@ -27,15 +27,6 @@ import { LoadingIndicator } from "../loadingIndicator/LoadingIndicator" import { FacetChart } from "../facetChart/FacetChart" import { faExternalLinkAlt } from "@fortawesome/free-solid-svg-icons" import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" -import { - EntitySelectionToggle, - EntitySelectionManager, -} from "../controls/EntitySelectionToggle" -import { - MapProjectionMenu, - MapProjectionMenuManager, -} from "../controls/MapProjectionMenu" -import { SettingsMenu, SettingsMenuManager } from "../controls/SettingsMenu" import { FooterManager } from "../footer/FooterManager" import { HeaderManager } from "../header/HeaderManager" import { SelectionArray } from "../selection/SelectionArray" @@ -47,15 +38,15 @@ import { RelatedQuestionsConfig, } from "@ourworldindata/types" import { DataTable, DataTableManager } from "../dataTable/DataTable" -import { - ContentSwitchers, - ContentSwitchersManager, -} from "../controls/ContentSwitchers" import { TimelineComponent, TIMELINE_HEIGHT, } from "../timeline/TimelineComponent" import { TimelineController } from "../timeline/TimelineController" +import { + ControlsRow, + ControlsRowManager, +} from "../controls/controlsRow/ControlsRow" export interface CaptionedChartManager extends ChartManager, @@ -63,10 +54,7 @@ export interface CaptionedChartManager FooterManager, HeaderManager, DataTableManager, - ContentSwitchersManager, - EntitySelectionManager, - MapProjectionMenuManager, - SettingsMenuManager { + ControlsRowManager { containerElement?: HTMLDivElement bakedGrapherURL?: string isReady?: boolean @@ -207,22 +195,6 @@ export class CaptionedChart extends React.Component { return !this.manager.isOnMapTab && hasStrategy } - @computed get showContentSwitchers(): boolean { - return (this.manager.availableTabs?.length ?? 0) > 1 - } - - @computed get showControls(): boolean { - return ( - SettingsMenu.shouldShow(this.manager) || - EntitySelectionToggle.shouldShow(this.manager) || - MapProjectionMenu.shouldShow(this.manager) - ) - } - - @computed get showControlsRow(): boolean { - return this.showContentSwitchers || this.showControls - } - @computed get chartTypeName(): ChartTypeName { const { manager } = this return this.manager.isOnMapTab @@ -274,44 +246,23 @@ export class CaptionedChart extends React.Component { return this.manager.isMedium ? 24 : 28 } - @computed private get sidePanelWidth(): number { - return this.manager.sidePanelBounds?.width ?? 0 + @computed private get showControlsRow(): boolean { + return ControlsRow.shouldShow(this.manager) } private renderControlsRow(): React.ReactElement { - const { showEntitySelectionToggle } = this.manager return ( - + ) } diff --git a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.scss b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.scss index dbf0428ae37..c168984adf5 100644 --- a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.scss +++ b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.scss @@ -1,4 +1,10 @@ .ContentSwitchers { + // keep in sync with variables in ContentSwitchers.tsx + $font-size: 13px; + $icon-width: 13px; + $icon-padding: 6px; + --outer-padding: 16px; + $light-stroke: #e7e7e7; $hover-fill: #f2f2f2; @@ -30,12 +36,12 @@ display: block; text-transform: capitalize; color: $light-text; - font-size: 13px; + font-size: $font-size; font-weight: 500; height: $height; line-height: $height; border-radius: $border-radius - $visual-gap; - padding: 0 16px; + padding: 0 var(--outer-padding); cursor: default; letter-spacing: 0.01em; white-space: nowrap; @@ -47,14 +53,14 @@ } .label { - margin-left: 6px; + margin-left: $icon-padding; } svg { color: $info-icon; &.custom-icon { - --size: 13px; + --size: $icon-width; display: inline-block; height: var(--size); @@ -106,6 +112,6 @@ &.GrapherComponentMedium { .ContentSwitchers:not(.iconOnly) li > button { - padding: 0 8px; + --outer-padding: 8px; } } diff --git a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx index ea4b5afca74..6e4b4e891da 100644 --- a/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx +++ b/packages/@ourworldindata/grapher/src/controls/ContentSwitchers.tsx @@ -6,19 +6,36 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome/index.js" import { faTable, faEarthAmericas } from "@fortawesome/free-solid-svg-icons" import { ChartTypeName, GrapherTabOption } from "@ourworldindata/types" import { chartIcons } from "./ChartIcons" +import { Bounds, capitalize } from "@ourworldindata/utils" export interface ContentSwitchersManager { availableTabs?: GrapherTabOption[] tab?: GrapherTabOption isNarrow?: boolean - type?: ChartTypeName + isMedium?: boolean + type: ChartTypeName isLineChartThatTurnedIntoDiscreteBar?: boolean } +// keep in sync with ContentSwitcher.scss +const TAB_FONT_SIZE = 13 +const ICON_WIDTH = 13 +const ICON_PADDING = 6 + @observer export class ContentSwitchers extends React.Component<{ manager: ContentSwitchersManager }> { + static shouldShow(manager: ContentSwitchersManager): boolean { + const test = new ContentSwitchers({ manager }) + return test.showTabs + } + + static width(manager: ContentSwitchersManager): number { + const test = new ContentSwitchers({ manager }) + return test.width + } + @computed private get manager(): ContentSwitchersManager { return this.props.manager } @@ -27,6 +44,10 @@ export class ContentSwitchers extends React.Component<{ return this.manager.availableTabs || [] } + @computed private get showTabs(): boolean { + return this.availableTabs.length > 1 + } + @computed private get showTabLabels(): boolean { return !this.manager.isNarrow } @@ -35,6 +56,25 @@ export class ContentSwitchers extends React.Component<{ return this.manager.type ?? ChartTypeName.LineChart } + @computed get width(): number { + return this.availableTabs.reduce((totalWidth, tab) => { + // keep in sync with ContentSwitcher.scss + const outerPadding = + this.showTabLabels && this.manager.isMedium ? 8 : 16 + + let tabWidth = 2 * outerPadding + ICON_WIDTH + + if (this.showTabLabels) { + const labelWidth = Bounds.forText(capitalize(tab), { + fontSize: TAB_FONT_SIZE, + }).width + tabWidth += labelWidth + ICON_PADDING + } + + return totalWidth + tabWidth + }, 0) + } + private tabIcon(tab: GrapherTabOption): React.ReactElement { const { manager } = this switch (tab) { diff --git a/packages/@ourworldindata/grapher/src/controls/LabeledSwitch.scss b/packages/@ourworldindata/grapher/src/controls/LabeledSwitch.scss index 3d7bbb43de1..4096841ada7 100644 --- a/packages/@ourworldindata/grapher/src/controls/LabeledSwitch.scss +++ b/packages/@ourworldindata/grapher/src/controls/LabeledSwitch.scss @@ -13,6 +13,9 @@ $lato: $sans-serif-font-stack; .settings-menu-contents { // on/off switch with label written to the right .labeled-switch { + // keep in sync with TableFilterToggle.tsx + // where the width of a labeled switch is calculated + display: flex; color: $light-text; font: $medium 13px/16px $lato; diff --git a/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.scss b/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.scss index 89775785f36..ee6102f111b 100644 --- a/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.scss +++ b/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.scss @@ -1,6 +1,9 @@ .map-projection-menu { - .control { + width: 150px; + + .menu { min-width: 150px; + right: 0; } .menu:before { diff --git a/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.tsx b/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.tsx index 2e9b5a2e491..579d74dcf8f 100644 --- a/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.tsx +++ b/packages/@ourworldindata/grapher/src/controls/MapProjectionMenu.tsx @@ -5,6 +5,7 @@ import { MapConfig } from "../mapCharts/MapConfig" import { MapProjectionName } from "@ourworldindata/types" import { MapProjectionLabels } from "../mapCharts/MapProjections" import { Dropdown } from "./Dropdown" +import { DEFAULT_BOUNDS } from "@ourworldindata/utils" export { AbsRelToggle } from "./settings/AbsRelToggle" export { FacetStrategySelector } from "./settings/FacetStrategySelector" @@ -27,6 +28,7 @@ interface MapProjectionMenuItem { @observer export class MapProjectionMenu extends React.Component<{ manager: MapProjectionMenuManager + maxWidth?: number }> { static shouldShow(manager: MapProjectionMenuManager): boolean { const menu = new MapProjectionMenu({ manager }) @@ -40,6 +42,10 @@ export class MapProjectionMenu extends React.Component<{ return !hideMapProjectionMenu && !!(isOnMapTab && projection) } + @computed private get maxWidth(): number { + return this.props.maxWidth ?? DEFAULT_BOUNDS.width + } + @action.bound onChange(selected: unknown): void { const { mapConfig } = this.props.manager if (selected && mapConfig) @@ -62,7 +68,10 @@ export class MapProjectionMenu extends React.Component<{ render(): React.ReactElement | null { return this.showMenu ? ( -
+
this.maxWidth } - @computed get menu(): React.ReactElement | void { - if (this.active) { - return this.menuContents - } + @computed get layout(): { + maxHeight: string + maxWidth: string + top: number + right: number + } { + const { top, bottom, right } = this.props, + maxHeight = `calc(100% - ${top + bottom}px)`, + maxWidth = `calc(100% - ${2 * right}px)` + return { maxHeight, maxWidth, top, right } } - @computed get menuContents(): React.ReactElement { + @computed get menuContentsChart(): React.ReactElement { const { manager, - chartType, showYScaleToggle, showXScaleToggle, showZoomToggle, @@ -288,7 +299,6 @@ export class SettingsMenu extends React.Component<{ xAxis, // compareEndPointsOnly, filledDimensions, - isOnTableTab, isOnChartTab, } = manager @@ -301,6 +311,83 @@ export class SettingsMenu extends React.Component<{ omitLoneAxisLabel = showYScaleToggle && !showXScaleToggle && yLabel === "Y axis" + return ( + <> + + {showFacetControl && ( + + )} + {showFacetYDomainToggle && ( + + )} + {showAbsRelToggle && } + {showNoDataAreaToggle && ( + + )} + {showZoomToggle && } + + + {showYScaleToggle && ( + + )} + {showXScaleToggle && ( + + )} +
+ A linear scale evenly spaces values, where each + increment represents a consistent change. A logarithmic + scale uses multiples of the starting value, with each + increment representing the same percentage increase. +
+ + + ) + } + + @computed get menuContentsTable(): JSX.Element { + const subtitle = `Only display table rows for ${ + this.manager.entityTypePlural ?? DEFAULT_GRAPHER_ENTITY_TYPE_PLURAL + } selected within the chart` + + return ( + + + + ) + } + + @computed get menu(): JSX.Element | void { + if (this.active) { + return this.menuContents + } + } + + @computed get menuContents(): JSX.Element { + const { manager, chartType } = this + const { isOnTableTab } = manager + const menuTitle = `${isOnTableTab ? "Table" : chartType} settings` return ( @@ -320,67 +407,17 @@ export class SettingsMenu extends React.Component<{ title={menuTitle} onDismiss={this.toggleVisibility} /> -
- - {showFacetControl && ( - - )} - {showFacetYDomainToggle && ( - - )} - {showAbsRelToggle && ( - - )} - {showNoDataAreaToggle && ( - - )} - {showZoomToggle && } - - - {showYScaleToggle && ( - - )} - {showXScaleToggle && ( - - )} -
- A linear scale evenly spaces values, where each - increment represents a consistent change. A - logarithmic scale uses multiples of the starting - value, with each increment representing the same - percentage increase. -
- + {isOnTableTab + ? this.menuContentsTable + : this.menuContentsChart}
) } - renderChartSettings(): React.ReactElement { + renderSettingsButtonAndPopup(): JSX.Element { const { active } = this return (
@@ -403,7 +440,7 @@ export class SettingsMenu extends React.Component<{ renderTableControls(): React.ReactElement { // Since tables only have a single control, display it inline rather than // placing it in the settings menu - return + return } render(): React.ReactElement | null { @@ -413,11 +450,15 @@ export class SettingsMenu extends React.Component<{ showTableFilterToggle, } = this - return isOnTableTab && showTableFilterToggle - ? this.renderTableControls() - : isOnChartTab && showSettingsMenuToggle - ? this.renderChartSettings() - : null + if (isOnTableTab && showTableFilterToggle) { + return this.shouldRenderTableControlsIntoPopup + ? this.renderSettingsButtonAndPopup() + : this.renderTableControls() + } + + return isOnChartTab && showSettingsMenuToggle + ? this.renderSettingsButtonAndPopup() + : null } } diff --git a/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx b/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx new file mode 100644 index 00000000000..0ae2db8b681 --- /dev/null +++ b/packages/@ourworldindata/grapher/src/controls/controlsRow/ControlsRow.tsx @@ -0,0 +1,117 @@ +import React from "react" +import { computed } from "mobx" +import { observer } from "mobx-react" + +import { Bounds, DEFAULT_BOUNDS, GrapherTabOption } from "@ourworldindata/utils" + +import { ContentSwitchers, ContentSwitchersManager } from "../ContentSwitchers" +import { + EntitySelectionToggle, + EntitySelectionManager, +} from "../EntitySelectionToggle" +import { + MapProjectionMenu, + MapProjectionMenuManager, +} from "../MapProjectionMenu" +import { SettingsMenu, SettingsMenuManager } from "../SettingsMenu" +import { DEFAULT_GRAPHER_FRAME_PADDING } from "../../core/GrapherConstants" + +export interface ControlsRowManager + extends ContentSwitchersManager, + EntitySelectionManager, + MapProjectionMenuManager, + SettingsMenuManager { + sidePanelBounds?: Bounds + availableTabs?: GrapherTabOption[] + showEntitySelectionToggle?: boolean + framePaddingHorizontal?: number + framePaddingVertical?: number +} + +@observer +export class ControlsRow extends React.Component<{ + manager: ControlsRowManager + maxWidth?: number + settingsMenuTop?: number +}> { + static shouldShow(manager: ControlsRowManager): boolean { + const test = new ControlsRow({ manager }) + return test.showControlsRow + } + + @computed private get manager(): ControlsRowManager { + return this.props.manager + } + + @computed private get maxWidth(): number { + return this.props.maxWidth ?? DEFAULT_BOUNDS.width + } + + @computed private get framePaddingHorizontal(): number { + return ( + this.manager.framePaddingHorizontal ?? DEFAULT_GRAPHER_FRAME_PADDING + ) + } + + @computed private get framePaddingVertical(): number { + return ( + this.manager.framePaddingVertical ?? DEFAULT_GRAPHER_FRAME_PADDING + ) + } + + @computed private get sidePanelWidth(): number { + return this.manager.sidePanelBounds?.width ?? 0 + } + + @computed private get contentSwitchersWidth(): number { + return ContentSwitchers.width(this.manager) + } + + @computed private get availableWidth(): number { + return this.maxWidth - this.contentSwitchersWidth - 16 + } + + @computed private get showControlsRow(): boolean { + return ( + SettingsMenu.shouldShow(this.manager) || + EntitySelectionToggle.shouldShow(this.manager) || + MapProjectionMenu.shouldShow(this.manager) || + ContentSwitchers.shouldShow(this.manager) + ) + } + + render(): JSX.Element { + const { showEntitySelectionToggle } = this.manager + return ( + + ) + } +} diff --git a/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx b/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx index be9fe2c4445..c2eff6702b5 100644 --- a/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx +++ b/packages/@ourworldindata/grapher/src/controls/settings/TableFilterToggle.tsx @@ -1,8 +1,9 @@ import React from "react" -import { action } from "mobx" +import { action, computed } from "mobx" import { observer } from "mobx-react" import { DEFAULT_GRAPHER_ENTITY_TYPE_PLURAL } from "../../core/GrapherConstants" import { LabeledSwitch } from "../LabeledSwitch" +import { Bounds } from "@ourworldindata/utils" export interface TableFilterToggleManager { showSelectionOnlyInDataTable?: boolean @@ -12,7 +13,22 @@ export interface TableFilterToggleManager { @observer export class TableFilterToggle extends React.Component<{ manager: TableFilterToggleManager + showTooltip?: boolean }> { + static width(manager: TableFilterToggleManager): number { + return new TableFilterToggle({ manager }).width + } + + private label = "Show selection only" + + // keep in sync with CSS + @computed get width(): number { + const toggleWidth = 30 + const infoIconWidth = 22 + const labelWidth = Bounds.forText(this.label, { fontSize: 13 }).width + return labelWidth + toggleWidth + infoIconWidth + 4 + } + @action.bound onToggle(): void { const { manager } = this.props manager.showSelectionOnlyInDataTable = @@ -28,7 +44,7 @@ export class TableFilterToggle extends React.Component<{ return (