geneontology / noctua-annotation-review

0 stars 0 forks source link

Comments following deployment on dev #36

Open lpalbou opened 3 years ago

lpalbou commented 3 years ago

First, overall looks great and glad to see that finally on dev.

Things to fix / discuss

  1. When there are no activities, like in this model, show a message stating so, otherwise it could feel like there was some errors while loading the model

Screen Shot 2021-04-05 at 9 22 02 AM

  1. the height of that grey bar "ACTIVITIES" is really too big, I would decrease to 30px; still feel a bit clunky but not critical

  2. I am surprised as I thought the "ACTIVITIES" bar was to list activities... and yet I find a "BP Annotation" and I imagine CC Annotation can appear there too. Seems like either that grey bar is useless OR you want to have multiple grey bars, one for activity, one for bp annotation and one for cc annotation (to show only when you have such annotations). And if it's the logic, then you don't need to show these badges "Activity Unit", "BP Annotation", "CC Annotation" as they would already be grouped under a same banner. Screen Shot 2021-04-05 at 9 24 04 AM

  3. I was in review, clear my basket and.. Then decided on the search icon (top left) and it showed me that: Screen Shot 2021-04-05 at 9 37 22 AM

I believe clicking on the search icon should switch you back to the search page, otherwise it's confusing.

The culprit is still that bar : Screen Shot 2021-04-05 at 9 38 57 AM

I believe we would have to redesign that later, as both this bar and the left bar control what you should see at the center.

  1. in the same aspect, I don't think it's intuitive/clear that after selecting models, you should click on the review button on the top: Screen Shot 2021-04-05 at 9 40 26 AM

Not for this V1, but I would suggest that whenever models are selected, a floating colorful button on the bottom right appears and state something like: "X models selected - Click here to review them". The fact the button appears upon model selection would help people know where to click for the next step right away

  1. I mentioned above I cleared my basket ("Clear List") in which I modified lipid transport into ion transport. But even though the modal state that any changes would not be saved... When I reselect those models, I still see my modifications: Screen Shot 2021-04-05 at 9 43 40 AM

So something is wrong here, maybe you don't call reset on the models ?

And indeed, even after a refresh of the page, you do see there are pending changes: Screen Shot 2021-04-05 at 9 47 46 AM

Note that clicking on "Undo changes" in the list of models to the left does reset the model changes. Screen Shot 2021-04-05 at 9 48 28 AM

  1. we don't want to have both "List" (bottom left) and "Basket" (title of modal). I prefer "List" as we are not an ecommerce Screen Shot 2021-04-05 at 9 44 49 AM
tmushayahama commented 3 years ago

For Number 3) Separating the Activities into group is not idea because the GPs are sorted alphanumerical. And its easy to visualize your GPs. @vanaukenk @vanaukenk can comment more on this.

The "activities bar" is there for the menu on the right side and clear separation between model list and model's contents list. I can decrease height to 30px.

tmushayahama commented 3 years ago

As for number 6) The reasoning was if you clear list nothing will be changed, it's like unselecting model. Neither "undo" or "save" will happen. It will just remove it from the basket untouched. The idea is because if you select someone's already edited model, we don't want to force save or undo just to unselect it

tmushayahama commented 3 years ago

I agree with the other issues especially 4 and 5, the workflow is not too smooth. We can discuss it soon if its only UI and quick fix for V1?

tmushayahama commented 3 years ago

Also we have to revisit the popups wording so they are all clear to what will happen (number 6)

lpalbou commented 3 years ago

For 6:

Screen Shot 2021-04-05 at 12 49 48 PM

So the second sentence is wrong; all your changes are actually saved but not stored. So nothing that was done was lost or reset, which may be an issue. Probable resolution: change the text, possibly change the actions too, eg: clear list without reset; clear list and reset; cancel. Need @ukemi & @vanaukenk feedback on what they want exactly here.

For 3:

Screen Shot 2021-04-05 at 12 55 44 PM

That's an awful lot of lost space just to fold/unfold models. Either we use that grey bar to fold activities together (like the title suggested) and have equivalent for BP & CC, or this should go away. If we remove the bar and still want the ability to fold/unfold everything, it's usually done by shift clicking the arrow up/down to fold/unfold and it's easy to teach to curators and is anyhow not blocking in their workflow.

tmushayahama commented 3 years ago

We will need more discussion on this. I think that bar should not be removed. 1) It is a divider to know that one is inside the a model or list of models as the model title and activity titles looks the same 2) The menu on the right contains more things that just the 2 options and I don't think keyboard and mouse shortcut binding is a way to go. 3) Most importantly, it is also a status bar like loading messages and errors like below

image

or saving model etc image

And mostly, I think we have lot of vertical space, and so not worried about letting it there.