feat(charts): Update V9 Charts to use DataVizGradientPalette #33323

shawn-grant · 2024-11-21T19:08:49Z

Implement DataVizGradientPalette and rounded corners in v9 charts as default

Modified

DonutChart
HorizontalBarChart

New Behavior (UI)

DonutChart

HorizontalBarChart

Notable Changes:

Added DataVizGradientPalette in utilities/gradients.ts
Replaced color properties with gradient in types/DataPoint.ts

New Usage

const data: ChartProps = {
  chartData: [
    {
      legend: 'first',
      data: 20000,
      // new `gradient` prop
      gradient: getGradientFromToken(DataVizGradientPalette.gradient1),
    },
    {
      legend: 'second',
      data: 39000,
      // custom gradient
      gradient: [ '#ff88cc', '#db3794' ],
    },
  ],
};

TODO

VerticalBarChart

📊 Bundle size report

✅ No changes found

AtishayMsft · 2024-11-22T02:39:35Z

PR CI is failing.

github-actions · 2024-11-22T02:42:42Z

Pull request demo site: URL

…ntui into chart-gradients

AtishayMsft · 2024-11-22T03:55:43Z

/azp run

azure-pipelines · 2024-11-22T03:55:56Z

Azure Pipelines successfully started running 1 pipeline(s).

AtishayMsft · 2024-11-22T03:58:16Z

packages/charts/react-charts-preview/library/src/components/DonutChart/Arc/Arc.tsx

-          className={classes.root}
-          style={{ fill: props.color, cursor: href ? 'pointer' : 'default' }}
+          d={pathData}
+          style={{ fill: 'transparent', cursor: href ? 'pointer' : 'default' }}


why removed the classname?

Moved the classname to the <div> that renders the gradients. The <path> element won't be visible, so the class and its styles aren't needed.
Test also break if both elements have the class.

AtishayMsft · 2024-11-22T03:58:49Z

packages/charts/react-charts-preview/library/src/components/DonutChart/Arc/Arc.tsx

-          className={classes.root}
-          style={{ fill: props.color, cursor: href ? 'pointer' : 'default' }}
+          d={pathData}
+          style={{ fill: 'transparent', cursor: href ? 'pointer' : 'default' }}
          onFocus={_onFocus.bind(this, props.data!.data, id)}
          data-is-focusable={props.activeArc === props.data!.data.legend || props.activeArc === ''}
          onMouseOver={_hoverOn.bind(this, props.data!.data)}
          onMouseMove={_hoverOn.bind(this, props.data!.data)}
          onMouseLeave={_hoverOff}
          onBlur={_onBlur}


why removed the opacity

Similar to previous comment, the element is to always be transparent, so opacity is unnecessary

AtishayMsft · 2024-11-22T04:01:22Z

packages/charts/react-charts-preview/library/src/components/DonutChart/Pie/Pie.tsx

@@ -25,7 +25,6 @@ export const Pie: React.FunctionComponent<PieProps> = React.forwardRef<HTMLDivEl
    const classes = usePieStyles_unstable(props);
    const pieForFocusRing = d3Pie()
      .sort(null)
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any


why remove this line

was showing as an unused eslint directive

AtishayMsft · 2024-11-22T04:03:26Z

packages/charts/react-charts-preview/library/src/components/DonutChart/DonutChart.tsx

-            }
-            return { ...item, defaultColor };
-          })
+          return { ...item, gradient: item.gradient ?? getNextGradient(index, 0) };


what happens if the gradient colors are undefined

if item.gradient == undefined it will use the default palette

But if item.gradient == ["", ""] the arc wont be visible. I can update it to also check for this condition

AtishayMsft · 2024-11-22T04:04:54Z

packages/charts/react-charts-preview/library/src/components/DonutChart/DonutChartRTL.test.tsx

-    const getById = queryAllByAttribute.bind(null, 'id');
-    expect(getById(container, /Pie.*?second/i)[0]).toHaveAttribute('opacity', '0.1');
+    const getByClass = queryAllByAttribute.bind(null, 'class');
+    expect(getByClass(container, /fui-donut-arc__root/i)[1]).toHaveStyle('opacity: 0.1');


dont use class names. These are internal and can change anytime

Ok, I'll switch to id

AtishayMsft · 2024-11-22T04:09:23Z

packages/charts/react-charts-preview/library/src/utilities/gradients.ts

