react-arborist icon indicating copy to clipboard operation
react-arborist copied to clipboard

Lack of control over delete leads to some weird scenarios

Open kid-icarus opened this issue 1 year ago • 2 comments
trafficstars

Consider the following scenario:

I have some entities in a store, and deleting those entities from the tree should also delete them from the store. However, I might need to validate that they can be deleted, and if not, display some error notification stating such. If the node can't be deleted, I'd like to not focus on the next sibling.

https://github.com/brimdata/react-arborist/assets/864752/23fa7e67-aaef-4929-8710-cc8feac19cc9

Next, lets say I hit delete on the last node. It wraps around to the root node, that has no siblings, but only children. If I were to hit delete again, all selection is lost, and the focus remains on the root node because the logic cant find the next node to focus on

https://github.com/brimdata/react-arborist/assets/864752/67a4d1f2-5576-4234-aed2-1eb2af781c3f

Lastly, it's not immediately obvious to me how I can manage focused state in this scenario: I attempt to delete a "forbidden" node, display a notification, and when the notification disappears a rerender happens. When this happens, the focus state resets to next sibling of the node that I was attempting to delete.

I have a couple thoughts here, wanted to see what if this makes sense to you.

  1. I could create my own tree container, and pass it to the renderContainer. However, the internal components used by the default container aren't exported. This means I'd have to recreate those.
  2. Perhaps we could add a prop, something to the effect of onDeleteOverride, whereby the full behavior of delete could be controlled. We could check for this prop in the default container, and if present, do not run the default backspace event handler. Rather, we'd just invoke whatever was passed in the prop.

Curious if you have any thoughts on a these suggestions or other directions this could go :)

kid-icarus avatar Mar 05 '24 19:03 kid-icarus