Skip to content

CHARTS-176: Fix zero-value bars not visible in small chart heights #47477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kangzj wants to merge 9 commits into
base: trunk
from
+260 −24
Open

CHARTS-176: Fix zero-value bars not visible in small chart heights #47477

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e18300e
CHARTS-176: Fix zero-value bars not visible in small chart heights
kangzj Mar 6, 2026
22f00c8
Move small chart example into ZeroValueComparison story
kangzj Mar 6, 2026
da9cf39
Address Copilot feedback: rename chartHeight to valueAxisLength, clam…
kangzj Mar 6, 2026
2962517
Simplify useZeroValueDisplay: just calculate 3px equivalent value
kangzj Mar 6, 2026
792e7c4
Cap zero-value visualValue at smallest non-zero value
kangzj Mar 6, 2026
44043fd
Boost any value below 3px threshold, not just zero
kangzj Mar 6, 2026
d1622dd
Differentiate zero (2px) from near-zero (3px) bars
kangzj Mar 6, 2026
d96b104
Clamp visual values to prevent exceeding maxAbsoluteValue
kangzj Mar 6, 2026
10ac62e
Address review feedback from Adam
kangzj Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Boost any value below 3px threshold, not just zero
  • Loading branch information
@kangzj
kangzj committed Mar 6, 2026
commit 44043fd0345f1ff4b2c8b8d5f6d4597cdfb1ecf7
71 changes: 38 additions & 33 deletions projects/js-packages/charts/src/hooks/test/use-zero-value-display.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,66 +26,72 @@ describe( 'useZeroValueDisplay', () => {
expect( result.current ).toBe( mockData );
} );

test( 'adds visualValue for zero values when enabled', () => {
test( 'adds visualValue for zero values', () => {
const { result } = renderHook( () =>
useZeroValueDisplay( mockData, { enabled: true, valueAxisLength: 100 } )
);

const enhancedData = result.current;
expect( enhancedData ).not.toBe( mockData );
expect( enhancedData[ 0 ].data[ 0 ] ).toHaveProperty( 'visualValue' );
expect(
( enhancedData[ 0 ].data[ 0 ] as { visualValue?: number } ).visualValue
).toBeGreaterThan( 0 );
} );

test( 'does not add visualValue for non-zero values', () => {
const { result } = renderHook( () =>
useZeroValueDisplay( mockData, { enabled: true, valueAxisLength: 100 } )
);

const enhancedData = result.current;
expect( enhancedData[ 0 ].data[ 1 ] ).not.toHaveProperty( 'visualValue' );
expect( enhancedData[ 0 ].data[ 2 ] ).not.toHaveProperty( 'visualValue' );
} );
test( 'adds visualValue for near-zero values that would render below minimum', () => {
const data: SeriesData[] = [
{
label: 'Series 1',
data: [
{ label: 'A', value: 1 }, // Would render as 1px (below 3px minimum)
{ label: 'B', value: 100 },
],
},
];

test( 'calculates visualValue as 3px equivalent, capped at min value', () => {
// mockData has values [0, 100, 200]
// pixelBasedValue = (3 / 100) * 200 = 6
// But min non-zero value is 100, so visualValue = min(6, 100) = 6
// With axis=100 and max=100, 3px threshold = 3
// Value of 1 < 3, so it gets boosted
const { result } = renderHook( () =>
useZeroValueDisplay( mockData, { enabled: true, valueAxisLength: 100 } )
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 100 } )
);

const visualValue = ( result.current[ 0 ].data[ 0 ] as { visualValue?: number } ).visualValue;
expect( visualValue ).toBe( 6 );
const enhancedData = result.current;
expect( enhancedData[ 0 ].data[ 0 ] ).toHaveProperty( 'visualValue' );
expect( ( enhancedData[ 0 ].data[ 0 ] as { visualValue?: number } ).visualValue ).toBe( 3 );
} );

test( 'caps visualValue at smallest non-zero value', () => {
// When 3px equivalent would exceed the smallest bar, cap it
test( 'does not add visualValue for values above minimum threshold', () => {
const data: SeriesData[] = [
{
label: 'Series 1',
data: [
{ label: 'A', value: 0 },
{ label: 'B', value: 2 }, // small value
{ label: 'C', value: 100 },
{ label: 'A', value: 10 }, // Would render as 10px (above 3px minimum)
{ label: 'B', value: 100 },
],
},
];

// pixelBasedValue = (3 / 100) * 100 = 3
// minAbsoluteValue = 2
// visualValue = min(3, 2) = 2 (capped so zero doesn't exceed smallest bar)
const { result } = renderHook( () =>
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 100 } )
);

const enhancedData = result.current;
expect( enhancedData[ 0 ].data[ 0 ] ).not.toHaveProperty( 'visualValue' );
expect( enhancedData[ 0 ].data[ 1 ] ).not.toHaveProperty( 'visualValue' );
} );

test( 'calculates visualValue as 3px equivalent', () => {
// mockData has values [0, 100, 200], max = 200
// minVisibleValue = (3 / 100) * 200 = 6
const { result } = renderHook( () =>
useZeroValueDisplay( mockData, { enabled: true, valueAxisLength: 100 } )
);

const visualValue = ( result.current[ 0 ].data[ 0 ] as { visualValue?: number } ).visualValue;
expect( visualValue ).toBe( 2 );
expect( visualValue ).toBe( 6 );
} );

test( 'scales visualValue based on axis length', () => {
test( 'scales minVisibleValue based on axis length', () => {
const data: SeriesData[] = [
{
label: 'Series 1',
Expand All @@ -96,11 +102,11 @@ describe( 'useZeroValueDisplay', () => {
},
];

// Small axis = larger pixelBasedValue, but capped at minValue (100)
// Small axis = larger minVisibleValue (to ensure 3px)
const { result: smallAxis } = renderHook( () =>
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 50 } )
);
// Large axis = smaller visualValue
// Large axis = smaller minVisibleValue
const { result: largeAxis } = renderHook( () =>
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 200 } )
);
Expand All @@ -110,8 +116,7 @@ describe( 'useZeroValueDisplay', () => {
const largeAxisValue = ( largeAxis.current[ 0 ].data[ 0 ] as { visualValue?: number } )
.visualValue;

// 3/50 * 100 = 6, but capped at 100 (only non-zero value) → 6
// 3/200 * 100 = 1.5, capped at 100 → 1.5
// 3/50 * 100 = 6 vs 3/200 * 100 = 1.5
expect( smallAxisValue ).toBe( 6 );
expect( largeAxisValue ).toBe( 1.5 );
} );
Expand All @@ -131,7 +136,7 @@ describe( 'useZeroValueDisplay', () => {
useZeroValueDisplay( zeroOnlyData, { enabled: true, valueAxisLength: 100 } )
);

// Should return original data since there are no non-zero values
// Should return original data since there are no non-zero values to calculate from
expect( result.current ).toBe( zeroOnlyData );
} );
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new hook tests don’t cover negative near-zero values or null values. Given BarChart supports negatives and DataPointDate.value is number | null, add cases to ensure boosted visualValue preserves sign and that null values remain untouched (no visualValue added).

Suggested change
} );
} );
test( 'negative near-zero values preserve sign when boosted', () => {
const data: SeriesData[] = [
{
label: 'Series 1',
data: [
{ label: 'NegativeNearZero', value: -1 }, // Would render as -1px
{ label: 'NegativeLarge', value: -100 },
{ label: 'PositiveMax', value: 100 },
],
},
];
const { result } = renderHook( () =>
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 100 } )
);
const negativeNearZero = result.current[ 0 ].data[ 0 ] as { visualValue?: number };
// Should have a boosted magnitude but keep the negative sign (e.g., -3px equivalent)
expect( negativeNearZero.visualValue ).toBeLessThan( 0 );
expect( Math.abs( negativeNearZero.visualValue! ) ).toBeGreaterThan( 1 );
} );
test( 'null values remain untouched without visualValue', () => {
const data: SeriesData[] = [
{
label: 'Series 1',
data: [
{ label: 'NullValue', value: null },
{ label: 'NonZero', value: 50 },
],
},
];
const { result } = renderHook( () =>
useZeroValueDisplay( data, { enabled: true, valueAxisLength: 100 } )
);
const nullPoint = result.current[ 0 ].data[ 0 ] as { value: number | null; visualValue?: number };
// The original null value should be preserved and no visualValue should be added
expect( nullPoint.value ).toBeNull();
expect( 'visualValue' in nullPoint ).toBe( false );
} );

