Skip to content
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.

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

AG 11450 - Remove use of Charts autoSize. #7979

Merged
merged 2 commits into from
May 20, 2024

Conversation

alantreadway
Copy link
Member

https://ag-grid.atlassian.net/browse/AG-11450

The Charts autoSize option is being removed for the 10.0.0 release - these updates should maintain backwards compatibility from a Grid perspective 🤞

@@ -648,126 +648,6 @@ exports[`chartModelMigration upgradeChartModel should upgrade 22.1.0-doughnut-ca
}
`;

exports[`chartModelMigration upgradeChartModel should upgrade 22.1.0-doughnut-callout successfully 1`] = `
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: Not sure why, but this was a redundant duplicate (see previous snapshot) - nx test @ag-grid-enterprise/charts -u removed it correctly.

Comment on lines +225 to +226
common.minHeight = 0;
common.minWidth = 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

These are needed to match previous behaviour, and should ensure full backwards compatibility.

There were about 3 examples which showed layout issues without this change due to the popup / container HTML element being smaller than the 'safe' OOB defaults we've added, as a cross-check that this is working.

Comment on lines +299 to +300
parent['minHeight'] = 600;
parent['minWidth'] = 300;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit of an odd case, if anyone was setting autoSize: false before then I'm pretty sure the layout would have been borked, unless they also set width and height - if they were falling into that case, this mutation should yield no difference.

@alantreadway alantreadway merged commit c71bcc3 into latest May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant