girder / girder_web_components

Reusable Javascript and VueJS components for interacting with a Girder server.
https://gwc.girder.org
Apache License 2.0
16 stars 9 forks source link

DataBrowser select behavior is strange #179

Closed subdavis closed 5 years ago

subdavis commented 5 years ago

Currently the data browser allows selections to persist when you change working directories. This behavior seems incorrect, but it has the benefit of being more flexible than the alternative.

For example, if DataBrowser were to reset the selection when the working directory changed, you'd need another prop to disable that behavior to get back to where we are now. Currently, downstream can listen to location change events and reset the selected prop itself without any additional props.

Should we keep the current, wonky behavior for the sake of flexibility?

matthewma7 commented 5 years ago

Oh, yes. I personally had some trouble with the persisted selection exceptional when I need to do something with the selection in the custom app. I guess the best option is to make the persistent selection optional? and deselect by default to gain the flexibility of persistent selection and also don't contradict common expectations or add burden to the downstream consumers of the component.

zachmullen commented 5 years ago

Given @matthewma7 's response, I'd say for now let's take no action, and wait until we hear from users if there is a problem.

subdavis commented 5 years ago

Good enough for me :+1:

subdavis commented 5 years ago

I was wrong, consumers have no control over the value of selected. #181 addresses this.

matthewma7 commented 5 years ago

I don't know if I made myself clear. I had some difficulty with the current persisted selection behavior. The issue may be alleviated if there is some indicator to show that there are records selected.

subdavis commented 5 years ago

some indicator to show that there are records selected.

I think there was probably an issue with the data coming from $emit.update-selected being wrong. Should be resolved by 181. Is the ability to listen to selection change events sufficient for your needs?

matthewma7 commented 5 years ago

some indicator to show that there are records selected.

I think there was probably an issue with the data coming from $emit.update-selected being wrong. Should be resolved by 181. Is the ability to listen to selection change events sufficient for your needs?

We might be talking about different issues. I mean a user could select something in a directory then change to another directory. At that moment, the selection state is there but no visual indication of that.

subdavis commented 5 years ago

there but no visual indication of that.

You'll see the indeterminate state of the checkbox in the header row, right?

matthewma7 commented 5 years ago

You'll see the indeterminate state of the checkbox in the header row, right?

Hm, maybe, that's a good point. but I kind of expect auto deselect or something more explicit about the selection as a new user, but once you know how it works, it could be ok.

jeffbaumes commented 5 years ago

You'll see the indeterminate state of the checkbox in the header row, right?

Once you select one more thing in your current view, the meaning of the indeterminate checkbox state is indeterminate :)

I'm not sure the original use case for the "navigate and select" workflow in the old Girder UI, but it is certainly nonstandard UI. The normal way to do cross-hierarchy selection is to use a tree view.

zachmullen commented 5 years ago

Sounds like many of us were surprised by this behavior. Would it be difficult to change it so that selection resets on location change?