From 8d73f329d4904bc93adf21df426ea8c3634ff74c Mon Sep 17 00:00:00 2001 From: evasseure Date: Fri, 3 May 2024 13:12:41 +0100 Subject: [PATCH 1/9] Add function to compute a bar label's layout --- packages/bar/src/compute/common.ts | 46 ++++++++++++++++++++++++++++++ packages/bar/src/props.ts | 2 ++ packages/bar/src/types.ts | 2 ++ 3 files changed, 50 insertions(+) diff --git a/packages/bar/src/compute/common.ts b/packages/bar/src/compute/common.ts index 058ccc3095..21f4767dcc 100644 --- a/packages/bar/src/compute/common.ts +++ b/packages/bar/src/compute/common.ts @@ -1,4 +1,6 @@ import { ScaleBandSpec, ScaleBand, computeScale } from '@nivo/scales' +import { defaultProps } from '../props' +import { BarCommonProps, BarDatum } from '../types' /** * Generates indexed scale. @@ -45,3 +47,47 @@ export const filterNullValues = >(data: }, {}) as Exclude export const coerceValue = (value: T) => [value, Number(value)] as const + +export type BarLabelLayout = { + labelX: number + labelY: number + textAnchor: 'start' | 'middle' +} + +/** + * Compute the label position and alignment based on a given position and offset. + */ +export function useComputeLabelLayout( + layout: BarCommonProps['layout'] = defaultProps.layout, + reverse: BarCommonProps['reverse'] = defaultProps.reverse, + labelPosition: BarCommonProps['labelPosition'] = defaultProps.labelPosition, + labelOffset: BarCommonProps['labelOffset'] = defaultProps.labelOffset +): (width: number, height: number) => BarLabelLayout { + return (width: number, height: number) => { + if (layout === 'horizontal') { + let x = width / 2 + if (labelPosition === 'start') { + x = reverse ? width : 0 + } else if (labelPosition === 'end') { + x = reverse ? 0 : width + } + return { + labelX: x + labelOffset, + labelY: height / 2, + textAnchor: labelPosition === 'center' ? 'middle' : 'start', + } + } else { + let y = height / 2 + if (labelPosition === 'start') { + y = reverse ? 0 : height + } else if (labelPosition === 'end') { + y = reverse ? height : 0 + } + return { + labelX: width / 2, + labelY: y + labelOffset, + textAnchor: 'middle', + } + } + } +} diff --git a/packages/bar/src/props.ts b/packages/bar/src/props.ts index 98c53fcf72..a75f57bc35 100644 --- a/packages/bar/src/props.ts +++ b/packages/bar/src/props.ts @@ -28,6 +28,8 @@ export const defaultProps = { enableLabel: true, label: 'formattedValue', + labelPosition: 'center' as const, + labelOffset: 0, labelSkipWidth: 0, labelSkipHeight: 0, labelTextColor: { from: 'theme', theme: 'labels.text.fill' }, diff --git a/packages/bar/src/types.ts b/packages/bar/src/types.ts index ec545851ed..9e49d9e747 100644 --- a/packages/bar/src/types.ts +++ b/packages/bar/src/types.ts @@ -230,6 +230,8 @@ export type BarCommonProps = { enableLabel: boolean label: PropertyAccessor, string> + labelPosition: 'start' | 'center' | 'end' + labelOffset: number labelFormat: string | LabelFormatter labelSkipWidth: number labelSkipHeight: number From 8885fbffb0b8c126079de932861f6f6ef43b3838 Mon Sep 17 00:00:00 2001 From: evasseure Date: Fri, 3 May 2024 13:18:56 +0100 Subject: [PATCH 2/9] Add support to SVG bars --- packages/bar/src/Bar.tsx | 20 ++++++++++++-------- packages/bar/src/BarItem.tsx | 3 ++- packages/bar/src/types.ts | 1 + 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/bar/src/Bar.tsx b/packages/bar/src/Bar.tsx index 76e6670a05..7538d41686 100644 --- a/packages/bar/src/Bar.tsx +++ b/packages/bar/src/Bar.tsx @@ -17,12 +17,14 @@ import { svgDefaultProps } from './props' import { BarCustomLayerProps, BarDatum, + BarItemProps, BarLayer, BarLayerId, BarSvgProps, ComputedBarDatumWithValue, } from './types' import { BarTotals } from './BarTotals' +import { useComputeLabelLayout } from './compute/common' type InnerBarProps = Omit< BarSvgProps, @@ -67,6 +69,8 @@ const InnerBar = ({ labelSkipWidth = svgDefaultProps.labelSkipWidth, labelSkipHeight = svgDefaultProps.labelSkipHeight, labelTextColor, + labelPosition = svgDefaultProps.labelPosition, + labelOffset = svgDefaultProps.labelOffset, markers = svgDefaultProps.markers, @@ -159,6 +163,8 @@ const InnerBar = ({ totalsOffset, }) + const computeLabelLayout = useComputeLabelLayout(layout, reverse, labelPosition, labelOffset) + const transition = useTransition< ComputedBarDatumWithValue, { @@ -172,6 +178,7 @@ const InnerBar = ({ opacity: number transform: string width: number + textAnchor: BarItemProps['style']['textAnchor'] } >(barsWithValue, { keys: bar => bar.key, @@ -181,8 +188,7 @@ const InnerBar = ({ height: 0, labelColor: getLabelColor(bar) as string, labelOpacity: 0, - labelX: bar.width / 2, - labelY: bar.height / 2, + ...computeLabelLayout(bar.width, bar.height), transform: `translate(${bar.x}, ${bar.y + bar.height})`, width: bar.width, ...(layout === 'vertical' @@ -199,8 +205,7 @@ const InnerBar = ({ height: bar.height, labelColor: getLabelColor(bar) as string, labelOpacity: 1, - labelX: bar.width / 2, - labelY: bar.height / 2, + ...computeLabelLayout(bar.width, bar.height), transform: `translate(${bar.x}, ${bar.y})`, width: bar.width, }), @@ -210,8 +215,7 @@ const InnerBar = ({ height: bar.height, labelColor: getLabelColor(bar) as string, labelOpacity: 1, - labelX: bar.width / 2, - labelY: bar.height / 2, + ...computeLabelLayout(bar.width, bar.height), transform: `translate(${bar.x}, ${bar.y})`, width: bar.width, }), @@ -221,15 +225,15 @@ const InnerBar = ({ height: 0, labelColor: getLabelColor(bar) as string, labelOpacity: 0, - labelX: bar.width / 2, + ...computeLabelLayout(bar.width, bar.height), labelY: 0, transform: `translate(${bar.x}, ${bar.y + bar.height})`, width: bar.width, ...(layout === 'vertical' ? {} : { + ...computeLabelLayout(bar.width, bar.height), labelX: 0, - labelY: bar.height / 2, height: bar.height, transform: `translate(${bar.x}, ${bar.y})`, width: 0, diff --git a/packages/bar/src/BarItem.tsx b/packages/bar/src/BarItem.tsx index 7ac2cf789f..81a4c4216f 100644 --- a/packages/bar/src/BarItem.tsx +++ b/packages/bar/src/BarItem.tsx @@ -17,6 +17,7 @@ export const BarItem = ({ labelY, transform, width, + textAnchor, }, borderRadius, @@ -104,7 +105,7 @@ export const BarItem = ({ opacity: number transform: string width: number + textAnchor: 'start' | 'middle' }> label: string From eb597e39d123b873bb64442508ccb34219dfbdb5 Mon Sep 17 00:00:00 2001 From: evasseure Date: Fri, 3 May 2024 13:20:37 +0100 Subject: [PATCH 3/9] Add support to CANVAS bar --- packages/bar/src/BarCanvas.tsx | 14 ++++++++++++-- packages/bar/src/types.ts | 10 ++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/bar/src/BarCanvas.tsx b/packages/bar/src/BarCanvas.tsx index e884ee5048..27459641e9 100644 --- a/packages/bar/src/BarCanvas.tsx +++ b/packages/bar/src/BarCanvas.tsx @@ -36,6 +36,7 @@ import { renderLegendToCanvas } from '@nivo/legends' import { useTooltip } from '@nivo/tooltip' import { useBar } from './hooks' import { BarTotalsData } from './compute/totals' +import { useComputeLabelLayout } from './compute/common' type InnerBarCanvasProps = Omit< BarCanvasProps, @@ -102,6 +103,9 @@ const InnerBarCanvas = ({ gridXValues, gridYValues, + labelPosition = canvasDefaultProps.labelPosition, + labelOffset = canvasDefaultProps.labelOffset, + layers = canvasDefaultProps.layers as BarCanvasLayer[], renderBar = ( ctx, @@ -114,6 +118,9 @@ const InnerBarCanvas = ({ label, labelColor, shouldRenderLabel, + labelX, + labelY, + textAnchor, } ) => { ctx.fillStyle = color @@ -150,9 +157,9 @@ const InnerBarCanvas = ({ if (shouldRenderLabel) { ctx.textBaseline = 'middle' - ctx.textAlign = 'center' + ctx.textAlign = textAnchor === 'middle' ? 'center' : textAnchor ctx.fillStyle = labelColor - ctx.fillText(label, x + width / 2, y + height / 2) + ctx.fillText(label, x + labelX, y + labelY) } }, @@ -311,6 +318,7 @@ const InnerBarCanvas = ({ ) const formatValue = useValueFormatter(valueFormat) + const computeLabelLayout = useComputeLabelLayout(layout, reverse, labelPosition, labelOffset) useEffect(() => { const ctx = canvasEl.current?.getContext('2d') @@ -375,6 +383,7 @@ const InnerBarCanvas = ({ label: getLabel(bar.data), labelColor: getLabelColor(bar) as string, shouldRenderLabel: shouldRenderBarLabel(bar), + ...computeLabelLayout(bar.width, bar.height), }) }) } else if (layer === 'legends') { @@ -436,6 +445,7 @@ const InnerBarCanvas = ({ barTotals, enableTotals, formatValue, + computeLabelLayout, ]) const handleMouseHover = useCallback( diff --git a/packages/bar/src/types.ts b/packages/bar/src/types.ts index 8a1326e88f..bfe37b395a 100644 --- a/packages/bar/src/types.ts +++ b/packages/bar/src/types.ts @@ -16,6 +16,7 @@ import { InheritedColorConfig, OrdinalColorScaleConfig } from '@nivo/colors' import { LegendProps } from '@nivo/legends' import { AnyScale, ScaleSpec, ScaleBandSpec } from '@nivo/scales' import { SpringValues } from '@react-spring/web' +import { BarLabelLayout } from './compute/common' export interface BarDatum { [key: string]: string | number @@ -186,10 +187,11 @@ export type RenderBarProps = Omit< | 'ariaLabel' | 'ariaLabelledBy' | 'ariaDescribedBy' -> & { - borderColor: string - labelColor: string -} +> & + BarLabelLayout & { + borderColor: string + labelColor: string + } export interface BarTooltipProps extends ComputedDatum { color: string From 9a6fb1fcbf8426792d35fbe8c6beb89be4d4e347 Mon Sep 17 00:00:00 2001 From: evasseure Date: Fri, 3 May 2024 15:15:45 +0100 Subject: [PATCH 4/9] Add tests --- packages/bar/tests/Bar.test.tsx | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/packages/bar/tests/Bar.test.tsx b/packages/bar/tests/Bar.test.tsx index 1fe68574c7..5c0b2e0eb9 100644 --- a/packages/bar/tests/Bar.test.tsx +++ b/packages/bar/tests/Bar.test.tsx @@ -2,6 +2,7 @@ import { mount } from 'enzyme' import { create, act, ReactTestRenderer, type ReactTestInstance } from 'react-test-renderer' import { LegendSvg, LegendSvgItem } from '@nivo/legends' import { Bar, BarDatum, BarItemProps, ComputedDatum, BarItem, BarTooltip, BarTotals } from '../' +import { useComputeLabelLayout } from '../src/compute/common' type IdValue = { id: string @@ -771,6 +772,84 @@ describe('totals layer', () => { }) }) +describe('labelPosition', () => { + it.each` + labelPosition | layout | expected + ${'start'} | ${'vertical'} | ${200} + ${'center'} | ${'vertical'} | ${100} + ${'end'} | ${'vertical'} | ${0} + ${'start'} | ${'horizontal'} | ${0} + ${'center'} | ${'horizontal'} | ${100} + ${'end'} | ${'horizontal'} | ${200} + `( + 'should position labels correctly on $layout charts when labelPosition=$labelPosition', + ({ labelPosition, layout, expected }) => { + const instance = create( + + ).root + + for (const bar of instance.findAllByType(BarItem)) { + const { labelX, labelY } = bar.props.style + if (layout === 'vertical') { + expect(labelY.animation.to).toBe(expected) + } else { + expect(labelX.animation.to).toBe(expected) + } + } + } + ) +}) + +describe('useComputeLabelLayout', () => { + it.each` + labelPosition | layout | offset | reverse | expectedValue | expectedTextAnchor + ${'start'} | ${'vertical'} | ${0} | ${false} | ${200} | ${'middle'} + ${'center'} | ${'vertical'} | ${0} | ${false} | ${100} | ${'middle'} + ${'end'} | ${'vertical'} | ${0} | ${false} | ${0} | ${'middle'} + ${'start'} | ${'horizontal'} | ${0} | ${false} | ${0} | ${'start'} + ${'center'} | ${'horizontal'} | ${0} | ${false} | ${100} | ${'middle'} + ${'end'} | ${'horizontal'} | ${0} | ${false} | ${200} | ${'start'} + ${'center'} | ${'vertical'} | ${-10} | ${false} | ${90} | ${'middle'} + ${'center'} | ${'vertical'} | ${10} | ${false} | ${110} | ${'middle'} + ${'center'} | ${'horizontal'} | ${-10} | ${false} | ${90} | ${'middle'} + ${'center'} | ${'horizontal'} | ${10} | ${false} | ${110} | ${'middle'} + ${'start'} | ${'vertical'} | ${0} | ${true} | ${0} | ${'middle'} + ${'center'} | ${'vertical'} | ${0} | ${true} | ${100} | ${'middle'} + ${'end'} | ${'vertical'} | ${0} | ${true} | ${200} | ${'middle'} + ${'start'} | ${'horizontal'} | ${0} | ${true} | ${200} | ${'start'} + ${'center'} | ${'horizontal'} | ${0} | ${true} | ${100} | ${'middle'} + ${'end'} | ${'horizontal'} | ${0} | ${true} | ${0} | ${'start'} + ${'center'} | ${'vertical'} | ${-10} | ${true} | ${90} | ${'middle'} + ${'center'} | ${'vertical'} | ${10} | ${true} | ${110} | ${'middle'} + ${'center'} | ${'horizontal'} | ${-10} | ${true} | ${90} | ${'middle'} + ${'center'} | ${'horizontal'} | ${10} | ${true} | ${110} | ${'middle'} + `( + 'should compute the correct label layout for (layout: $layout, labelPosition: $labelPosition, offset: $offset, reverse: $reverse)', + ({ labelPosition, layout, offset, reverse, expectedValue, expectedTextAnchor }) => { + const computeLabelLayout = useComputeLabelLayout(layout, reverse, labelPosition, offset) + const { labelX, labelY, textAnchor } = computeLabelLayout(200, 200) + if (layout === 'vertical') { + expect(labelY).toBe(expectedValue) + } else { + expect(labelX).toBe(expectedValue) + } + expect(textAnchor).toBe(expectedTextAnchor) + } + ) +}) + describe('tooltip', () => { it('should render a tooltip when hovering a slice', () => { let component: ReactTestRenderer From 416d4e5073504aa036438ae3e59046843b2dada5 Mon Sep 17 00:00:00 2001 From: evasseure Date: Fri, 3 May 2024 13:21:41 +0100 Subject: [PATCH 5/9] Storybook + website --- storybook/stories/bar/Bar.stories.tsx | 15 ++++++++++- website/src/data/components/bar/meta.yml | 2 ++ website/src/data/components/bar/props.ts | 32 ++++++++++++++++++++++++ website/src/pages/bar/api.tsx | 2 ++ website/src/pages/bar/canvas.js | 2 ++ website/src/pages/bar/index.js | 2 ++ 6 files changed, 54 insertions(+), 1 deletion(-) diff --git a/storybook/stories/bar/Bar.stories.tsx b/storybook/stories/bar/Bar.stories.tsx index 2c9ddb5f6f..fae22968b2 100644 --- a/storybook/stories/bar/Bar.stories.tsx +++ b/storybook/stories/bar/Bar.stories.tsx @@ -2,7 +2,7 @@ import type { Meta, StoryObj } from '@storybook/react' import { generateCountriesData, sets } from '@nivo/generators' import { random, range } from 'lodash' import { useTheme } from '@nivo/core' -import { Bar, BarDatum, BarItemProps } from '@nivo/bar' +import { Bar, BarCanvas, BarDatum, BarItemProps } from '@nivo/bar' import { AxisTickProps } from '@nivo/axes' const meta: Meta = { @@ -298,6 +298,19 @@ export const WithTotals: Story = { render: () => , } +export const WithTopLabels: Story = { + render: () => ( + + ), +} + const DataGenerator = (initialIndex, initialState) => { let index = initialIndex let state = initialState diff --git a/website/src/data/components/bar/meta.yml b/website/src/data/components/bar/meta.yml index 989f3706c5..d13f37f38d 100644 --- a/website/src/data/components/bar/meta.yml +++ b/website/src/data/components/bar/meta.yml @@ -36,6 +36,8 @@ Bar: link: bar--with-annotations - label: Using totals link: bar--with-totals + - label: Using top labels + link: bar--with-top-labels description: | Bar chart which can display multiple data series, stacked or side by side. Also supports both vertical and horizontal layout, with negative values descending diff --git a/website/src/data/components/bar/props.ts b/website/src/data/components/bar/props.ts index 2e5cd406b4..afe0f809fa 100644 --- a/website/src/data/components/bar/props.ts +++ b/website/src/data/components/bar/props.ts @@ -416,6 +416,38 @@ const props: ChartProperty[] = [ control: { type: 'inheritedColor' }, group: 'Labels', }, + { + key: 'labelPosition', + help: 'Defines the position of the label relative to its bar.', + type: `'start' | 'center' | 'end'`, + flavors: allFlavors, + required: false, + defaultValue: svgDefaultProps.labelPosition, + control: { + type: 'radio', + choices: [ + { label: 'start', value: 'start' }, + { label: 'center', value: 'center' }, + { label: 'end', value: 'end' }, + ], + }, + group: 'Labels', + }, + { + key: 'labelOffset', + help: 'Defines the vertical or horizontal (depends on layout) offset of the label.', + type: 'number', + flavors: ['svg', 'canvas', 'api'], + required: false, + defaultValue: svgDefaultProps.labelOffset, + control: { + type: 'range', + unit: 'px', + min: -16, + max: 16, + }, + group: 'Labels', + }, { key: 'enableTotals', help: 'Enable/disable totals labels.', diff --git a/website/src/pages/bar/api.tsx b/website/src/pages/bar/api.tsx index 198fe61915..dbc84596a2 100644 --- a/website/src/pages/bar/api.tsx +++ b/website/src/pages/bar/api.tsx @@ -117,6 +117,8 @@ const BarApi = () => { from: 'color', modifiers: [['darker', 1.6]], }, + labelPosition: 'center', + labelOffset: 0, }} /> diff --git a/website/src/pages/bar/canvas.js b/website/src/pages/bar/canvas.js index f053bd1d53..78cf372ce9 100644 --- a/website/src/pages/bar/canvas.js +++ b/website/src/pages/bar/canvas.js @@ -97,6 +97,8 @@ const initialProperties = { from: 'color', modifiers: [['darker', 1.6]], }, + labelPosition: 'center', + labelOffset: 0, isInteractive: true, 'custom tooltip example': false, diff --git a/website/src/pages/bar/index.js b/website/src/pages/bar/index.js index e7c91f2d13..c5af0ec674 100644 --- a/website/src/pages/bar/index.js +++ b/website/src/pages/bar/index.js @@ -115,6 +115,8 @@ const initialProperties = { from: 'color', modifiers: [['darker', 1.6]], }, + labelPosition: 'center', + labelOffset: 0, legends: [ { From 394999ab42e0deead0430b0b67f3b23d97628f1e Mon Sep 17 00:00:00 2001 From: evasseure Date: Tue, 7 May 2024 09:43:43 +0100 Subject: [PATCH 6/9] review: columns 3 for website --- website/src/data/components/bar/props.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/website/src/data/components/bar/props.ts b/website/src/data/components/bar/props.ts index afe0f809fa..0958aa6cd0 100644 --- a/website/src/data/components/bar/props.ts +++ b/website/src/data/components/bar/props.ts @@ -430,6 +430,7 @@ const props: ChartProperty[] = [ { label: 'center', value: 'center' }, { label: 'end', value: 'end' }, ], + columns: 3, }, group: 'Labels', }, From ca514d4c6141b6b82625f194037f581f53e2d88f Mon Sep 17 00:00:00 2001 From: evasseure Date: Tue, 7 May 2024 10:24:40 +0100 Subject: [PATCH 7/9] review: mirror offset when reversed --- packages/bar/src/compute/common.ts | 7 +++++-- packages/bar/tests/Bar.test.tsx | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/bar/src/compute/common.ts b/packages/bar/src/compute/common.ts index 21f4767dcc..924b447e1b 100644 --- a/packages/bar/src/compute/common.ts +++ b/packages/bar/src/compute/common.ts @@ -64,6 +64,9 @@ export function useComputeLabelLayout( labelOffset: BarCommonProps['labelOffset'] = defaultProps.labelOffset ): (width: number, height: number) => BarLabelLayout { return (width: number, height: number) => { + // If the chart is reversed, we want to make sure the offset is also reversed + const computedLabelOffset = labelOffset * (reverse ? -1 : 1) + if (layout === 'horizontal') { let x = width / 2 if (labelPosition === 'start') { @@ -72,7 +75,7 @@ export function useComputeLabelLayout( x = reverse ? 0 : width } return { - labelX: x + labelOffset, + labelX: x + computedLabelOffset, labelY: height / 2, textAnchor: labelPosition === 'center' ? 'middle' : 'start', } @@ -85,7 +88,7 @@ export function useComputeLabelLayout( } return { labelX: width / 2, - labelY: y + labelOffset, + labelY: y - computedLabelOffset, textAnchor: 'middle', } } diff --git a/packages/bar/tests/Bar.test.tsx b/packages/bar/tests/Bar.test.tsx index 5c0b2e0eb9..55e6bfd80e 100644 --- a/packages/bar/tests/Bar.test.tsx +++ b/packages/bar/tests/Bar.test.tsx @@ -821,8 +821,8 @@ describe('useComputeLabelLayout', () => { ${'start'} | ${'horizontal'} | ${0} | ${false} | ${0} | ${'start'} ${'center'} | ${'horizontal'} | ${0} | ${false} | ${100} | ${'middle'} ${'end'} | ${'horizontal'} | ${0} | ${false} | ${200} | ${'start'} - ${'center'} | ${'vertical'} | ${-10} | ${false} | ${90} | ${'middle'} - ${'center'} | ${'vertical'} | ${10} | ${false} | ${110} | ${'middle'} + ${'center'} | ${'vertical'} | ${-10} | ${false} | ${110} | ${'middle'} + ${'center'} | ${'vertical'} | ${10} | ${false} | ${90} | ${'middle'} ${'center'} | ${'horizontal'} | ${-10} | ${false} | ${90} | ${'middle'} ${'center'} | ${'horizontal'} | ${10} | ${false} | ${110} | ${'middle'} ${'start'} | ${'vertical'} | ${0} | ${true} | ${0} | ${'middle'} @@ -833,8 +833,8 @@ describe('useComputeLabelLayout', () => { ${'end'} | ${'horizontal'} | ${0} | ${true} | ${0} | ${'start'} ${'center'} | ${'vertical'} | ${-10} | ${true} | ${90} | ${'middle'} ${'center'} | ${'vertical'} | ${10} | ${true} | ${110} | ${'middle'} - ${'center'} | ${'horizontal'} | ${-10} | ${true} | ${90} | ${'middle'} - ${'center'} | ${'horizontal'} | ${10} | ${true} | ${110} | ${'middle'} + ${'center'} | ${'horizontal'} | ${-10} | ${true} | ${110} | ${'middle'} + ${'center'} | ${'horizontal'} | ${10} | ${true} | ${90} | ${'middle'} `( 'should compute the correct label layout for (layout: $layout, labelPosition: $labelPosition, offset: $offset, reverse: $reverse)', ({ labelPosition, layout, offset, reverse, expectedValue, expectedTextAnchor }) => { From 0c1fe536fb15bd14ff398600441331a1b9f1a89e Mon Sep 17 00:00:00 2001 From: evasseure Date: Tue, 7 May 2024 10:27:27 +0100 Subject: [PATCH 8/9] review: mirror text anchor while horizontal and reversed --- packages/bar/src/compute/common.ts | 4 ++-- packages/bar/tests/Bar.test.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/bar/src/compute/common.ts b/packages/bar/src/compute/common.ts index 924b447e1b..149cc26a5e 100644 --- a/packages/bar/src/compute/common.ts +++ b/packages/bar/src/compute/common.ts @@ -51,7 +51,7 @@ export const coerceValue = (value: T) => [value, Number(value)] as const export type BarLabelLayout = { labelX: number labelY: number - textAnchor: 'start' | 'middle' + textAnchor: 'start' | 'middle' | 'end' } /** @@ -77,7 +77,7 @@ export function useComputeLabelLayout( return { labelX: x + computedLabelOffset, labelY: height / 2, - textAnchor: labelPosition === 'center' ? 'middle' : 'start', + textAnchor: labelPosition === 'center' ? 'middle' : reverse ? 'end' : 'start', } } else { let y = height / 2 diff --git a/packages/bar/tests/Bar.test.tsx b/packages/bar/tests/Bar.test.tsx index 55e6bfd80e..ca6b178a91 100644 --- a/packages/bar/tests/Bar.test.tsx +++ b/packages/bar/tests/Bar.test.tsx @@ -828,9 +828,9 @@ describe('useComputeLabelLayout', () => { ${'start'} | ${'vertical'} | ${0} | ${true} | ${0} | ${'middle'} ${'center'} | ${'vertical'} | ${0} | ${true} | ${100} | ${'middle'} ${'end'} | ${'vertical'} | ${0} | ${true} | ${200} | ${'middle'} - ${'start'} | ${'horizontal'} | ${0} | ${true} | ${200} | ${'start'} + ${'start'} | ${'horizontal'} | ${0} | ${true} | ${200} | ${'end'} ${'center'} | ${'horizontal'} | ${0} | ${true} | ${100} | ${'middle'} - ${'end'} | ${'horizontal'} | ${0} | ${true} | ${0} | ${'start'} + ${'end'} | ${'horizontal'} | ${0} | ${true} | ${0} | ${'end'} ${'center'} | ${'vertical'} | ${-10} | ${true} | ${90} | ${'middle'} ${'center'} | ${'vertical'} | ${10} | ${true} | ${110} | ${'middle'} ${'center'} | ${'horizontal'} | ${-10} | ${true} | ${110} | ${'middle'} From f2776647a485cecab84766346a9be10ac606fa9d Mon Sep 17 00:00:00 2001 From: evasseure Date: Mon, 1 Jul 2024 15:31:03 +0100 Subject: [PATCH 9/9] review: center -> middle --- packages/bar/src/compute/common.ts | 2 +- packages/bar/src/props.ts | 2 +- packages/bar/src/types.ts | 2 +- packages/bar/tests/Bar.test.tsx | 28 ++++++++++++------------ website/src/data/components/bar/props.ts | 4 ++-- website/src/pages/bar/api.tsx | 2 +- website/src/pages/bar/canvas.js | 2 +- website/src/pages/bar/index.js | 2 +- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/bar/src/compute/common.ts b/packages/bar/src/compute/common.ts index 149cc26a5e..c851ce758b 100644 --- a/packages/bar/src/compute/common.ts +++ b/packages/bar/src/compute/common.ts @@ -77,7 +77,7 @@ export function useComputeLabelLayout( return { labelX: x + computedLabelOffset, labelY: height / 2, - textAnchor: labelPosition === 'center' ? 'middle' : reverse ? 'end' : 'start', + textAnchor: labelPosition === 'middle' ? 'middle' : reverse ? 'end' : 'start', } } else { let y = height / 2 diff --git a/packages/bar/src/props.ts b/packages/bar/src/props.ts index a75f57bc35..b9940fe5da 100644 --- a/packages/bar/src/props.ts +++ b/packages/bar/src/props.ts @@ -28,7 +28,7 @@ export const defaultProps = { enableLabel: true, label: 'formattedValue', - labelPosition: 'center' as const, + labelPosition: 'middle' as const, labelOffset: 0, labelSkipWidth: 0, labelSkipHeight: 0, diff --git a/packages/bar/src/types.ts b/packages/bar/src/types.ts index bfe37b395a..1a74a4dfe1 100644 --- a/packages/bar/src/types.ts +++ b/packages/bar/src/types.ts @@ -233,7 +233,7 @@ export type BarCommonProps = { enableLabel: boolean label: PropertyAccessor, string> - labelPosition: 'start' | 'center' | 'end' + labelPosition: 'start' | 'middle' | 'end' labelOffset: number labelFormat: string | LabelFormatter labelSkipWidth: number diff --git a/packages/bar/tests/Bar.test.tsx b/packages/bar/tests/Bar.test.tsx index ca6b178a91..e4d4f4dd1d 100644 --- a/packages/bar/tests/Bar.test.tsx +++ b/packages/bar/tests/Bar.test.tsx @@ -776,10 +776,10 @@ describe('labelPosition', () => { it.each` labelPosition | layout | expected ${'start'} | ${'vertical'} | ${200} - ${'center'} | ${'vertical'} | ${100} + ${'middle'} | ${'vertical'} | ${100} ${'end'} | ${'vertical'} | ${0} ${'start'} | ${'horizontal'} | ${0} - ${'center'} | ${'horizontal'} | ${100} + ${'middle'} | ${'horizontal'} | ${100} ${'end'} | ${'horizontal'} | ${200} `( 'should position labels correctly on $layout charts when labelPosition=$labelPosition', @@ -816,25 +816,25 @@ describe('useComputeLabelLayout', () => { it.each` labelPosition | layout | offset | reverse | expectedValue | expectedTextAnchor ${'start'} | ${'vertical'} | ${0} | ${false} | ${200} | ${'middle'} - ${'center'} | ${'vertical'} | ${0} | ${false} | ${100} | ${'middle'} + ${'middle'} | ${'vertical'} | ${0} | ${false} | ${100} | ${'middle'} ${'end'} | ${'vertical'} | ${0} | ${false} | ${0} | ${'middle'} ${'start'} | ${'horizontal'} | ${0} | ${false} | ${0} | ${'start'} - ${'center'} | ${'horizontal'} | ${0} | ${false} | ${100} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${0} | ${false} | ${100} | ${'middle'} ${'end'} | ${'horizontal'} | ${0} | ${false} | ${200} | ${'start'} - ${'center'} | ${'vertical'} | ${-10} | ${false} | ${110} | ${'middle'} - ${'center'} | ${'vertical'} | ${10} | ${false} | ${90} | ${'middle'} - ${'center'} | ${'horizontal'} | ${-10} | ${false} | ${90} | ${'middle'} - ${'center'} | ${'horizontal'} | ${10} | ${false} | ${110} | ${'middle'} + ${'middle'} | ${'vertical'} | ${-10} | ${false} | ${110} | ${'middle'} + ${'middle'} | ${'vertical'} | ${10} | ${false} | ${90} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${-10} | ${false} | ${90} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${10} | ${false} | ${110} | ${'middle'} ${'start'} | ${'vertical'} | ${0} | ${true} | ${0} | ${'middle'} - ${'center'} | ${'vertical'} | ${0} | ${true} | ${100} | ${'middle'} + ${'middle'} | ${'vertical'} | ${0} | ${true} | ${100} | ${'middle'} ${'end'} | ${'vertical'} | ${0} | ${true} | ${200} | ${'middle'} ${'start'} | ${'horizontal'} | ${0} | ${true} | ${200} | ${'end'} - ${'center'} | ${'horizontal'} | ${0} | ${true} | ${100} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${0} | ${true} | ${100} | ${'middle'} ${'end'} | ${'horizontal'} | ${0} | ${true} | ${0} | ${'end'} - ${'center'} | ${'vertical'} | ${-10} | ${true} | ${90} | ${'middle'} - ${'center'} | ${'vertical'} | ${10} | ${true} | ${110} | ${'middle'} - ${'center'} | ${'horizontal'} | ${-10} | ${true} | ${110} | ${'middle'} - ${'center'} | ${'horizontal'} | ${10} | ${true} | ${90} | ${'middle'} + ${'middle'} | ${'vertical'} | ${-10} | ${true} | ${90} | ${'middle'} + ${'middle'} | ${'vertical'} | ${10} | ${true} | ${110} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${-10} | ${true} | ${110} | ${'middle'} + ${'middle'} | ${'horizontal'} | ${10} | ${true} | ${90} | ${'middle'} `( 'should compute the correct label layout for (layout: $layout, labelPosition: $labelPosition, offset: $offset, reverse: $reverse)', ({ labelPosition, layout, offset, reverse, expectedValue, expectedTextAnchor }) => { diff --git a/website/src/data/components/bar/props.ts b/website/src/data/components/bar/props.ts index 0958aa6cd0..d6312df6aa 100644 --- a/website/src/data/components/bar/props.ts +++ b/website/src/data/components/bar/props.ts @@ -419,7 +419,7 @@ const props: ChartProperty[] = [ { key: 'labelPosition', help: 'Defines the position of the label relative to its bar.', - type: `'start' | 'center' | 'end'`, + type: `'start' | 'middle' | 'end'`, flavors: allFlavors, required: false, defaultValue: svgDefaultProps.labelPosition, @@ -427,7 +427,7 @@ const props: ChartProperty[] = [ type: 'radio', choices: [ { label: 'start', value: 'start' }, - { label: 'center', value: 'center' }, + { label: 'middle', value: 'middle' }, { label: 'end', value: 'end' }, ], columns: 3, diff --git a/website/src/pages/bar/api.tsx b/website/src/pages/bar/api.tsx index dbc84596a2..742cabab31 100644 --- a/website/src/pages/bar/api.tsx +++ b/website/src/pages/bar/api.tsx @@ -117,7 +117,7 @@ const BarApi = () => { from: 'color', modifiers: [['darker', 1.6]], }, - labelPosition: 'center', + labelPosition: 'middle', labelOffset: 0, }} /> diff --git a/website/src/pages/bar/canvas.js b/website/src/pages/bar/canvas.js index 78cf372ce9..55ac6e11f3 100644 --- a/website/src/pages/bar/canvas.js +++ b/website/src/pages/bar/canvas.js @@ -97,7 +97,7 @@ const initialProperties = { from: 'color', modifiers: [['darker', 1.6]], }, - labelPosition: 'center', + labelPosition: 'middle', labelOffset: 0, isInteractive: true, diff --git a/website/src/pages/bar/index.js b/website/src/pages/bar/index.js index c5af0ec674..c440ea6afa 100644 --- a/website/src/pages/bar/index.js +++ b/website/src/pages/bar/index.js @@ -115,7 +115,7 @@ const initialProperties = { from: 'color', modifiers: [['darker', 1.6]], }, - labelPosition: 'center', + labelPosition: 'middle', labelOffset: 0, legends: [