From b80535d74110de4e1e2e3459ef46bbb215d3baf3 Mon Sep 17 00:00:00 2001 From: Boris Serdiuk Date: Tue, 18 Jul 2023 16:41:37 +0200 Subject: [PATCH] revert: Add fit-height property to charts (#1341) --- pages/area-chart/fit-height.page.tsx | 104 -------------- .../mixed-line-bar-chart/fit-height.page.tsx | 80 ----------- pages/pie-chart/fit-height.page.tsx | 81 ----------- .../__snapshots__/documenter.test.ts.snap | 45 +----- .../area-chart-initial-state.test.tsx | 25 +--- src/area-chart/chart-container.tsx | 21 +-- src/area-chart/internal.tsx | 5 - src/area-chart/model/index.ts | 1 - src/area-chart/model/use-chart-model.ts | 16 +-- .../cartesian-chart/chart-container.tsx | 31 +--- .../components/cartesian-chart/interfaces.ts | 6 - .../components/cartesian-chart/styles.scss | 47 +------ .../chart-plot/__tests__/chart-plot.test.tsx | 4 +- src/internal/components/chart-plot/index.tsx | 8 +- .../components/chart-wrapper/index.tsx | 60 ++++---- .../components/chart-wrapper/styles.scss | 19 --- .../__tests__/mixed-chart.test.tsx | 13 +- src/mixed-line-bar-chart/chart-container.tsx | 30 +--- src/mixed-line-bar-chart/internal.tsx | 5 +- src/mixed-line-bar-chart/styles.scss | 1 + src/pie-chart/__tests__/utils.test.tsx | 34 +---- src/pie-chart/index.tsx | 14 +- src/pie-chart/interfaces.ts | 6 - src/pie-chart/labels.tsx | 10 +- src/pie-chart/pie-chart.tsx | 133 +++++++----------- src/pie-chart/segments.tsx | 12 +- src/pie-chart/styles.scss | 22 --- src/pie-chart/utils.ts | 61 ++------ 28 files changed, 133 insertions(+), 761 deletions(-) delete mode 100644 pages/area-chart/fit-height.page.tsx delete mode 100644 pages/mixed-line-bar-chart/fit-height.page.tsx delete mode 100644 pages/pie-chart/fit-height.page.tsx diff --git a/pages/area-chart/fit-height.page.tsx b/pages/area-chart/fit-height.page.tsx deleted file mode 100644 index eea9e4c9bc..0000000000 --- a/pages/area-chart/fit-height.page.tsx +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -import React, { useContext } from 'react'; - -import { createLinearTimeLatencyProps } from './series'; -import { AreaChart, Box, Button, Checkbox, SpaceBetween } from '~components'; -import AppContext, { AppContextType } from '../app/app-context'; -import ScreenshotArea from '../utils/screenshot-area'; - -type DemoContext = React.Context< - AppContextType<{ fitHeight: boolean; hideFilter: boolean; hideLegend: boolean; minHeight: number }> ->; - -const chartData = createLinearTimeLatencyProps(); - -export default function () { - const { urlParams, setUrlParams } = useContext(AppContext as DemoContext); - const minHeight = parseInt(urlParams.minHeight?.toString() || '0'); - const heights = [800, 600, 400, 300, 200, 100]; - const fitHeight = urlParams.fitHeight ?? true; - return ( - -

Area chart fit height

- - - setUrlParams({ fitHeight: e.detail.checked })}> - fit height - - setUrlParams({ hideFilter: e.detail.checked })}> - hide filter - - setUrlParams({ hideLegend: e.detail.checked })}> - hide legend - - - setUrlParams({ minHeight: parseInt(e.target.value) })} - /> - - - - - - - {heights.map(height => ( - - {height}px -
- {}} - empty={ - - No data - - There is no data to display - - - } - noMatch={ - - No matching data - - There is no data to display - - - - } - i18nStrings={{ - filterLabel: 'Filter displayed data', - filterPlaceholder: 'Filter data', - filterSelectedAriaLabel: '(selected)', - detailTotalLabel: 'Total', - detailPopoverDismissAriaLabel: 'Dismiss', - legendAriaLabel: 'Legend', - chartAriaRoleDescription: 'area chart', - xAxisAriaRoleDescription: 'x axis', - yAxisAriaRoleDescription: 'y axis', - xTickFormatter: value => `${value}\nxxx`, - }} - xDomain={[0, 119]} - {...chartData} - /> -
- - ))} - - - - ); -} diff --git a/pages/mixed-line-bar-chart/fit-height.page.tsx b/pages/mixed-line-bar-chart/fit-height.page.tsx deleted file mode 100644 index fb1c833d8e..0000000000 --- a/pages/mixed-line-bar-chart/fit-height.page.tsx +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -import React, { useContext } from 'react'; - -import { Box, Button, Checkbox, MixedLineBarChart, SpaceBetween } from '~components'; -import AppContext, { AppContextType } from '../app/app-context'; -import ScreenshotArea from '../utils/screenshot-area'; -import { barChartInstructions, commonProps, data3, data4 } from './common'; -import { colorChartsThresholdInfo } from '~design-tokens'; - -type DemoContext = React.Context< - AppContextType<{ fitHeight: boolean; hideFilter: boolean; hideLegend: boolean; minHeight: number }> ->; - -export default function () { - const { urlParams, setUrlParams } = useContext(AppContext as DemoContext); - const minHeight = parseInt(urlParams.minHeight?.toString() || '0'); - const heights = [800, 600, 400, 300, 200, 100]; - const fitHeight = urlParams.fitHeight ?? true; - return ( - -

Mixed chart fit height

- - - setUrlParams({ fitHeight: e.detail.checked })}> - fit height - - setUrlParams({ hideFilter: e.detail.checked })}> - hide filter - - setUrlParams({ hideLegend: e.detail.checked })}> - hide legend - - - setUrlParams({ minHeight: parseInt(e.target.value) })} - /> - - - - - - - {heights.map(height => ( - - {height}px -
- x !== 'Chocolate') }, - { title: 'Calories', type: 'line', data: data3 }, - { title: 'Threshold', type: 'threshold', y: 420, color: colorChartsThresholdInfo }, - ]} - xDomain={data3.map(d => d.x)} - yDomain={[0, 650]} - xTitle="Food" - yTitle="Calories (kcal)" - xScaleType="categorical" - ariaLabel="Mixed chart 1" - ariaDescription={barChartInstructions} - detailPopoverFooter={xValue => } - /> -
- - ))} - - - - ); -} diff --git a/pages/pie-chart/fit-height.page.tsx b/pages/pie-chart/fit-height.page.tsx deleted file mode 100644 index 8a26e3fa66..0000000000 --- a/pages/pie-chart/fit-height.page.tsx +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 -import React, { useContext } from 'react'; - -import { Box, Button, Checkbox, PieChart, SegmentedControl, SpaceBetween } from '~components'; -import AppContext, { AppContextType } from '../app/app-context'; -import ScreenshotArea from '../utils/screenshot-area'; -import { FoodData, commonProps, data1 } from './common'; - -type DemoContext = React.Context< - AppContextType<{ - fitHeight: boolean; - hideFilter: boolean; - hideLegend: boolean; - minSize: 'large' | 'medium' | 'small'; - }> ->; - -export default function () { - const { urlParams, setUrlParams } = useContext(AppContext as DemoContext); - const minSize = urlParams.minSize ?? 'small'; - const heights = [800, 600, 400, 300, 200, 100]; - const fitHeight = urlParams.fitHeight ?? true; - return ( - -

Pie chart fit height

- - - setUrlParams({ fitHeight: e.detail.checked })}> - fit height - - setUrlParams({ hideFilter: e.detail.checked })}> - hide filter - - setUrlParams({ hideLegend: e.detail.checked })}> - hide legend - - - setUrlParams({ minSize: e.detail.selectedId as any })} - /> - - - - - - - {heights.map(height => ( - - {height}px -
- - {...commonProps} - fitHeight={fitHeight} - hideFilter={urlParams.hideFilter} - hideLegend={urlParams.hideLegend} - data={data1} - ariaLabel="Food facts" - size={minSize} - detailPopoverFooter={segment => } - variant="donut" - innerMetricValue="180" - /> -
- - ))} - - - - ); -} diff --git a/src/__tests__/__snapshots__/documenter.test.ts.snap b/src/__tests__/__snapshots__/documenter.test.ts.snap index 71b9449933..b3c723f84c 100644 --- a/src/__tests__/__snapshots__/documenter.test.ts.snap +++ b/src/__tests__/__snapshots__/documenter.test.ts.snap @@ -773,17 +773,10 @@ Do not use \`ariaLabel\` and \`ariaLabelledby\` at the same time.", "optional": true, "type": "string", }, - Object { - "description": "Enable this property to make the chart fit into the available height of the parent container.", - "name": "fitHeight", - "optional": true, - "type": "boolean", - }, Object { "defaultValue": "500", "description": "An optional pixel value number that fixes the height of the chart area. -If not set explicitly, the component will use a default height that is defined internally. -When used with \`fitHeight\`, this property defines the minimum height of the chart area.", +If not set explicitly, the component will use a default height that is defined internally.", "name": "height", "optional": true, "type": "number", @@ -1982,17 +1975,10 @@ See the usage guidelines for more details.", "optional": true, "type": "string", }, - Object { - "description": "Enable this property to make the chart fit into the available height of the parent container.", - "name": "fitHeight", - "optional": true, - "type": "boolean", - }, Object { "defaultValue": "500", "description": "An optional pixel value number that fixes the height of the chart area. -If not set explicitly, the component will use a default height that is defined internally. -When used with \`fitHeight\`, this property defines the minimum height of the chart area.", +If not set explicitly, the component will use a default height that is defined internally.", "name": "height", "optional": true, "type": "number", @@ -7735,17 +7721,10 @@ See the usage guidelines for more details.", "optional": true, "type": "string", }, - Object { - "description": "Enable this property to make the chart fit into the available height of the parent container.", - "name": "fitHeight", - "optional": true, - "type": "boolean", - }, Object { "defaultValue": "500", "description": "An optional pixel value number that fixes the height of the chart area. -If not set explicitly, the component will use a default height that is defined internally. -When used with \`fitHeight\`, this property defines the minimum height of the chart area.", +If not set explicitly, the component will use a default height that is defined internally.", "name": "height", "optional": true, "type": "number", @@ -8338,17 +8317,10 @@ See the usage guidelines for more details.", "optional": true, "type": "string", }, - Object { - "description": "Enable this property to make the chart fit into the available height of the parent container.", - "name": "fitHeight", - "optional": true, - "type": "boolean", - }, Object { "defaultValue": "500", "description": "An optional pixel value number that fixes the height of the chart area. -If not set explicitly, the component will use a default height that is defined internally. -When used with \`fitHeight\`, this property defines the minimum height of the chart area.", +If not set explicitly, the component will use a default height that is defined internally.", "name": "height", "optional": true, "type": "number", @@ -9546,12 +9518,6 @@ Each pair has the following properties: "optional": true, "type": "string", }, - Object { - "description": "Enable this property to make the chart fit into the available height of the parent container.", - "name": "fitHeight", - "optional": true, - "type": "boolean", - }, Object { "defaultValue": "false", "description": "Hides the label descriptions next to the chart segments when set to \`true\`.", @@ -9717,8 +9683,7 @@ The function is called with the data object of each segment and is expected to r }, Object { "defaultValue": "\\"medium\\"", - "description": "Specifies the size of the pie or donut chart. -When used with \`fitHeight\`, this property defines the minimum size of the chart area.", + "description": "Specifies the size of the pie or donut chart.", "inlineType": Object { "name": "", "type": "union", diff --git a/src/area-chart/__tests__/area-chart-initial-state.test.tsx b/src/area-chart/__tests__/area-chart-initial-state.test.tsx index e873728b8b..f05617ac73 100644 --- a/src/area-chart/__tests__/area-chart-initial-state.test.tsx +++ b/src/area-chart/__tests__/area-chart-initial-state.test.tsx @@ -6,8 +6,6 @@ import { AreaChartWrapper } from '../../../lib/components/test-utils/dom'; import AreaChart, { AreaChartProps } from '../../../lib/components/area-chart'; import { KeyCode } from '@cloudscape-design/test-utils-core/dist/utils'; import popoverStyles from '../../../lib/components/popover/styles.css.js'; -import chartWrapperStyles from '../../../lib/components/internal/components/chart-wrapper/styles.css.js'; -import cartesianStyles from '../../../lib/components/internal/components/cartesian-chart/styles.css.js'; import { warnOnce } from '@cloudscape-design/component-toolkit/internal'; import TestI18nProvider from '../../../lib/components/internal/i18n/testing'; import { cloneDeep } from 'lodash'; @@ -86,28 +84,9 @@ test('error and recovery texts are assigned', () => { expect(wrapper.findStatusContainer()!.getElement()).toHaveTextContent('Ooops! Try again'); }); -test('explicit chart height is assigned', () => { +test('chart height is assigned', () => { const { wrapper } = renderAreaChart(); - expect(wrapper.findChart()!.getElement().style.height).toContain('333'); -}); - -test('when fitHeight=true chart height is flexible', () => { - const { wrapper } = renderAreaChart( - - ); - expect(wrapper.findChart()!.getElement().style.height).toContain('100%'); -}); - -test('when fitHeight=false content min height is explicitly set', () => { - const { wrapper } = renderAreaChart(); - expect(wrapper.findByClassName(chartWrapperStyles.content)?.getElement()).toHaveStyle({ minHeight: '333px' }); -}); - -test.each([false, true])('when fitHeight=%s plot min-height is explicitly set', fitHeight => { - const { wrapper } = renderAreaChart(); - const chartElement = wrapper.findByClassName(cartesianStyles['chart-container-plot-wrapper'])!.getElement(); - expect(chartElement.style.minHeight).toBeDefined(); - expect(parseInt(chartElement.style.minHeight)).toBeGreaterThanOrEqual(333); + expect(wrapper.findChart()!.getElement()).toHaveAttribute('height', '333'); }); test('empty text is assigned', () => { diff --git a/src/area-chart/chart-container.tsx b/src/area-chart/chart-container.tsx index 5a45dc7e2d..a122c9ffc2 100644 --- a/src/area-chart/chart-container.tsx +++ b/src/area-chart/chart-container.tsx @@ -44,8 +44,6 @@ interface ChartContainerProps > { model: ChartModel; autoWidth: (value: number) => void; - fitHeight?: boolean; - minHeight: number; } export default memo(ChartContainer) as typeof ChartContainer; @@ -70,8 +68,6 @@ function ChartContainer({ yAxisAriaRoleDescription, detailPopoverDismissAriaLabel, } = {}, - fitHeight, - minHeight, xTickFormatter = deprecatedXTickFormatter, yTickFormatter = deprecatedYTickFormatter, detailTotalFormatter = deprecatedDetailTotalFormatter, @@ -110,8 +106,6 @@ function ChartContainer({ return ( } leftAxisLabelMeasure={ ({ chartPlot={ ({ onFocus={model.handlers.onSVGFocus} onBlur={model.handlers.onSVGBlur} > - - = SomeRequired< InternalBaseComponentProps; export default function InternalAreaChart({ - fitHeight, height, xScaleType, yScaleType, @@ -95,7 +94,6 @@ export default function InternalAreaChart({ controlledOnHighlightChange ); const model = useChartModel({ - fitHeight, externalSeries, visibleSeries, setVisibleSeries, @@ -139,7 +137,6 @@ export default function InternalAreaChart({ ref={mergedRef} {...baseProps} className={clsx(baseProps.className, styles.root)} - fitHeight={!!fitHeight} contentMinHeight={height} defaultFilter={ showFilters && !hideFilter ? ( @@ -184,8 +181,6 @@ export default function InternalAreaChart({ ariaLabelledby={ariaLabelledby} ariaDescription={ariaDescription} i18nStrings={i18nStrings} - fitHeight={fitHeight} - minHeight={height} /> ) : null } diff --git a/src/area-chart/model/index.ts b/src/area-chart/model/index.ts index faebf2c902..9e22c62596 100644 --- a/src/area-chart/model/index.ts +++ b/src/area-chart/model/index.ts @@ -32,7 +32,6 @@ export interface ChartModel { interactions: ReadonlyAsyncStore>; refs: { plot: React.RefObject; - plotMeasure: React.Ref; container: React.RefObject; verticalMarker: React.RefObject; popoverRef: React.RefObject; diff --git a/src/area-chart/model/use-chart-model.ts b/src/area-chart/model/use-chart-model.ts index aed775f809..31d3d02d7d 100644 --- a/src/area-chart/model/use-chart-model.ts +++ b/src/area-chart/model/use-chart-model.ts @@ -1,7 +1,7 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 import { AreaChartProps } from '../interfaces'; -import React, { useEffect, useMemo, useRef, RefObject, MouseEvent, useState } from 'react'; +import React, { useEffect, useMemo, useRef, RefObject, MouseEvent } from 'react'; import { findClosest, circleIndex } from './utils'; import { nodeContains } from '../../internal/utils/dom'; @@ -15,14 +15,12 @@ import { ChartModel } from './index'; import { ChartPlotRef } from '../../internal/components/chart-plot'; import { throttle } from '../../internal/utils/throttle'; import { useReaction } from '../async-store'; -import { useResizeObserver } from '../../internal/hooks/container-queries'; const MAX_HOVER_MARGIN = 6; const SVG_HOVER_THROTTLE = 25; const POPOVER_DEADZONE = 12; export interface UseChartModelProps { - fitHeight?: boolean; externalSeries: readonly AreaChartProps.Series[]; visibleSeries: readonly AreaChartProps.Series[]; setVisibleSeries: (series: readonly AreaChartProps.Series[]) => void; @@ -39,7 +37,6 @@ export interface UseChartModelProps { // Represents the core the chart logic, including the model of all allowed user interactions. export default function useChartModel({ - fitHeight, externalSeries: allSeries, visibleSeries: series, setVisibleSeries, @@ -49,7 +46,7 @@ export default function useChartModel({ yDomain, xScaleType, yScaleType, - height: explicitHeight, + height, width, popoverRef, }: UseChartModelProps): ChartModel { @@ -58,14 +55,6 @@ export default function useChartModel({ const containerRef = useRef(null); const verticalMarkerRef = useRef(null); - const plotMeasureRef = useRef(null); - const [measuredHeight, setHeight] = useState(0); - useResizeObserver( - () => plotMeasureRef.current, - entry => fitHeight && setHeight(entry.borderBoxHeight) - ); - const height = fitHeight ? measuredHeight : explicitHeight; - const stableSetVisibleSeries = useStableEventHandler(setVisibleSeries); const model = useMemo(() => { @@ -352,7 +341,6 @@ export default function useChartModel({ }, refs: { plot: plotRef, - plotMeasure: plotMeasureRef, container: containerRef, verticalMarker: verticalMarkerRef, popoverRef, diff --git a/src/internal/components/cartesian-chart/chart-container.tsx b/src/internal/components/cartesian-chart/chart-container.tsx index 0730c08447..df10fe70a2 100644 --- a/src/internal/components/cartesian-chart/chart-container.tsx +++ b/src/internal/components/cartesian-chart/chart-container.tsx @@ -3,11 +3,8 @@ import React, { forwardRef } from 'react'; import styles from './styles.css.js'; -import clsx from 'clsx'; interface CartesianChartContainerProps { - minHeight: number; - fitHeight: boolean; leftAxisLabel: React.ReactNode; leftAxisLabelMeasure: React.ReactNode; bottomAxisLabel: React.ReactNode; @@ -15,38 +12,22 @@ interface CartesianChartContainerProps { popover: React.ReactNode; } -const CONTENT_MIN_HEIGHT_BOUNDARY = 40; - export const CartesianChartContainer = forwardRef( ( - { - minHeight, - fitHeight, - leftAxisLabel, - leftAxisLabelMeasure, - bottomAxisLabel, - chartPlot, - popover, - }: CartesianChartContainerProps, + { leftAxisLabel, leftAxisLabelMeasure, bottomAxisLabel, chartPlot, popover }: CartesianChartContainerProps, ref: React.Ref ) => { - const withFitHeight = (className: string) => clsx(className, fitHeight && styles['fit-height']); return ( -
+
{leftAxisLabel} -
+
{leftAxisLabelMeasure} -
-
-
{chartPlot}
-
+
+ {chartPlot} -
{bottomAxisLabel}
+ {bottomAxisLabel}
{popover} diff --git a/src/internal/components/cartesian-chart/interfaces.ts b/src/internal/components/cartesian-chart/interfaces.ts index 055f4497ba..27149b2886 100644 --- a/src/internal/components/cartesian-chart/interfaces.ts +++ b/src/internal/components/cartesian-chart/interfaces.ts @@ -86,7 +86,6 @@ export interface CartesianChartProps extends B /** * An optional pixel value number that fixes the height of the chart area. * If not set explicitly, the component will use a default height that is defined internally. - * When used with `fitHeight`, this property defines the minimum height of the chart area. */ height?: number; @@ -187,11 +186,6 @@ export interface CartesianChartProps extends B * Called when the highlighted series has changed because of user interaction. */ onHighlightChange?: NonCancelableEventHandler>; - - /** - * Enable this property to make the chart fit into the available height of the parent container. - */ - fitHeight?: boolean; } export namespace CartesianChartProps { diff --git a/src/internal/components/cartesian-chart/styles.scss b/src/internal/components/cartesian-chart/styles.scss index 7118cd273a..08fa16b630 100644 --- a/src/internal/components/cartesian-chart/styles.scss +++ b/src/internal/components/cartesian-chart/styles.scss @@ -114,55 +114,14 @@ display: flex; width: 100%; flex-direction: column; - - &.fit-height { - height: 100%; - min-height: inherit; - } } -.chart-container-outer { - display: flex; - - &.fit-height { - flex: 1; - } -} - -.chart-container-inner { - position: relative; +.chart-container__vertical { display: flex; flex-direction: column; width: 100%; } -.chart-container-plot-wrapper { - display: contents; - - &.fit-height { - display: block; - position: relative; - flex: 1; - } -} - -.chart-container-plot { - display: contents; - - &.fit-height { - display: block; - position: absolute; - top: 0; - right: 0; - bottom: 0; - left: 0; - } -} - -.chart-container-bottom-labels { - display: contents; - - &.fit-height { - display: block; - } +.chart-container__horizontal { + display: flex; } diff --git a/src/internal/components/chart-plot/__tests__/chart-plot.test.tsx b/src/internal/components/chart-plot/__tests__/chart-plot.test.tsx index ea990638c5..32530e954c 100644 --- a/src/internal/components/chart-plot/__tests__/chart-plot.test.tsx +++ b/src/internal/components/chart-plot/__tests__/chart-plot.test.tsx @@ -65,8 +65,8 @@ describe('initial state', () => { ); const plot = plotWrapper.getElement(); - expect(plot.style.width).toContain('200'); - expect(plot.style.height).toContain('100'); + expect(plot.getAttribute('width')).toBe('200'); + expect(plot.getAttribute('height')).toBe('100'); expect(plot.style.margin).toContain('1px 2px 3px 4px'); expect(plot.textContent).toContain('Test'); expect(plot.classList.contains(styles.clickable)).toBe(true); diff --git a/src/internal/components/chart-plot/index.tsx b/src/internal/components/chart-plot/index.tsx index 931b2e330b..ee186975f6 100644 --- a/src/internal/components/chart-plot/index.tsx +++ b/src/internal/components/chart-plot/index.tsx @@ -23,8 +23,8 @@ export interface ChartPlotRef { } export interface ChartPlotProps { - width: number | string; - height: number | string; + width: number; + height: number; transform?: string; offsetTop?: number; offsetBottom?: number; @@ -172,9 +172,9 @@ function ChartPlot( aria-hidden="false" {...plotAria} ref={svgRef} + width={width} + height={height} style={{ - width, - height, marginTop: offsetTop, marginBottom: offsetBottom, marginLeft: offsetLeft, diff --git a/src/internal/components/chart-wrapper/index.tsx b/src/internal/components/chart-wrapper/index.tsx index 97c1ed75b6..494eb0c972 100644 --- a/src/internal/components/chart-wrapper/index.tsx +++ b/src/internal/components/chart-wrapper/index.tsx @@ -10,7 +10,6 @@ import InternalBox from '../../../box/internal.js'; import InternalSpaceBetween from '../../../space-between/internal.js'; interface ChartWrapperProps extends BaseComponentProps { - fitHeight: boolean; defaultFilter: React.ReactNode; additionalFilters: React.ReactNode; reserveFilterSpace: boolean; @@ -36,49 +35,38 @@ export const ChartWrapper = forwardRef( onBlur, contentClassName, contentMinHeight, - fitHeight, ...props }: ChartWrapperProps, ref: React.Ref ) => { const baseProps = getBaseProps(props); return ( -
-
- {(defaultFilter || additionalFilters) && ( - - - {defaultFilter} - {additionalFilters} - - - )} +
+ {(defaultFilter || additionalFilters) && ( + + + {defaultFilter} + {additionalFilters} + + + )} -
- {chartStatus} - {chart} -
- - {legend && {legend}} +
+ {chartStatus} + {chart}
+ + {legend && {legend}}
); } diff --git a/src/internal/components/chart-wrapper/styles.scss b/src/internal/components/chart-wrapper/styles.scss index caef6ad956..6419763a63 100644 --- a/src/internal/components/chart-wrapper/styles.scss +++ b/src/internal/components/chart-wrapper/styles.scss @@ -10,21 +10,6 @@ @include styles.styles-reset; position: relative; display: block; - - &--fit-height { - height: 100%; - overflow-y: auto; - } -} - -.inner-wrapper { - display: contents; - - &--fit-height { - display: flex; - flex-direction: column; - height: 100%; - } } .has-default-filter { @@ -48,10 +33,6 @@ margin-bottom: calc(2 * #{awsui.$font-body-m-line-height}); } -.content--fit-height { - flex: 1; -} - .filter-container { /* used in test-utils */ } diff --git a/src/mixed-line-bar-chart/__tests__/mixed-chart.test.tsx b/src/mixed-line-bar-chart/__tests__/mixed-chart.test.tsx index 160bf23991..59706baf5f 100644 --- a/src/mixed-line-bar-chart/__tests__/mixed-chart.test.tsx +++ b/src/mixed-line-bar-chart/__tests__/mixed-chart.test.tsx @@ -811,20 +811,11 @@ describe('Reserve space', () => { const reserveFilterClass = chartWrapperStyles['content--reserve-filter']; const reserveLegendClass = chartWrapperStyles['content--reserve-legend']; - test('by applying the correct minimum height when fitHeight=false', () => { - const { wrapper } = renderMixedChart(); + test('by applying the correct minimum height', () => { + const { wrapper } = renderMixedChart(); expect(wrapper.findByClassName(chartWrapperStyles.content)?.getElement()).toHaveStyle({ minHeight: '100px' }); }); - test.each([false, true])('when fitHeight=%s plot min-height is explicitly set', fitHeight => { - const { wrapper } = renderMixedChart( - - ); - const chartElement = wrapper.findByClassName(cartesianStyles['chart-container-plot-wrapper'])!.getElement(); - expect(chartElement.style.minHeight).toBeDefined(); - expect(parseInt(chartElement.style.minHeight)).toBeGreaterThanOrEqual(100); - }); - test('unless there is a chart showing', () => { const { wrapper } = renderMixedChart(); diff --git a/src/mixed-line-bar-chart/chart-container.tsx b/src/mixed-line-bar-chart/chart-container.tsx index 5f6ddfb760..90992f7760 100644 --- a/src/mixed-line-bar-chart/chart-container.tsx +++ b/src/mixed-line-bar-chart/chart-container.tsx @@ -33,7 +33,6 @@ import useContainerWidth from '../internal/utils/use-container-width'; import { useMergeRefs } from '../internal/hooks/use-merge-refs'; import { nodeBelongs } from '../internal/utils/node-belongs'; import { CartesianChartContainer } from '../internal/components/cartesian-chart/chart-container'; -import { useResizeObserver } from '../internal/hooks/container-queries'; const LEFT_LABELS_MARGIN = 16; const BOTTOM_LABELS_OFFSET = 12; @@ -44,7 +43,6 @@ export interface ChartContainerProps { series: ReadonlyArray>; visibleSeries: ReadonlyArray>; - fitHeight?: boolean; height: number; detailPopoverSize: MixedLineBarChartProps['detailPopoverSize']; detailPopoverFooter: MixedLineBarChartProps['detailPopoverFooter']; @@ -81,8 +79,7 @@ export interface ChartContainerProps { } export default function ChartContainer({ - fitHeight, - height: explicitPlotHeight, + height: plotHeight, series, visibleSeries, highlightedSeries, @@ -121,14 +118,6 @@ export default function ChartContainer({ const containerRef = useMergeRefs(containerMeasureRef, containerRefObject); const popoverRef = useRef(null); - const plotMeasureRef = useRef(null); - const [measuredHeight, setHeight] = useState(0); - useResizeObserver( - () => plotMeasureRef.current, - entry => fitHeight && setHeight(entry.borderBoxHeight) - ); - const plotHeight = fitHeight ? measuredHeight : explicitPlotHeight; - const isRefresh = useVisualRefresh(); const linesOnly = series.every(({ series }) => series.type === 'line' || series.type === 'threshold'); @@ -456,8 +445,6 @@ export default function ChartContainer({ return ( } leftAxisLabelMeasure={ ({ chartPlot={ ({ onBlur={onSVGBlur} onKeyDown={onSVGKeyDown} > - - = SomeRequired< InternalBaseComponentProps; export default function InternalMixedLineBarChart({ - fitHeight, height, xScaleType, yScaleType, @@ -218,7 +217,6 @@ export default function InternalMixedLineBarChart { }); }); }); - -describe.each([false, true])('getDimensionsBySize visualRefresh=%s', visualRefresh => { - const d = visualRefresh ? refreshDimensionsBySize : dimensionsBySize; - - test.each(['small', 'medium', 'large'] as const)('get correct dimensions for size="%s"', size => { - const dimensions = getDimensionsBySize({ size, hasLabels: true, visualRefresh }); - expect(dimensions).toEqual({ ...d[size], size }); - }); - - test.each([ - [d.medium.outerRadius * 2 + d.medium.padding * 2 - 1, 'small'], - [d.large.outerRadius * 2 + d.large.padding * 2 - 1, 'medium'], - [d.large.outerRadius * 2 + d.large.padding * 2 + 1, 'large'], - ])('matches size correctly for height=$0 and hasLabels=false', (height, matchedSize) => { - const dimensions = getDimensionsBySize({ size: height, hasLabels: false, visualRefresh }); - expect(dimensions.size).toBe(matchedSize); - }); - - test.each([ - [d.medium.outerRadius * 2 + d.medium.padding * 2 + d.medium.paddingLabels * 2 - 1, 'small'], - [d.large.outerRadius * 2 + d.large.padding * 2 + d.large.paddingLabels * 2 - 1, 'medium'], - [d.large.outerRadius * 2 + d.large.padding * 2 + d.large.paddingLabels * 2 + 1, 'large'], - ])('matches size correctly for height=$0 and hasLabels=true', (height, matchedSize) => { - const dimensions = getDimensionsBySize({ size: height, hasLabels: true, visualRefresh }); - expect(dimensions.size).toBe(matchedSize); - }); -}); diff --git a/src/pie-chart/index.tsx b/src/pie-chart/index.tsx index aa9762b0f4..3994cbe56e 100644 --- a/src/pie-chart/index.tsx +++ b/src/pie-chart/index.tsx @@ -21,13 +21,10 @@ import useContainerWidth from '../internal/utils/use-container-width'; import { nodeBelongs } from '../internal/utils/node-belongs'; import { ChartWrapper } from '../internal/components/chart-wrapper'; import ChartStatusContainer, { getChartStatus } from '../internal/components/chart-status-container'; -import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; -import { getDimensionsBySize } from './utils'; export { PieChartProps }; const PieChart = function PieChart({ - fitHeight, variant = 'pie', size = 'medium', hideTitles = false, @@ -154,20 +151,13 @@ const PieChart = function PieChart { pieData: PieArcDatum>[]; visibleDataSum: number; - dimensions: Dimension; + size: NonNullable; hideTitles: boolean; hideDescriptions: boolean; highlightedSegment: PieChartProps.Datum | null; @@ -69,7 +69,7 @@ function LabelElement({ export default ({ pieData, - dimensions, + size, highlightedSegment, segmentDescription, visibleDataSum, @@ -80,7 +80,7 @@ export default ({ const containerBoundaries = useElementBoundaries(containerRef); const markers = useMemo(() => { - const { outerRadius: radius, innerLabelPadding } = dimensions; + const { outerRadius: radius, innerLabelPadding } = dimensionsBySize[size]; // More arc factories for the label positioning const arcMarkerStart = arc>() @@ -118,7 +118,7 @@ export default ({ datum, }; }); - }, [pieData, dimensions]); + }, [pieData, size]); const rootRef = useRef(null); diff --git a/src/pie-chart/pie-chart.tsx b/src/pie-chart/pie-chart.tsx index e515537f5c..da9197d353 100644 --- a/src/pie-chart/pie-chart.tsx +++ b/src/pie-chart/pie-chart.tsx @@ -13,15 +13,13 @@ import InternalBox from '../box/internal'; import Labels from './labels'; import { PieChartProps, SeriesInfo } from './interfaces'; import styles from './styles.css.js'; -import { defaultDetails, getDimensionsBySize } from './utils'; +import { defaultDetails, dimensionsBySize, refreshDimensionsBySize } from './utils'; import Segments from './segments'; +import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; import ChartPlot, { ChartPlotRef } from '../internal/components/chart-plot'; import { SomeRequired } from '../internal/types'; import { useInternalI18n } from '../internal/i18n/context'; import { nodeBelongs } from '../internal/utils/node-belongs'; -import clsx from 'clsx'; -import { useResizeObserver } from '../internal/hooks/container-queries'; -import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; export interface InternalChartDatum { index: number; @@ -35,7 +33,6 @@ interface InternalPieChartProps 'variant' | 'size' | 'i18nStrings' | 'hideTitles' | 'hideDescriptions' > { width: number; - height: number; highlightedSegment: T | null; onHighlightChange: (segment: null | T) => void; @@ -56,11 +53,8 @@ export interface TooltipData { } export default ({ - fitHeight, - height: explicitHeight, variant, size, - width, i18nStrings, ariaLabel, ariaLabelledby, @@ -72,6 +66,7 @@ export default ({ detailPopoverContent, detailPopoverSize, detailPopoverFooter, + width, segmentDescription, highlightedSegment, onHighlightChange, @@ -86,26 +81,16 @@ export default ({ const focusedSegmentRef = useRef(null); const popoverTrackRef = useRef(null); const popoverRef = useRef(null); - - const hasLabels = !(hideTitles && hideDescriptions); const isRefresh = useVisualRefresh(); - const [measuredHeight, setHeight] = useState(0); - useResizeObserver( - () => plotRef.current?.svg ?? null, - entry => fitHeight && setHeight(entry.borderBoxHeight) - ); - const height = fitHeight ? measuredHeight : explicitHeight; + const dimensions = isRefresh ? refreshDimensionsBySize[size] : dimensionsBySize[size]; + const radius = dimensions.outerRadius; - const dimensions = useMemo( - () => - getDimensionsBySize({ size: fitHeight ? Math.min(height, width) : size, hasLabels, visualRefresh: isRefresh }), - [fitHeight, height, width, size, hasLabels, isRefresh] - ); + const hasLabels = !(hideTitles && hideDescriptions); + const height = 2 * (radius + dimensions.padding + (hasLabels ? dimensions.paddingLabels : 0)); // Inner content is only available for donut charts and the inner description is not displayed for small charts - const hasInnerContent = - variant === 'donut' && (innerMetricValue || (innerMetricDescription && dimensions.size !== 'small')); + const hasInnerContent = variant === 'donut' && (innerMetricValue || (innerMetricDescription && size !== 'small')); const innerMetricId = useUniqueId('awsui-pie-chart__inner'); @@ -296,76 +281,60 @@ export default ({ }; return ( -
-
+ - - + {hasLabels && ( + - {hasLabels && ( - - )} - -
- + )} + {hasInnerContent && (
{innerMetricValue && ( - + {innerMetricValue} )} - {innerMetricDescription && dimensions.size !== 'small' && ( + {innerMetricDescription && size !== 'small' && ( {innerMetricDescription} diff --git a/src/pie-chart/segments.tsx b/src/pie-chart/segments.tsx index 131f67bbdb..82a0d8453a 100644 --- a/src/pie-chart/segments.tsx +++ b/src/pie-chart/segments.tsx @@ -4,8 +4,9 @@ import React, { useMemo } from 'react'; import { arc, PieArcDatum } from 'd3-shape'; import { PieChartProps } from './interfaces'; -import { Dimension } from './utils'; +import { dimensionsBySize, refreshDimensionsBySize } from './utils'; import { InternalChartDatum } from './pie-chart'; +import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; import styles from './styles.css.js'; import clsx from 'clsx'; import { useInternalI18n } from '../internal/i18n/context'; @@ -13,11 +14,12 @@ import { useInternalI18n } from '../internal/i18n/context'; interface SegmentsProps { pieData: Array>>; highlightedSegment: T | null; - dimensions: Dimension; + size: NonNullable; variant: PieChartProps['variant']; focusedSegmentRef: React.RefObject; popoverTrackRef: React.RefObject; segmentAriaRoleDescription?: string; + onMouseDown: (datum: InternalChartDatum) => void; onMouseOver: (datum: InternalChartDatum) => void; onMouseOut: (event: React.MouseEvent) => void; @@ -26,7 +28,7 @@ interface SegmentsProps { export default function Segments({ pieData, highlightedSegment, - dimensions, + size, variant, focusedSegmentRef, popoverTrackRef, @@ -36,8 +38,10 @@ export default function Segments({ onMouseOut, }: SegmentsProps) { const i18n = useInternalI18n('pie-chart'); + const isRefresh = useVisualRefresh(); const { arcFactory, highlightedArcFactory } = useMemo(() => { + const dimensions = isRefresh ? refreshDimensionsBySize[size] : dimensionsBySize[size]; const radius = dimensions.outerRadius; const innerRadius = variant === 'pie' ? 0 : dimensions.innerRadius; const cornerRadius = dimensions.cornerRadius || 0; @@ -55,7 +59,7 @@ export default function Segments({ arcFactory, highlightedArcFactory, }; - }, [dimensions, variant]); + }, [size, variant, isRefresh]); const centroid = useMemo(() => { for (const datum of pieData) { diff --git a/src/pie-chart/styles.scss b/src/pie-chart/styles.scss index 4ae6435623..6907bba55d 100644 --- a/src/pie-chart/styles.scss +++ b/src/pie-chart/styles.scss @@ -27,10 +27,6 @@ } } -.content--fit-height { - flex: 1; -} - .status-container { /* used in test utils */ } @@ -38,24 +34,6 @@ .chart-container { display: flex; flex: 1; - - &--fit-height { - height: 100%; - min-height: inherit; - } -} - -.chart-container-chart-plot { - display: contents; - - &--fit-height { - display: block; - position: absolute; - top: 0; - right: 0; - bottom: 0; - left: 0; - } } .inner-content { diff --git a/src/pie-chart/utils.ts b/src/pie-chart/utils.ts index 48aa91e576..3824ef6618 100644 --- a/src/pie-chart/utils.ts +++ b/src/pie-chart/utils.ts @@ -4,7 +4,7 @@ import { ComponentFormatFunction } from '../internal/i18n/context'; import { PieChartProps } from './interfaces'; import styles from './styles.css.js'; -export interface Dimension { +interface Dimension { innerRadius: number; outerRadius: number; padding: number; @@ -13,31 +13,27 @@ export interface Dimension { cornerRadius?: number; } -const paddingLabels = 44; // = 2 * (size-lineHeight-body-100) -const defaultPadding = 12; // = space-s -const smallPadding = 8; // = space-xs - export const dimensionsBySize: Record, Dimension> = { small: { innerRadius: 33, outerRadius: 50, - innerLabelPadding: smallPadding, - padding: smallPadding, - paddingLabels, + innerLabelPadding: 8, + padding: 8, // = space-xs + paddingLabels: 44, // = 2 * (size-lineHeight-body-100) }, medium: { innerRadius: 66, outerRadius: 100, - innerLabelPadding: defaultPadding, - padding: defaultPadding, - paddingLabels, + innerLabelPadding: 12, + padding: 12, // = space-s + paddingLabels: 44, // = 2 * (size-lineHeight-body-100) }, large: { innerRadius: 93, outerRadius: 140, - innerLabelPadding: defaultPadding, - padding: defaultPadding, - paddingLabels, + innerLabelPadding: 12, + padding: 12, // = space-s + paddingLabels: 44, // = 2 * (size-lineHeight-body-100) }, }; @@ -59,43 +55,6 @@ export const refreshDimensionsBySize: Record, }, }; -/** - * When `size` is a string ("small", "medium" or "large") the predefined pie chart element dimensions for classic and visual refresh are used. - * When `size` is a number the outer and inner radii are computed and the rest of the dimensions are taken from the closest predefined size. - */ -export function getDimensionsBySize({ - size, - hasLabels, - visualRefresh, -}: { - size: NonNullable | number; - hasLabels: boolean; - visualRefresh?: boolean; -}): Dimension & { size: NonNullable } { - if (typeof size === 'string') { - const dimensions = visualRefresh ? refreshDimensionsBySize[size] : dimensionsBySize[size]; - return { ...dimensions, size }; - } - const sizeSpec = visualRefresh ? refreshDimensionsBySize : dimensionsBySize; - const getPixelSize = (d: Dimension) => d.outerRadius * 2 + d.padding * 2 + (hasLabels ? d.paddingLabels : 0) * 2; - - let matchedSize: NonNullable = 'small'; - if (size > getPixelSize(sizeSpec.medium)) { - matchedSize = 'medium'; - } - if (size > getPixelSize(sizeSpec.large)) { - matchedSize = 'large'; - } - - const padding = sizeSpec[matchedSize].padding; - const paddingLabels = hasLabels ? sizeSpec[matchedSize].paddingLabels : 0; - const radiiRatio = sizeSpec[matchedSize].outerRadius / sizeSpec[matchedSize].innerRadius; - const outerRadius = (size - 2 * paddingLabels - 2 * padding) / 2; - const innerRadius = outerRadius / radiiRatio; - - return { ...sizeSpec[matchedSize], outerRadius, innerRadius, size: matchedSize }; -} - export const defaultDetails = (i18n: ComponentFormatFunction<'pie-chart'>, i18nStrings: PieChartProps.I18nStrings) => (datum: PieChartProps.Datum, dataSum: number) =>