+const DEFAULT_COLORS = Object.values(defaultGradientPalette);
+const TOKENS = Object.values(DataVizGradientPalette);
+
+const getThemeSpecificGradient = (gradients: [string, string][], isDarkTheme: boolean): [string, string] => {


how can user use a self defined gradient tuple. Could you add an example for it.

Should I add a new story for charts with custom gradients?

AtishayMsft · 2024-11-22T04:12:37Z

...charts/react-charts-preview/library/src/components/HorizontalBarChart/HorizontalBarChart.tsx

@@ -310,13 +333,13 @@ export const HorizontalBarChart: React.FunctionComponent<HorizontalBarChartProps
            x: points.chartData![0].horizontalBarChartdata!.y - datapoint!,
            y: points.chartData![0].horizontalBarChartdata!.y,
          },
-          color: tokens.colorBackgroundOverlay,
+          gradient: getGradientFromToken(DataVizGradientPalette.disabled),


use token here as used earlier.

shawn-grant · 2024-11-22T05:37:08Z

PR CI is failing.

CI error should be fixed with this commit 0cc5413

AtishayMsft · 2024-11-22T05:47:42Z

/azp run

azure-pipelines · 2024-11-22T05:47:54Z

Azure Pipelines successfully started running 1 pipeline(s).

shawn-grant added 16 commits November 20, 2024 21:58

add gradient declarations

e5e79fc

replace color with gradient in ChartDataPoint and add gradient utilit…

b362372

…y tests

refactor: update test-data.ts dataPoints to use gradient property

758f388

feat: implement gradient support in DonutChart components

09b1252

test: update DonutChart tests to use gradient instead of color

eef1cf2

feat: add gradient support to HorizontalBarChart and update rendering…

0c562b5

… logic

refactor: replace color with gradient in HorizontalBarChart test cases

2eb7d67

build: update charts api readme

942d9a3

add update-snapshots script to package.json

d3cc0db

update tests and test snapshot

5d90039

Merge branch 'master' into chart-gradients

f64134a

refactor: update all DonutChart stories to use gradient pallete

d9aa34d

refactor: update HorizontalBarChart stories to use gradient palette

40f628c

Merge branch 'master' into chart-gradients

355faa6

fix HBC test snapshots

18988c5

update change files

61e5ad7

shawn-grant marked this pull request as ready for review November 21, 2024 20:27

shawn-grant requested a review from a team as a code owner November 21, 2024 20:27

AtishayMsft reviewed Nov 22, 2024

View reviewed changes

change/@fluentui-react-charts-preview-d9ff82b5-6274-429e-8f9d-bd400e647d4b.json Outdated Show resolved Hide resolved

shawn-grant added 4 commits November 22, 2024 02:46

update change files

fc6d1c4

Merge branch 'chart-gradients' of https://github.com/shawn-grant/flue…

e10f630

…ntui into chart-gradients

update change type to patch

8a3d7bf

Merge branch 'master' into chart-gradients

751e2e0

AtishayMsft reviewed Nov 22, 2024

View reviewed changes

build: fix formatting issues

c58d7af

AtishayMsft reviewed Nov 22, 2024

View reviewed changes

build: fix ci fail (vr-tests-react-components)

0cc5413

shawn-grant requested review from Mitch-At-Work and a team as code owners November 22, 2024 05:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(charts): Update V9 Charts to use DataVizGradientPalette #33323

feat(charts): Update V9 Charts to use DataVizGradientPalette #33323

shawn-grant commented Nov 21, 2024 •

edited

Loading

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

github-actions bot commented Nov 22, 2024 •

edited

Loading

AtishayMsft commented Nov 22, 2024

github-actions bot commented Nov 22, 2024

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024 •

edited

Loading

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant Nov 22, 2024

AtishayMsft Nov 22, 2024

shawn-grant commented Nov 22, 2024

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

feat(charts): Update V9 Charts to use DataVizGradientPalette #33323

Are you sure you want to change the base?

feat(charts): Update V9 Charts to use DataVizGradientPalette #33323

Conversation

shawn-grant commented Nov 21, 2024 • edited Loading

Modified

New Behavior (UI)

DonutChart

HorizontalBarChart

Notable Changes:

New Usage

TODO

Related

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

github-actions bot commented Nov 22, 2024 • edited Loading

📊 Bundle size report

AtishayMsft commented Nov 22, 2024

github-actions bot commented Nov 22, 2024

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

Choose a reason for hiding this comment

shawn-grant Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shawn-grant commented Nov 22, 2024

AtishayMsft commented Nov 22, 2024

azure-pipelines bot commented Nov 22, 2024

shawn-grant commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 22, 2024 •

edited

Loading

shawn-grant Nov 22, 2024 •

edited

Loading