From 4681f4644f069601dbfff04dc1425a820b7cf11b Mon Sep 17 00:00:00 2001 From: Marshall Peterson Date: Fri, 8 Mar 2024 16:17:58 -0700 Subject: [PATCH 1/2] feat: add numberFormat presets --- src/components/Axis/Axis.tsx | 2 +- src/specBuilder/axis/axisLabelUtils.test.ts | 24 +++++++++-- src/specBuilder/axis/axisLabelUtils.ts | 41 ++++++++++++++----- .../axis/axisReferenceLineUtils.test.ts | 1 + src/specBuilder/axis/axisSpecBuilder.ts | 4 +- src/specBuilder/axis/axisTestUtils.ts | 1 + src/specBuilder/axis/axisUtils.test.ts | 2 + src/specBuilder/specUtils.test.ts | 9 ++++ src/specBuilder/specUtils.ts | 18 ++++++++ .../trendlineAnnotationUtils.test.ts | 4 -- .../trendlineAnnotationUtils.ts | 6 +-- src/stories/components/Axis/Axis.story.tsx | 17 +++++++- src/stories/components/Axis/Axis.test.tsx | 36 +++++++++++++++- src/types/Chart.ts | 3 +- src/types/specBuilderTypes.ts | 1 + 15 files changed, 141 insertions(+), 28 deletions(-) diff --git a/src/components/Axis/Axis.tsx b/src/components/Axis/Axis.tsx index 0308f6a91..a4529b625 100644 --- a/src/components/Axis/Axis.tsx +++ b/src/components/Axis/Axis.tsx @@ -34,7 +34,7 @@ const Axis: FC = ({ labelFormat, labelOrientation = DEFAULT_LABEL_ORIENTATION, labels, - numberFormat, + numberFormat = 'shortNumber', range = undefined, subLabels, ticks = false, diff --git a/src/specBuilder/axis/axisLabelUtils.test.ts b/src/specBuilder/axis/axisLabelUtils.test.ts index d080f0278..29cd6f7bc 100644 --- a/src/specBuilder/axis/axisLabelUtils.test.ts +++ b/src/specBuilder/axis/axisLabelUtils.test.ts @@ -15,6 +15,7 @@ import { getLabelAnchorValues, getLabelAngle, getLabelFormat, + getLabelNumberFormat, getLabelOffset, getLabelValue, labelIsParallelToAxis, @@ -166,9 +167,7 @@ describe('getLabelFormat()', () => { expect(labelFormat[0]).toEqual({ test: 'isNumber(datum.value)', signal: "format(datum.value, '.2f')" }); }); test('should not include the number format test if numberFormat does not exist or is an empty string', () => { - expect( - getLabelFormat({ ...defaultAxisProps, labelFormat: 'linear', numberFormat: undefined }, 'xLinear') - ).toHaveLength(3); + expect(getLabelFormat({ ...defaultAxisProps, labelFormat: 'linear' }, 'xLinear')).toHaveLength(3); expect( getLabelFormat({ ...defaultAxisProps, labelFormat: 'linear', numberFormat: '' }, 'xLinear') ).toHaveLength(3); @@ -204,3 +203,22 @@ describe('getLabelFormat()', () => { ); }); }); + +describe('getLabelNumberFormat()', () => { + test('should return correct signal for shortNumber', () => { + expect(getLabelNumberFormat('shortNumber')).toHaveProperty( + 'signal', + "upper(replace(format(datum.value, '.3~s'), /(\\d+)G/, '$1B'))" + ); + }); + test('should return correct signal for shortCurrency', () => { + expect(getLabelNumberFormat('shortCurrency')).toHaveProperty( + 'signal', + "upper(replace(format(datum.value, '$.3~s'), /(\\d+)G/, '$1B'))" + ); + }); + test('should return correct signal for string specifier', () => { + const numberFormat = '.2f'; + expect(getLabelNumberFormat(numberFormat)).toHaveProperty('signal', `format(datum.value, '${numberFormat}')`); + }); +}); diff --git a/src/specBuilder/axis/axisLabelUtils.ts b/src/specBuilder/axis/axisLabelUtils.ts index 21c57af34..cbbaacfb7 100644 --- a/src/specBuilder/axis/axisLabelUtils.ts +++ b/src/specBuilder/axis/axisLabelUtils.ts @@ -9,7 +9,8 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ -import { AxisSpecProps, Granularity, Label, LabelAlign, Orientation, Position } from 'types'; +import { getD3FormatSpecifierFromNumberFormat } from '@specBuilder/specUtils'; +import { AxisSpecProps, Granularity, Label, LabelAlign, NumberFormat, Orientation, Position } from 'types'; import { Align, Baseline, @@ -241,23 +242,41 @@ export const getLabelFormat = ( return { signal: 'formatTimeDurationLabels(datum)' }; } - // if it's a number and greater than or equal to 1000, we want to format it in scientific notation (but with B instead of G) ex. 1K, 20M, 1.3B return [ - ...(numberFormat ? [{ test: 'isNumber(datum.value)', signal: `format(datum.value, '${numberFormat}')` }] : []), - { - test: 'isNumber(datum.value) && abs(datum.value) >= 1000', - signal: "upper(replace(format(datum.value, '.3~s'), 'G', 'B'))", - }, - { - test: 'isNumber(datum.value)', - signal: 'format(datum.value, ",")', - }, + getLabelNumberFormat(numberFormat), ...(truncateLabels && scaleName.includes('Band') && labelIsParallelToAxis(position, labelOrientation) ? [{ signal: 'truncateText(datum.value, bandwidth("xBand")/(1- paddingInner), "normal", 14)' }] : [{ signal: 'datum.value' }]), ]; }; +/** + * gets the number format tests and signals based on the numberFormat + * @param numberFormat + * @returns + */ +export const getLabelNumberFormat = ( + numberFormat: NumberFormat | string +): { + test?: string; +} & TextValueRef => { + const defaultTest = 'isNumber(datum.value)'; + if (numberFormat === 'shortNumber') { + return { + test: `${defaultTest} && abs(datum.value) >= 1000`, + signal: "upper(replace(format(datum.value, '.3~s'), /(\\d+)G/, '$1B'))", + }; + } + if (numberFormat === 'shortCurrency') { + return { + test: `${defaultTest} && abs(datum.value) >= 1000`, + signal: "upper(replace(format(datum.value, '$.3~s'), /(\\d+)G/, '$1B'))", + }; + } + const d3FormatSpecifier = getD3FormatSpecifierFromNumberFormat(numberFormat); + return { test: defaultTest, signal: `format(datum.value, '${d3FormatSpecifier}')` }; +}; + /** * Gets the axis label encoding * @param labelAlign diff --git a/src/specBuilder/axis/axisReferenceLineUtils.test.ts b/src/specBuilder/axis/axisReferenceLineUtils.test.ts index 8665053f8..46cd95fae 100644 --- a/src/specBuilder/axis/axisReferenceLineUtils.test.ts +++ b/src/specBuilder/axis/axisReferenceLineUtils.test.ts @@ -49,6 +49,7 @@ const defaultAxisProps: AxisSpecProps = { labelFontWeight: DEFAULT_LABEL_FONT_WEIGHT, labelOrientation: DEFAULT_LABEL_ORIENTATION, labels: [], + numberFormat: 'shortNumber', position: 'bottom', scaleType: 'linear', subLabels: [], diff --git a/src/specBuilder/axis/axisSpecBuilder.ts b/src/specBuilder/axis/axisSpecBuilder.ts index f46cca827..beb997626 100644 --- a/src/specBuilder/axis/axisSpecBuilder.ts +++ b/src/specBuilder/axis/axisSpecBuilder.ts @@ -62,6 +62,7 @@ export const addAxis = produce { labelOrientation: 'horizontal', labels: [], name: 'axis0', + numberFormat: 'shortNumber', position: 'left', scaleType: 'linear', subLabels: [], @@ -112,6 +113,7 @@ describe('getDefaultAxis()', () => { labelOrientation: 'horizontal', labels: [], name: 'axis0', + numberFormat: 'shortNumber', position: 'left', scaleType: 'point', subLabels: [], diff --git a/src/specBuilder/specUtils.test.ts b/src/specBuilder/specUtils.test.ts index faeb888bc..f3eccc8b0 100644 --- a/src/specBuilder/specUtils.test.ts +++ b/src/specBuilder/specUtils.test.ts @@ -23,6 +23,7 @@ import { BandScale, OrdinalScale } from 'vega'; import { getColorValue, + getD3FormatSpecifierFromNumberFormat, getDimensionField, getFacetsFromProps, getFacetsFromScales, @@ -195,4 +196,12 @@ describe('specUtils', () => { ); }); }); + + describe('getD3FormatSpecifierFromNumberFormat()', () => { + test('should return proper formats', () => { + expect(getD3FormatSpecifierFromNumberFormat('currency')).toEqual('$,.2f'); + expect(getD3FormatSpecifierFromNumberFormat('standardNumber')).toEqual(','); + expect(getD3FormatSpecifierFromNumberFormat(',.2f')).toEqual(',.2f'); + }); + }); }); diff --git a/src/specBuilder/specUtils.ts b/src/specBuilder/specUtils.ts index 3674471e7..b0c08b161 100644 --- a/src/specBuilder/specUtils.ts +++ b/src/specBuilder/specUtils.ts @@ -20,6 +20,7 @@ import { LineType, LineTypeFacet, LineWidth, + NumberFormat, OpacityFacet, SpectrumColor, SymbolSize, @@ -294,3 +295,20 @@ export const mergeValuesIntoData = (data, values) => { export const getDimensionField = (dimension: string, scaleType?: ScaleType) => { return scaleType === 'time' ? DEFAULT_TRANSFORMED_TIME_DIMENSION : dimension; }; + +/** + * Gets the d3 format specifier for named number formats. + * shortNumber and shortCurrency are not included since these require additional logic + * @param numberFormat + * @returns + */ +export const getD3FormatSpecifierFromNumberFormat = (numberFormat: NumberFormat | string): string => { + switch (numberFormat) { + case 'currency': + return '$,.2f'; // currency format + case 'standardNumber': + return ','; // standard number format + default: + return numberFormat; + } +}; diff --git a/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.test.ts b/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.test.ts index 941dd7de2..478da8587 100644 --- a/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.test.ts +++ b/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.test.ts @@ -155,10 +155,6 @@ describe('getTrendlineAnnotationTextMark()', () => { const textMark = getTrendlineAnnotationTextMark({ ...defaultAnnotationProps, prefix: '' }); expect(textMark.encode?.enter?.text).toHaveProperty('signal', `format(datum.datum.${TRENDLINE_VALUE}, '')`); }); - test('should increase offset if badge is true', () => { - const textMark = getTrendlineAnnotationTextMark({ ...defaultAnnotationProps, badge: true }); - expect(textMark.transform?.[0]).toHaveProperty('offset', [4, 4, 4, 4, 5.65, 5.65, 5.65, 5.65]); - }); }); describe('getTextFill()', () => { diff --git a/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.ts b/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.ts index aa0e6e69a..3141796c1 100644 --- a/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.ts +++ b/src/specBuilder/trendlineAnnotation/trendlineAnnotationUtils.ts @@ -191,10 +191,8 @@ export const getTrendlineAnnotationPointY = ({ * @returns TextMark */ export const getTrendlineAnnotationTextMark = (annotation: TrendlineAnnotationSpecProps): TextMark => { - const { badge, name, numberFormat, prefix, trendlineName, markName } = annotation; + const { name, numberFormat, prefix, trendlineName, markName } = annotation; const textPrefix = prefix ? `'${prefix} ' + ` : ''; - // need more offset if there is a badge to make room for said badge - const offset = badge ? [4, 4, 4, 4, 5.65, 5.65, 5.65, 5.65] : [2, 2, 2, 2, 2.83, 2.83, 2.83, 2.83]; const fill = getTextFill({ ...annotation }); return { name, @@ -212,7 +210,7 @@ export const getTrendlineAnnotationTextMark = (annotation: TrendlineAnnotationSp type: 'label', size: { signal: '[width, height]' }, avoidMarks: [trendlineName, `${markName}_group`], - offset, + offset: [6, 6, 6, 6, 8.49, 8.49, 8.49, 8.49], anchor: ['top', 'bottom', 'right', 'left', 'top-right', 'top-left', 'bottom-right', 'bottom-left'], }, ], diff --git a/src/stories/components/Axis/Axis.story.tsx b/src/stories/components/Axis/Axis.story.tsx index a775eec2d..70f9e6b34 100644 --- a/src/stories/components/Axis/Axis.story.tsx +++ b/src/stories/components/Axis/Axis.story.tsx @@ -24,6 +24,20 @@ import timeData from './timeData.json'; export default { title: 'RSC/Axis', component: Axis, + argTypes: { + lineType: { + control: 'select', + options: ['solid', 'dashed', 'dotted', 'dotDash', 'shortDash', 'longDash', 'twoDash'], + }, + lineWidth: { + control: 'inline-radio', + options: ['XS', 'S', 'M', 'L', 'XL'], + }, + numberFormat: { + control: 'select', + options: ['currency', 'shortCurrency', 'shortNumber', 'standardNumber', '$,.2f', ',.2%', '.3s'], + }, + }, }; const data = [ @@ -191,13 +205,14 @@ NonLinearAxis.args = { const NumberFormat = bindWithProps(AxisStory); NumberFormat.args = { - numberFormat: '$,.2f', + numberFormat: 'shortCurrency', position: 'left', baseline: true, grid: true, labelFormat: 'linear', ticks: true, title: 'Price', + range: [0, 2000000], }; const CustomXRange = bindWithProps(LinearAxisStory); diff --git a/src/stories/components/Axis/Axis.test.tsx b/src/stories/components/Axis/Axis.test.tsx index 8e3f83775..45d653685 100644 --- a/src/stories/components/Axis/Axis.test.tsx +++ b/src/stories/components/Axis/Axis.test.tsx @@ -168,13 +168,45 @@ describe('Axis', () => { describe('NumberFormat', () => { test('Should render the number format provided', async () => { - render(); + render(); const chart = await findChart(); expect(chart).toBeInTheDocument(); - expect(screen.getByText('$1.00')).toBeInTheDocument(); + expect(screen.getByText('2,000,000.00')).toBeInTheDocument(); + expect(screen.getByText('0.00')).toBeInTheDocument(); + }); + test('currency', async () => { + render(); + const chart = await findChart(); + expect(chart).toBeInTheDocument(); + + expect(screen.getByText('$2,000,000.00')).toBeInTheDocument(); expect(screen.getByText('$0.00')).toBeInTheDocument(); }); + test('shortCurrency', async () => { + render(); + const chart = await findChart(); + expect(chart).toBeInTheDocument(); + + expect(screen.getByText('$2M')).toBeInTheDocument(); + expect(screen.getByText('$500K')).toBeInTheDocument(); + }); + test('shortNumber', async () => { + render(); + const chart = await findChart(); + expect(chart).toBeInTheDocument(); + + expect(screen.getByText('2M')).toBeInTheDocument(); + expect(screen.getByText('500K')).toBeInTheDocument(); + }); + test('standardNumber', async () => { + render(); + const chart = await findChart(); + expect(chart).toBeInTheDocument(); + + expect(screen.getByText('2,000,000')).toBeInTheDocument(); + expect(screen.getByText('500,000')).toBeInTheDocument(); + }); }); describe('DurationLabelFormat', () => { diff --git a/src/types/Chart.ts b/src/types/Chart.ts index 841e9a12b..e65d1c06e 100644 --- a/src/types/Chart.ts +++ b/src/types/Chart.ts @@ -88,6 +88,7 @@ export type LineTypes = LineType[] | LineType[][]; export type Opacities = number[] | number[][]; export type SymbolShapes = ChartSymbolShape[] | ChartSymbolShape[][]; export type ChartSymbolShape = 'rounded-square' | SymbolShape; +export type NumberFormat = 'currency' | 'shortCurrency' | 'shortNumber' | 'standardNumber'; export interface SharedChartProps extends SpecProps { /** Vega config that can be used to tweak the style of the chart. @see https://vega.github.io/vega/docs/config/ */ @@ -212,7 +213,7 @@ export interface AxisProps extends BaseProps { * * see {@link https://d3js.org/d3-format#locale_format} */ - numberFormat?: string; + numberFormat?: NumberFormat | string; /** The minimum and maximum values for the axis, for example: `[-10, 10]`. * * Note: This prop is only supported for axes with `linear` or `time` scale types. diff --git a/src/types/specBuilderTypes.ts b/src/types/specBuilderTypes.ts index 55491b354..2a5b56246 100644 --- a/src/types/specBuilderTypes.ts +++ b/src/types/specBuilderTypes.ts @@ -54,6 +54,7 @@ type AxisPropsWithDefaults = | 'labelFontWeight' | 'labelOrientation' | 'labels' + | 'numberFormat' | 'subLabels' | 'ticks'; From cc2852b4b1709c9b147d1415122c91cba8ae7c18 Mon Sep 17 00:00:00 2001 From: Marshall Peterson Date: Fri, 8 Mar 2024 16:34:42 -0700 Subject: [PATCH 2/2] fix: tests --- src/specBuilder/axis/axisLabelUtils.test.ts | 24 ++++---------------- src/specBuilder/axis/axisSpecBuilder.test.ts | 12 ++++------ src/specBuilder/axis/axisUtils.test.ts | 12 ++-------- 3 files changed, 11 insertions(+), 37 deletions(-) diff --git a/src/specBuilder/axis/axisLabelUtils.test.ts b/src/specBuilder/axis/axisLabelUtils.test.ts index 29cd6f7bc..f97b8ecf2 100644 --- a/src/specBuilder/axis/axisLabelUtils.test.ts +++ b/src/specBuilder/axis/axisLabelUtils.test.ts @@ -158,27 +158,13 @@ describe('getLabelAngle', () => { }); describe('getLabelFormat()', () => { - test('should include the number format test if numberFormat exists', () => { - const labelFormat = getLabelFormat( - { ...defaultAxisProps, labelFormat: 'linear', numberFormat: '.2f' }, - 'xLinear' - ); - expect(labelFormat).toHaveLength(4); - expect(labelFormat[0]).toEqual({ test: 'isNumber(datum.value)', signal: "format(datum.value, '.2f')" }); - }); - test('should not include the number format test if numberFormat does not exist or is an empty string', () => { - expect(getLabelFormat({ ...defaultAxisProps, labelFormat: 'linear' }, 'xLinear')).toHaveLength(3); - expect( - getLabelFormat({ ...defaultAxisProps, labelFormat: 'linear', numberFormat: '' }, 'xLinear') - ).toHaveLength(3); - }); test('should include text truncation if truncateText is true', () => { const labelEncodings = getLabelFormat({ ...defaultAxisProps, truncateLabels: true }, 'xBand'); - expect(labelEncodings).toHaveLength(3); - expect(labelEncodings[2].signal).toContain('truncateText'); + expect(labelEncodings).toHaveLength(2); + expect(labelEncodings[1].signal).toContain('truncateText'); }); test('should not include text truncation if the scale name does not include band', () => { - expect(getLabelFormat({ ...defaultAxisProps, truncateLabels: true }, 'xLinear')[2].signal).not.toContain( + expect(getLabelFormat({ ...defaultAxisProps, truncateLabels: true }, 'xLinear')[1].signal).not.toContain( 'truncateText' ); }); @@ -187,13 +173,13 @@ describe('getLabelFormat()', () => { getLabelFormat( { ...defaultAxisProps, truncateLabels: true, position: 'bottom', labelOrientation: 'vertical' }, 'xBand' - )[2].signal + )[1].signal ).not.toContain('truncateText'); expect( getLabelFormat( { ...defaultAxisProps, truncateLabels: true, position: 'left', labelOrientation: 'horizontal' }, 'yBand' - )[2].signal + )[1].signal ).not.toContain('truncateText'); }); test('should return duration formatter if labelFormat is duration', () => { diff --git a/src/specBuilder/axis/axisSpecBuilder.test.ts b/src/specBuilder/axis/axisSpecBuilder.test.ts index 20d4a8aa6..5366c007d 100644 --- a/src/specBuilder/axis/axisSpecBuilder.test.ts +++ b/src/specBuilder/axis/axisSpecBuilder.test.ts @@ -49,11 +49,7 @@ const defaultAxis: Axis = { text: [ { test: 'isNumber(datum.value) && abs(datum.value) >= 1000', - signal: "upper(replace(format(datum.value, '.3~s'), 'G', 'B'))", - }, - { - test: 'isNumber(datum.value)', - signal: 'format(datum.value, ",")', + signal: "upper(replace(format(datum.value, '.3~s'), /(\\d+)G/, '$1B'))", }, { signal: 'datum.value' }, ], @@ -279,7 +275,7 @@ describe('Spec builder, Axis', () => { scaleName: 'xLinear', scaleType: 'linear', })[0].encode?.labels?.update?.text as ProductionRule; - expect(labelTextEncoding).toHaveLength(4); + expect(labelTextEncoding).toHaveLength(3); expect(labelTextEncoding[0]).toEqual({ test: "abs(scale('xLinear', 10) - scale('xLinear', datum.value)) < 30", value: '', @@ -296,8 +292,8 @@ describe('Spec builder, Axis', () => { scaleType: 'linear', })[0].encode?.labels?.update?.text as ProductionRule; - // 2 tests for the two reference lines plus 3 default tests = 5 tests - expect(labelTextEncoding).toHaveLength(5); + // 2 tests for the two reference lines plus 2 default tests = 4 tests + expect(labelTextEncoding).toHaveLength(4); }); test('should set the values on the axis if labels is set', () => { const axes = addAxes([], { diff --git a/src/specBuilder/axis/axisUtils.test.ts b/src/specBuilder/axis/axisUtils.test.ts index 6f6f0ea74..7cb381a0b 100644 --- a/src/specBuilder/axis/axisUtils.test.ts +++ b/src/specBuilder/axis/axisUtils.test.ts @@ -82,11 +82,7 @@ describe('getDefaultAxis()', () => { text: [ { test: 'isNumber(datum.value) && abs(datum.value) >= 1000', - signal: "upper(replace(format(datum.value, '.3~s'), 'G', 'B'))", - }, - { - test: 'isNumber(datum.value)', - signal: 'format(datum.value, ",")', + signal: "upper(replace(format(datum.value, '.3~s'), /(\\d+)G/, '$1B'))", }, { signal: 'datum.value', @@ -146,11 +142,7 @@ describe('getDefaultAxis()', () => { text: [ { test: 'isNumber(datum.value) && abs(datum.value) >= 1000', - signal: "upper(replace(format(datum.value, '.3~s'), 'G', 'B'))", - }, - { - test: 'isNumber(datum.value)', - signal: 'format(datum.value, ",")', + signal: "upper(replace(format(datum.value, '.3~s'), /(\\d+)G/, '$1B'))", }, { signal: 'datum.value',