Skip to content

Conversation

@AtishayMsft
Copy link
Contributor

Previous Behavior

New Behavior

Related Issue(s)

  • Fixes #

@github-actions
Copy link

github-actions bot commented Dec 6, 2025

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-charts
DeclarativeChart
756.617 kB
217.631 kB
756.621 kB
217.634 kB
4 B
3 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-charts
AreaChart
406.783 kB
124.495 kB
react-charts
DonutChart
317.224 kB
94.361 kB
react-charts
FunnelChart
308.774 kB
91.361 kB
react-charts
GanttChart
389.926 kB
118.05 kB
react-charts
GaugeChart
316.655 kB
93.799 kB
react-charts
GroupedVerticalBarChart
397.772 kB
120.667 kB
react-charts
HeatMapChart
391.973 kB
119.859 kB
react-charts
HorizontalBarChart
296.951 kB
86.138 kB
react-charts
HorizontalBarChartWithAxis
63 B
83 B
react-charts
Legends
235.905 kB
69.363 kB
react-charts
LineChart
417.347 kB
126.389 kB
react-charts
PolarChart
345.84 kB
105.418 kB
react-charts
SankeyChart
213.881 kB
65.62 kB
react-charts
ScatterChart
397.174 kB
120.59 kB
react-charts
Sparkline
91.393 kB
28.708 kB
react-charts
VerticalBarChart
434.249 kB
126.27 kB
react-charts
VerticalStackedBarChart
403.686 kB
121.535 kB
🤖 This report was generated against f17bb7ecaae51c2c01401a7d37e09a7fde13a373

@github-actions
Copy link

github-actions bot commented Dec 6, 2025

Pull request demo site: URL

@@ -0,0 +1,247 @@
# Vega-Lite Schema Integration Summary
Copy link

@github-actions github-actions bot Dec 6, 2025

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual changes to review in the Visual Change Report

vr-tests-react-components/CalendarCompat 4 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/CalendarCompat.multiDayView.default.chromium_1.png 676 Changed
vr-tests-react-components/CalendarCompat.multiDayView - Dark Mode.default.chromium.png 2200 Changed
vr-tests-react-components/CalendarCompat.multiDayView - High Contrast.default.chromium.png 2279 Changed
vr-tests-react-components/CalendarCompat.multiDayView.default.chromium.png 675 Changed
vr-tests-react-components/Charts-DonutChart 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Charts-DonutChart.Dynamic.default.chromium.png 5581 Changed
vr-tests-react-components/Positioning 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Positioning.Positioning end.chromium.png 740 Changed
vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png 743 Changed
vr-tests-react-components/TagPicker 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/TagPicker.disabled - Dark Mode.disabled input hover.chromium.png 658 Changed
vr-tests-react-components/TagPicker.disabled - RTL.disabled input hover.chromium.png 635 Changed

There were 3 duplicate changes discarded. Check the build logs for more information.

@AtishayMsft AtishayMsft marked this pull request as ready for review January 30, 2026 08:30
@AtishayMsft AtishayMsft requested review from a team as code owners January 30, 2026 08:30
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these change summary reports needed?

/**
* Helper to compare two arrays for equality
*/
function arraysEqual(a: string[], b: string[]): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function can be imported from the utilities:

export function areArraysEqual(arr1?: string[], arr2?: string[]): boolean {
if (arr1 === arr2 || (!arr1 && !arr2)) {
return true;
}
if (!arr1 || !arr2 || arr1.length !== arr2.length) {
return false;
}
for (let i = 0; i < arr1.length; i++) {
if (arr1[i] !== arr2[i]) {
return false;
}
}
return true;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to keep these adapters inside the VegaDeclarativeChart folder?

// Inline schemas (25 total covering various chart types)
// These are the default schemas shown in "show few" mode
const ALL_SCHEMAS: Record<string, any> = {
adCtrScatter: {
Copy link
Contributor

Choose a reason for hiding this comment

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

These schemas can be stringified to reduce LOC.

// These are the default schemas shown in "show few" mode
const ALL_SCHEMAS: Record<string, any> = {
adCtrScatter: {
$schema: 'https://vega.github.io/schema/vega-lite/v5.json',
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be helpful to include the recommended Vega schema version in the JSDoc comments and the example docs?

ALL_OPTIONS.sort((a, b) => {
if (a.category !== b.category) {
// Priority order for categories
const categoryOrder = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be extracted for reuse

name.includes('expense') ||
name.includes('roi') ||
name.includes('financial') ||
name.includes('dividend')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be simplified to:

['stock', 'portfolio', 'profit', 'revenue', 'cashflow', 'budget', 'expense', 'roi', 'financial', 'dividend'].some(keyword => name.includes(keyword))

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to combine these 3 test files into 1 since they all contain component tests?

/**
* Hook to determine if dark theme is active
*/
function useIsDarkTheme(): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

useIsDarkTheme and useColorMapping hooks can be extracted for reuse.

| 'donut'
| 'heatmap'
| 'histogram'
| 'polar';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can’t we use the same mapping as DeclarativeChart?

const chartProps = transformer(spec, colorMap, isDarkTheme) as any;

// Special handling for charts with different prop patterns
if (chartType.type === 'donut') {
Copy link
Contributor

Choose a reason for hiding this comment

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

These conditions may not be needed since legendProps and componentRef are available on all charts.

* @param encoding - Vega-Lite encoding with polar axis settings
* @returns Object with projected x, y arrays and metadata
*/
function projectPolarToCartesian(
Copy link
Contributor

Choose a reason for hiding this comment

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

projectPolarToCartesian, transformVegaLiteToPolarLineChartProps and transformVegaLiteToPolarScatterChartProps functions can be removed.

@@ -0,0 +1,27 @@
{
"$schema": "https://vega.github.io/schema/vega-lite/v5.json",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these json files being used. Remove if not.

"description": "Multi-series line chart with category10 color scheme",
"mark": "line",
"data": {
"values": [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are these json files being used. Remove if not

/**
* Selected legends (used for multi-select legend interaction)
*/
selectedLegends?: string[];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

revert this change. Looks to be a merge conflict

@AtishayMsft AtishayMsft marked this pull request as draft February 9, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants