grapoza / vue-tree

Tree components for Vue 3
MIT License
95 stars 12 forks source link

treeNodeSelectedChange API improvement? #298

Open luckyrat opened 1 year ago

luckyrat commented 1 year ago

Is your feature request related to a problem? Please describe. I'd like to track when the selected node is changed. Although the current API helps somewhat, it doesn't allow me to differentiate between a new node being selected and all nodes being unselected by the user.

It appears that sometimes/always the "unselected" node is sent to the treeNodeSelectedChange callback before the newly selected node.

Describe the solution you'd like Either reliably send all deselection callbacks before selection ones, or change the API so that it has two optional parameters (oldNode, newNode). Ideally that would be sent just once for a selection change in single mode.

For single-selection mode, this change would mean that a single variable in my parent component could track the selection status, whereas with the current API, I would have to duplicate the entire tree and track the selection status of each node, allowing for the brief period in which more than one node can be selected.

grapoza commented 1 year ago

I'll have to take a look at this and see if there's a good way to wrap these up; right now it's every node for itself, so some reswizzling would have to occur (possibly with tree-level selection tracking, etc). It's a good idea, though. Maybe I could even introduce new events specific to something being selected or deselected (instead of just the state change), and have the selected event behave the way that's describe here? It may be too many events for selection, though. It's probably something best thought about after sleeping on it.

It probably doesn't directly help your scenario, but I wonder what the behavior of the getSelected method is if called from a treeNodeSelectedChange handler? I'd worry about race conditions or that kind of weird issue.

Just out of curiosity, would you be able to use an array of selected nodes to track them instead of an entire shadow tree? I'm always interesting in hearing about people's use cases.

luckyrat commented 1 year ago

It probably doesn't directly help your scenario, but I wonder what the behavior of the getSelected method is if called from a treeNodeSelectedChange handler? I'd worry about race conditions or that kind of weird issue.

I haven't tried yet but would also be a bit wary about that due to my lack of deep knowledge about the Vue reactivity lifecycle - perhaps it's clearly defined enough that race conditions couldn't occur but I'm not currently a stage of development where I can experiment with something I'm this unfamiliar with.

Just out of curiosity, would you be able to use an array of selected nodes to track them instead of an entire shadow tree? I'm always interesting in hearing about people's use cases.

Probably. I figured that if I would need a shadow tree for everything in order to get filtering to work (#296) I might as well use that same structure. The current impact to users is that they are able to have zero items selected and still pass form validation. All that happens is the most-recently selected node is submitted which in my use-case is not a critical bug, but may well lead to some occasional confusion so I would prefer to be able to disable the submit button when I know that no node is selected. I appreciate that calling getSelected at submission time could provide an additional check but then I'll need to set up a full form validation feedback process which will be a huge jump in complexity from just optionally disabling the submit button.

I'll be happy to send you a direct link to the code that uses your component once I've got it merged into my main branch. I'll be doing that ASAP so there will be a lot of messy comments relating to my exploration of the challenges related to this issue and #296 but I doubt it'll be so terrible that you can't get the gist of what I'm trying to achieve.

luckyrat commented 1 year ago

Here's where I'm using the vue-tree component: https://github.com/kee-org/browser-addon/blob/master/src/popup/components/SaveWhere.vue

grapoza commented 1 year ago

Awesome, thanks! I'll take a look tonight.

On Mon, Feb 6, 2023, 11:58 AM Chris Tomlinson @.***> wrote:

Here's where I'm using the vue-tree component: https://github.com/kee-org/browser-addon/blob/master/src/popup/components/SaveWhere.vue

— Reply to this email directly, view it on GitHub https://github.com/grapoza/vue-tree/issues/298#issuecomment-1419410120, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXOZJJGSC2H4Q3JKWQYFGTWWEUR3ANCNFSM6AAAAAAUQN4POY . You are receiving this because you commented.Message ID: @.***>