xcfox / react-tile-pane

A React tiling pane manager
MIT License
28 stars 3 forks source link

move() destroys component from DOM #5

Open ugokoli opened 1 month ago

ugokoli commented 1 month ago

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?

xcfox commented 1 month ago

This feature seems to be very frequent and I'd be happy to add it. This may involve more changes and I'll take the time to work on it. this may take weeks, if I'm too slow, a PR is welcome

xcfox commented 1 month ago

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 })
ugokoli commented 1 month ago

Thanks for considering this.

I believe a specific behavior will go a long way to cater to many developer use cases and needs. A developer may want to have both remove and hide beviour within one scope. Setting the property in the scope will restrict this use case specifically.

xcfox commented 1 month ago

For a specific pane, does it sometimes need to be removed and sometimes hidden? What do you think of the following way:

const App: Rect.FC = () => {
  return (
    <TileProvider tilePanes={paneList} rootNode={rootPane} hidingPanes={[names.apple, names.banana]}>
       ...
    </TileProvider>
  )
}

This allows us to use methods other than useMovePane and still make sure the pane is hidden.

ugokoli commented 1 month ago

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. Unless the goal is not to provide too much flexibility, then defining this parameter in the scope is okay. Otherwise, useMovePane will come in handy for everyone.