- Notifications
You must be signed in to change notification settings - Fork 38
Feature: AG Grid 33 #381
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
Feature: AG Grid 33 #381
Conversation
| Hi @KoolADE85 I think there was a discussion about AG Grid supporting V32 for the long term because there are so many breaking changes going from v32 to v33. However, going from v33 to v34 does not have any breaking changes, so it might actually be better to do v34 and skip v33. https://www.ag-grid.com/react-data-grid/upgrading-to-ag-grid-34/ This won't have a big impact on our docs because most of the new features are for Enterprise and they aren't in the dash-docs. |
Good call! I have some test failures around filters when I use v34. I'm going to investigate whether it's the test or the component itself that needs updating. |
| Before we do this, I need we need to branch v32 from main. |
| Also, we should look at this, if this PR is good, should probably do theirs. :) |
0d79034 to 8c67d20 Compare | Should make sure there is a way to set the theme to legacy though. So that people can opt for not having to create the theme. |
OK, this gets a little tricky since AG Grid doesn't support importing legacy theme CSS files (as we were previously) AND setting a modern theme object. So in my update, I propose that anyone who wants to keep using a legacy CSS theme, to import the CSS by hand (we won't automatically import, for example, Our codebase going forward, would support AG Grid's modern theme system by default. See This approach seems right because, in case of any errors/warnings, the message that AG Grid outputs in the console matches what users would need to do to their dash app in order to get the grid looking right. |
We can still support these files and not import them directly into the grid the same way. There are a couple of repos that do this in order to make it easier to import these files that are otherwise dependencies to make the package load properly. |
| Alright, I'm giving this a try: lazy load the css for legacy themes if the user has specified a |
60d5741 to 744b9e5 Compare 744b9e5 to 739b1b7 Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 💃
This PR upgrades AG Grid to version 33.