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

creating customSetProps to handle setProps #331

Merged
merged 4 commits into from
Nov 1, 2024
Merged

Conversation

BSd3v
Copy link
Collaborator

@BSd3v BSd3v commented Oct 7, 2024

creating customSetProps to handle setProps when component no longer in dash tree

@BSd3v BSd3v requested a review from ndrezn October 7, 2024 16:37
@ndrezn ndrezn requested a review from T4rk1n October 7, 2024 19:51
@ndrezn
Copy link
Member

ndrezn commented Oct 9, 2024

Is there a specific issue that this change resolves?

@BSd3v
Copy link
Collaborator Author

BSd3v commented Oct 9, 2024

There are a couple of cases that lead to this:

  • when a component is removed entirely, as in a page container removes the element or removed as a child from a dom (eg test_cs002_column_state)
    • this leads to setProps from internal component updates not merging with the grid component in dash and results in a dash renderer error for passing an object to an expected component
  • when a component is unmounted and the underlying grid can no longer perform actions, this pertains to updates when no longer in the DOM tree (eg tabs that dont keep the component in the DOM)
    • this leads to an error inside the component where the state (mounted) and props are out of sync and it tries to apply changes to the grid even though it is currently unmounted. Often seen as JS errors.

@gvwilson gvwilson added feature something new community community contribution P2 considered for next cycle labels Oct 15, 2024
@gvwilson
Copy link
Contributor

@T4rk1n can you please have a look at this? Once it is reviewed and merged, @BSd3v can do a new dash-ag-grid release.

Copy link

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

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

LGTM 💃

@gvwilson
Copy link
Contributor

gvwilson commented Nov 1, 2024

Thanks @T4rk1n - @BSd3v please go ahead and push the button.

@BSd3v BSd3v merged commit f27dd83 into plotly:main Nov 1, 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 feature something new P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JS error when enabling both ‘floatingFilter’ and ‘agGroupColumnFilter’
4 participants