Copilot uses AI. Check for mistakes.
} );
22 changes: 10 additions & 12 deletions projects/js-packages/charts/src/hooks/use-zero-value-display.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ export interface UseZeroValueDisplayOptions {
}

/**
* Minimum pixel height for zero-value bars to ensure visibility.
* Minimum pixel height/width for bars to ensure visibility.
*/
const MIN_PIXEL_HEIGHT = 3;
const MIN_PIXEL_SIZE = 3;
Copy link
Contributor

@adamwoodnz adamwoodnz Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this value is a little high. 3px looks to me like there is some data. Would just 1px be enough?

Copy link
Contributor Author

@kangzj kangzj Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2026-03-06 at 5 14 41 PM

1px is a bit too little 🤔

Copy link
Contributor

@adamwoodnz adamwoodnz Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2px then 😂

Copy link
Contributor Author

@kangzj kangzj Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya it was 2px actually. 3px is the min height for non zero values.


export const useZeroValueDisplay = (
data: SeriesData[],
Expand All @@ -33,30 +33,28 @@ export const useZeroValueDisplay = (
return useMemo( () => {
if ( ! enabled || ! valueAxisLength || valueAxisLength <= 0 ) return data;

// Find max and min absolute values
// Find max absolute value
let maxAbsoluteValue = 0;
let minAbsoluteValue = Infinity;
for ( const series of data ) {
for ( const point of series.data ) {
if ( point.value !== null && point.value !== 0 ) {
const absValue = Math.abs( point.value );
maxAbsoluteValue = Math.max( maxAbsoluteValue, absValue );
minAbsoluteValue = Math.min( minAbsoluteValue, absValue );
maxAbsoluteValue = Math.max( maxAbsoluteValue, Math.abs( point.value ) );
}
}
}

if ( maxAbsoluteValue === 0 ) return data;

// Calculate the value that renders as MIN_PIXEL_HEIGHT pixels,
// but never exceed the smallest actual value (so zero bars don't appear larger than real data)
const pixelBasedValue = ( MIN_PIXEL_HEIGHT / valueAxisLength ) * maxAbsoluteValue;
const minVisibleValue = Math.min( pixelBasedValue, minAbsoluteValue );
// Calculate the minimum value that renders as MIN_PIXEL_SIZE pixels
const minVisibleValue = ( MIN_PIXEL_SIZE / valueAxisLength ) * maxAbsoluteValue;

return data.map( series => ( {
...series,
data: series.data.map( ( point: DataPointDate ): EnhancedDataPoint => {
if ( point.value === 0 ) {
const absValue = Math.abs( point.value ?? 0 );

// Boost any value that would render smaller than MIN_PIXEL_SIZE
if ( absValue < minVisibleValue ) {
return {
...point,
visualValue: minVisibleValue,
Expand Down