Fix issue where trend indicator would render outside chart bounds

Shopify · Jan 20, 2025 · a0597dd · a0597dd
1 parent 36fadba
commit a0597dd
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 7 deletions.
diff --git a/packages/polaris-viz/CHANGELOG.md b/packages/polaris-viz/CHANGELOG.md
@@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
-<!-- ## Unreleased -->
+## Unreleased
+
+### Fixed
+
+- Fixed issue in `<SimpleBarChart />` where the trend indicator was not being positioned correctly when the value was `null`.
 
 ## [15.8.0] - 2025-01-16
 

diff --git a/...ges/polaris-viz/src/components/SimpleBarChart/stories/playground/NoTrendTight.stories.tsx b/...ges/polaris-viz/src/components/SimpleBarChart/stories/playground/NoTrendTight.stories.tsx
@@ -0,0 +1,65 @@
+import type {Story} from '@storybook/react';
+
+import {SimpleBarChart, SimpleBarChartProps} from '../../../../components';
+import {META} from '../meta';
+import type {SimpleBarChartDataSeries} from '../../types';
+import {
+  DEFAULT_THEME_NAME,
+  PolarisVizProvider,
+} from '@shopify/polaris-viz-core';
+
+export default {
+  ...META,
+  title: `${META.title}/Playground`,
+};
+
+const DATA: SimpleBarChartDataSeries[] = [
+  {
+    name: '',
+    data: [
+      {key: 'One', value: 6000},
+      {key: 'Two', value: 5300},
+    ],
+    metadata: {
+      trends: {
+        '0': {},
+      },
+    },
+  },
+  {
+    name: '',
+    data: [
+      {key: 'One', value: 4500},
+      {key: 'Two', value: 1500},
+    ],
+  },
+];
+
+const Template: Story<SimpleBarChartProps> = () => {
+  const svgStyle = `
+    svg {
+      background: rgba(0, 255, 0, 0.1);
+    }
+  `;
+
+  return (
+    <div style={{height: 600, width: 800}}>
+      <style>{svgStyle}</style>
+      <PolarisVizProvider
+        animated={{} as any}
+        themes={{
+          [DEFAULT_THEME_NAME]: {
+            chartContainer: {
+              backgroundColor: 'rgba(255, 0, 0, 0.1)',
+              padding: '20px',
+            },
+          },
+        }}
+      >
+        <SimpleBarChart data={DATA} />
+      </PolarisVizProvider>
+    </div>
+  );
+};
+
+export const NoTrendTight = Template.bind({});
diff --git a/packages/polaris-viz/src/components/SimpleBarChart/utilities.ts b/packages/polaris-viz/src/components/SimpleBarChart/utilities.ts
@@ -1,6 +1,7 @@
 import {HORIZONTAL_BAR_LABEL_OFFSET} from '@shopify/polaris-viz-core';
 
 import {
+  TREND_INDICATOR_NO_VALUE_WIDTH,
   estimateTrendIndicatorWidth,
   TREND_INDICATOR_FONT_WEIGHT,
 } from '../TrendIndicator';
@@ -24,15 +25,18 @@ export function getLongestTrendIndicator(
       for (const [index, trend] of trendEntries) {
         const dataPoint = seriesData[index];
 
-        if (trend == null || trend.value == null || dataPoint?.value == null) {
+        if (trend == null || dataPoint?.value == null) {
           return longestTrendIndicator;
         }
 
-        const trendStringWidth = estimateTrendIndicatorWidth(
-          trend.value,
-          fontSize,
-          TREND_INDICATOR_FONT_WEIGHT,
-        ).totalWidth;
+        const trendStringWidth =
+          trend.value == null
+            ? TREND_INDICATOR_NO_VALUE_WIDTH
+            : estimateTrendIndicatorWidth(
+                trend.value,
+                fontSize,
+                TREND_INDICATOR_FONT_WEIGHT,
+              ).totalWidth;
 
         // Positive value
         if (dataPoint.value > 0) {

diff --git a/packages/polaris-viz/src/components/TrendIndicator/index.ts b/packages/polaris-viz/src/components/TrendIndicator/index.ts
@@ -3,3 +3,4 @@ export type {TrendIndicatorProps} from './TrendIndicator';
 export {estimateTrendIndicatorWidth} from './utilities/estimateTrendIndicatorWidth';
 export {HEIGHT as TREND_INDICATOR_HEIGHT} from './constants';
 export {FONT_WEIGHT as TREND_INDICATOR_FONT_WEIGHT} from './constants';
+export {NO_VALUE_WIDTH as TREND_INDICATOR_NO_VALUE_WIDTH} from './constants';