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

fix for JSON Parse error when using dash_ag_grid and dash pages #300

Merged
merged 10 commits into from
Oct 2, 2024

Conversation

BSd3v
Copy link
Collaborator

@BSd3v BSd3v commented May 1, 2024

Fixes #299.

  • testing for grid destroyed before trying to get the state

@BSd3v BSd3v linked an issue May 1, 2024 that may be closed by this pull request
@gvwilson gvwilson assigned archmoj and BSd3v and unassigned archmoj May 27, 2024
@gvwilson gvwilson requested a review from archmoj May 27, 2024 12:26
@gvwilson
Copy link
Contributor

@archmoj could you please have a look at this (or let us know that you don't feel you have enough context to review)?

@archmoj
Copy link

archmoj commented May 27, 2024

@BSd3v Thanks very much for the PR. It's looking good.
Could you add a test to lock down this bug from happening again?

@gvwilson I am not familiar with this repository.
By looking at the list of contributors @AnnMarieW, @ndrezn and @Farkites may help approving this PR.

BTW. I was wondering perhaps this check could/should be moved to be inside getColumnState as an early return to avoid duplicates and simplify this PR as well.

@BSd3v
Copy link
Collaborator Author

BSd3v commented May 27, 2024

Hello @archmoj,

getColumnState is not something that this repository owns as it comes from the underlying AG Grid. While yes they could do a return, they provide the warning that you are calling a function against a destroyed grid.

@gvwilson gvwilson changed the title fixing #299 fix for JSON Parse error when using dash_ag_grid and dash pages May 30, 2024
@BSd3v
Copy link
Collaborator Author

BSd3v commented May 31, 2024

Hello @gvwilson this issue can crop up at any point the grid is destroyed, this doesn't just apply to when using pages. Although it's a lot easier to test when using pages. 😁

@gvwilson gvwilson self-assigned this Jun 26, 2024
@gvwilson gvwilson added community community contribution fix fixes something broken P2 considered for next cycle labels Aug 13, 2024
Copy link
Collaborator

@AnnMarieW AnnMarieW left a comment

Choose a reason for hiding this comment

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

💃

Looks good!

@BSd3v BSd3v requested a review from ndrezn September 30, 2024 14:48
@gvwilson gvwilson merged commit 7641e00 into plotly:main Oct 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution fix fixes something broken P2 considered for next cycle
Projects
None yet
4 participants