m2ms / fragalysis-frontend

The React, Redux frontend built by webpack
Other
1 stars 1 forks source link

Rework behaviour of RHS selection boxes #894

Open phraenquex opened 2 years ago

phraenquex commented 2 years ago

@rsanchezgarc to fully spec out.

rsanchezgarc commented 2 years ago

Currently, the default behaviour for the selection checkboxes on the RHS is to add them automatically to the selected compounds tab. We want to provide several functions to the checkboxes that would be triggered by different buttons. At the moment, we need the next two functionalities, but they could be extended in the future. 1) Add /remove them to the selected compounds tab. (Like default behaviour now, but triggered by button) 2) Ping the compounds so that they are not affected by the fast navigation mode using the little arrows. 3) Ping the compounds so that they are affected by the fast navigation mode using the little arrows.

Every compound affected by the fast navigation, when clicked the little arrow, will be replaced by the next one in the list. The little arrows should have a keyboard hotkey, and/or being displayed in a fixed position, so that you can click quickly

boriskovar-m2ms commented 1 year ago

@phraenquex @rsanchezgarc When iterating through the list of dataset compounds what should I do when the next compound is pinned/locked compound?

Options I can think of:

  1. Settings with which I'm iterating will be applied to pinned compound - this doesn't play well with semantics of locked/pinned molecule in a sense that I expect for them to be unaffected
  2. The pinned compound stays unaffected - this means I need to click two times to go to the next compound (the one after the current pinned one)
  3. The pinned compound is skipped altogether - so the pinned compound stays unaffected and we will skip directly to the compound which goes right after the pinned one.

To me the option no. 3 seems like the most sensible and least confusing behavior.

phraenquex commented 1 year ago

Yes, I think it's #3. (Possibly '2, but definitely not #1.) @rsanchezgarc ?

phraenquex commented 1 year ago

Fantastic.. Two corner cases:

boriskovar-m2ms commented 1 year ago

@phraenquex @rsanchezgarc Suggestion for second issue:

boriskovar-m2ms commented 1 year ago

@phraenquex @rsanchezgarc Also there is a problem with first requirement (selected but not displayed compound should be included in the iteration). Problem is that the new semantic of the checkbox is a lock now but the suggested change says that following one is locked:

image

And this one is unlocked: image

But if I arrive at this compound from previous molecule it should be still unlocked but it looks exactly the same as exhibit one: image

Now to distinguish between the case 1 and 3 I need to track in a complicated way (because of multiple datasets, multiple ways how to iterate i.e. global and local arrows, lock/unlock it by manually making something visible, drag&drop etc.) a new metadata. This will take some time to get it right (a lot of edge cases). Of course it is possible to implement but it's not going to be such a quick win as was envisioned so I'm asking whether should I go on and implement this. Currently we are not keeping any 'iteration' metadata and whole algorithm works just by looking and the current state and figuring out what it's the next step and with this approach it's now impossible to implement this requirement.

phraenquex commented 1 year ago

@boriskovar-m2ms I think I understand your first question and makes me worry that what we've ended up with is unintuitive. Specifically, if you have to unselect something to activate it for the global arrow, then that surely indicates the logic is upside down.

@rsanchezgarc, on Boris's point that the selection box now has only a single, very narrow function: is this what we wanted?

@boriskovar-m2ms this ticket is no longer running on a test stack (I think) - can you deploy it on a different stack than tibor's?

phraenquex commented 1 year ago

@boriskovar-m2ms I don't understand what problem you're describing in your 2nd question. For scenario 3, you'd just keep going - does the global up/down button not remember what the current compound is? (i.e. is there not the concept of a cursor?)

boriskovar-m2ms commented 1 year ago

@phraenquex OK I will implement the dialog which will appear right next to the "local" or "global" arrow when used and there is a more then one unlocked compound visible in the NGL view.

boriskovar-m2ms commented 1 year ago

@phraenquex yes currently we are just moving settings from first compound which is unlocked and has something visible in the NGL view (other unlocked compounds will be just unselected). So currently the scenario 3 is exactly the same as scenario 1 so the algorithm can't decide what to do. There is no concept of cursor because until now there was no need and cursors don't like operating on dynamic data (user can select/unselect stuff between iterations, move it around with drag&drop, filter some compounds out and do other stuff) and that's why also programming languages (here the cursor is usually called and iterator though) usually throw an exception if the collection is modified while iterating it. I can implement it but just wanted you to know that it's not as simple as it seems at the first glance (leitmotif of fragalysis for last couple of years).

rsanchezgarc commented 1 year ago

@rsanchezgarc, on Boris's point that the selection box now has only a single, very narrow function: is this what we wanted?

When we decided to add the cart icon to save compounds, the checkbox does not need to do anything but select the molecules to keep. I wonder if what we really want is to keep always visible a particular group (something that was already added to one colour group). If so, we could remove completely the checkboxes. However, my intuition is that checkboxes mix quite well with action buttons, like a button that says fix in GUI, or add To group or whatever. I think this needs some discussion, probably 121.

boriskovar-m2ms commented 1 year ago

Just one side note: If we consider locked compounds only when they're selected and something is visible then we lose ability to skip some compounds which I'm not interested seeing anymore (no idea if this is valid use case) because now just selected compound where nothing is visible in NGL is skipped while iterating through the list of compounds.

phraenquex commented 1 year ago

Frank's test: click down/up button very fast, triggers the dialog log in , then selections are greyed out.

phraenquex commented 1 year ago

@boriskovar-m2ms explained, and @phraenquex says: do NOT implement the "first" corner case. That means, curent behaviour is correct: if a compound is selected but not displayed, skip over it anyway. If the user doesn't like it, they can simply unselect and move up again.

phraenquex commented 1 year ago

@boriskovar-m2ms: if I remove compounds from the selected list (by clicking on the coloured box), and then press "undo", the compound does not reappear on the list. The tooltip on the Undo button does say that it will undo the un-selection, but clicking the button does not do what the tooltip says it will.

phraenquex commented 1 year ago

@rsanchezgarc it's probably a bit late to ask, but is this cursor behaviour really intuitive? (@matteoferla @Waztom please try it out and comment too.) Currently: I click the up/down arrow, something changes... but can I easily tell which one is selected? (E.g. if I had to look away for 5 minutes). Not really: I have to find the one compound that is displayed but not selected ("locked") - that's a lot of looking-around.

I see two options:

  1. Make it be the "active" compound that is selected; and the up/down skips over displayed-but-not-selected compounds
  2. Add some other visual queue on the active compound (e.g. all characters bold; magnify the 2D image; ....)

I'm guessing #2 is by far the easiest for @boriskovar-m2ms. But #1 would give the RHS a cursor, and that could become very useful in future.

@boriskovar-m2ms : could you also add a shopping cart next to the up/down arrows, which adds the active compound to the selected colour. Then I don't have to move the mouse that far.

phraenquex commented 1 year ago

@boriskovar-m2ms this is quite ugly - if it's easy to change layout, I suggest something like this.

image

image

boriskovar-m2ms commented 1 year ago

@boriskovar-m2ms: if I remove compounds from the selected list (by clicking on the coloured box), and then press "undo", the compound does not reappear on the list. The tooltip on the Undo button does say that it will undo the un-selection, but clicking the button does not do what the tooltip says it will.

Fixed together with whole bunch of other bugfixes. And now it's deployed on tibor's stack.

phraenquex commented 1 year ago

After group discussion: don't implement "cursor" (not now), but colour the box with arrows some dark(ish) colour.

New ticket: