-
Notifications
You must be signed in to change notification settings - Fork 3
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
move() destroys component from DOM #5
Comments
This feature seems to be very frequent and I'd be happy to add it. |
Do you think we should provide a scoped property in the Provider or specify specific behavior when hiding the pane? // Provide a scope property
const App: Rect.FC = () => {
return (
<TileProvider tilePanes={paneList} rootNode={rootPane} hidingPane={true}>
...
</TileProvider>
)
} // specific behavior
move(name, null, { hidingPane: true }) |
Thanks for considering this. I believe a specific behavior will go a long way to cater to many developer use cases and needs. |
For a specific pane, does it sometimes need to be removed and sometimes hidden? const App: Rect.FC = () => {
return (
<TileProvider tilePanes={paneList} rootNode={rootPane} hidingPanes={[names.apple, names.banana]}>
...
</TileProvider>
)
} This allows us to use methods other than |
This approach will work for most cases as well, but I can imaging some developer wanting to offer their users the options to hide or delete a pane in their UI. |
This is a very important use case for a declarative framework like React.
We want to prevent multiple rerenders as much as possible and manage states easily.
Calling the
useMovePane()
hook to either remove or add a pane is okay, but how about hiding a pane without removing it from the DOM?move(name, isShowing ? null : [0.99, 0.5])
A pane that is removed and added in a toggle manner gets rerendered all the time and the previous state is lost. Ref to previous elements are also missing and it creates a buggy experience.
Is there a way currently to achieve hiding a pane in addition to adding and removing?
Or is it something we can add in the next update?
The text was updated successfully, but these errors were encountered: