Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nodes tree UI/UX #2606

Merged
merged 22 commits into from
Oct 29, 2021
Merged

Conversation

odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Oct 27, 2021

What do these changes do?

  • Sort nodes
  • Bind node's running state to its icon color
  • Add "new node" button that opens service catalog

NodesTree

Related issue/s

related to ITISFoundation/osparc-issues#546

How to test

Checklist

@odeimaiz odeimaiz added the a:frontend issue affecting the front-end (area group) label Oct 27, 2021
@odeimaiz odeimaiz added this to the Anti-PER milestone Oct 27, 2021
@odeimaiz odeimaiz self-assigned this Oct 27, 2021
@odeimaiz odeimaiz changed the title Nodes tree improvements WIP: Nodes tree improvements Oct 27, 2021
@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #2606 (8e50ca7) into master (3aff410) will increase coverage by 0.0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2606   +/-   ##
======================================
  Coverage    77.0%   77.0%           
======================================
  Files         631     631           
  Lines       24756   24756           
  Branches     2432    2432           
======================================
+ Hits        19076   19081    +5     
+ Misses       5039    5035    -4     
+ Partials      641     640    -1     
Flag Coverage Δ
integrationtests 66.6% <ø> (-0.1%) ⬇️
unittests 71.5% <ø> (+<0.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
..._director_v2/modules/dynamic_sidecar/client_api.py 77.2% <0.0%> (-1.3%) ⬇️
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 90.6% <0.0%> (-1.2%) ⬇️
.../director/src/simcore_service_director/producer.py 61.2% <0.0%> (-0.5%) ⬇️
...simcore_service_webserver/projects/projects_api.py 84.8% <0.0%> (+0.3%) ⬆️
...rector_v2/modules/comp_scheduler/base_scheduler.py 93.6% <0.0%> (+0.7%) ⬆️
...ce_webserver/resource_manager/garbage_collector.py 76.0% <0.0%> (+1.6%) ⬆️
...simcore_service_director_v2/modules/dask_client.py 94.1% <0.0%> (+1.6%) ⬆️
...ector_v2/modules/comp_scheduler/background_task.py 94.1% <0.0%> (+1.9%) ⬆️
...ore_service_director_v2/utils/client_decorators.py 80.7% <0.0%> (+7.6%) ⬆️

@odeimaiz odeimaiz changed the title WIP: Nodes tree improvements Nodes tree improvements Oct 27, 2021
@odeimaiz odeimaiz changed the title Nodes tree improvements Nodes tree UI/UX Oct 27, 2021
@mguidon
Copy link
Member

mguidon commented Oct 27, 2021

One thing immediately pops to mind:
I guess the reason for having the Add Nodes button is to have quick access to add nodes. I think having to move the mouse over to the services dialog defeats that purpose. Maybe directly show a drop-down with all possible services? (of course with the corresponding service thmubnail as icon...)

@odeimaiz
Copy link
Member Author

odeimaiz commented Oct 27, 2021

One thing immediately pops to mind: I guess the reason for having the Add Nodes button is to have quick access to add nodes. I think having to move the mouse over to the services dialog defeats that purpose. Maybe directly show a drop-down with all possible services? (of course with the corresponding service thmubnail as icon...)

We got some feedback from Wales saying that it's not easy to know what to do when you have a workbench with a node already there, meaning that the "double click" message is not shown anymore. You can get to that point if you start a study that already contains a node, so he suggested the "Add new node" button.

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about having a + button without the text on the side somehow. it feels like this button is lost in the middle (same feeling about the artifacts button). other than that I like a lot the colour changes

@schilds77
Copy link

Nice, I love the sorting of the nodes and the new icon style, definitely where we want to go. A very solid step in the right direction.

As for the add node button: I see the need, and I would leave it like that for now. I had a slightly different idea in my mental backlog though: What we haven't covered so far with regard to the two column design is what to do if no node is selected (in fact we haven't decided if empty selection is allowed at all). I would allow that, which could also be the default state for a newly created study, and embed the study selection view into the second column... That way it's really close to the mouse and doesn't consume other view real estate...

Next we can play with that node selection view in the secondary column: E.g. we could add inline + buttons to the right of nodes, which would bring up this node selection view, which would allow the user to create new nodes already connected correctly. Or we could vary this and have new nodes connected to the currently selected nodes etc... You see I haven't reached the end of that line of thought yet!

And so I think we are not yet quite there for this to be finalised. Once the main tree gets some more love (sections, icons etc...), then I think it's the right time to work on this!

For now I think this is very good!

Copy link

@schilds77 schilds77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good step, go for it!

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loving it!

services/director/src/simcore_service_director/producer.py Outdated Show resolved Hide resolved
Copy link
Contributor

@KZzizzle KZzizzle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really cool! In light of StS's comments I think it's good to go. The only comment I would have had is if when no nodes are selected to default to the study options since it's a bit weird to have the second column completely empty (even if the user can collapse it manually)

@odeimaiz odeimaiz merged commit e82aea3 into ITISFoundation:master Oct 29, 2021
@odeimaiz odeimaiz deleted the feature/ui-ux-nodes-tree branch April 21, 2022 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:frontend issue affecting the front-end (area group)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants