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

More intuitive add node menu #248

Closed
wants to merge 25 commits into from
Closed

More intuitive add node menu #248

wants to merge 25 commits into from

Conversation

ChemicalXandco
Copy link
Contributor

@ChemicalXandco ChemicalXandco commented Sep 16, 2018

At the moment, when you add a node this menu appears:

It:

  • is not very helpful
  • doesn't make use of space efficiently, so takes up lots of vertical space and barely any horizontal
  • when more nodes are added it will just take up more space, potentially filling the entire height of the window

This fixes it by adding categories which then list the nodes, this makes it easier to navigate and makes more sense.

Sometime in the future, when the documentation is resolved, there should be a way added to get information on each node from this menu.

@ChemicalXandco ChemicalXandco changed the title WIP Better, More intuitive Add node dialog Better, More intuitive Add node dialog Sep 29, 2018
@fabiencastan
Copy link
Member

Hi @ChemicalXandco ,
Thanks for the PR. We don't have the time to look at it right now. But we will come back to you asap.

@ChemicalXandco
Copy link
Contributor Author

Ok I understand.

@natowi
Copy link
Member

natowi commented Oct 11, 2018

@ChemicalXandco your new Add node menu, including the preview images and descriptions looks really nice. However I would propose to keep a variant of the old "Add node" menu:
right click -> new add node menu
left click -> display the search bar (only!) from the old "Add node" menu, but with node name auto-complete feature. So users familiar with the node-names can quickly add new nodes to the graph.

@ChemicalXandco
Copy link
Contributor Author

@natowi that might be useful but maybe I should just add a search bar, I myself don't feel the need to add like nodes very quickly, as the processing time far outweighs it any time spent sorting out the graph. I think it is better to use this all the time, as it helps new users a lot. Let me know what you think is best, but I think it is unnecessary to have 2 at the same time.

@natowi
Copy link
Member

natowi commented Oct 12, 2018

@ChemicalXandco A search bar would be nice. Can you maybe add a mouse double click event to the list elements to add nodes? As a user, this is how I would expect it to work.

@fabiencastan
Copy link
Member

Hi @ChemicalXandco,
Really sorry for the huge delay to answer and thanks for the contribution.

We agree that the current menu is too dense and difficult to read. On the other hand, the current proposition doesn't look like a right click menu and will not scale when we will continue to add new nodes.
The main usage of the graph editor is for advanced users, who want to navigate quickly.

So a first step would be to:

  • add sub-menus with categories, as in many nodal softwares (e.g. "Sparse Reconstruction", "Dense Reconstruction", "Utils", "Mesh Post-Processing")
  • add an "Help" tab on each node

Then, to make it easier for new users (as you suggested the need), we could consider an help entry in the menus which provides node description with help, with a way to create a new node from there. In that case, a modal window would make sense.

What do you think?

Would you be interested in contributing to these changes? If yes, we could discuss about it in more technical aspects. For instance, we try to limit as much as possible the knowledge of the nodes in the QML part. So if we add documentation on each node in should be on the node description on the Python side.

Best,
Fabien

@ChemicalXandco
Copy link
Contributor Author

ChemicalXandco commented Jan 28, 2019

@fabiencastan
I am interested in contributing to these changes as in its current form, I agree that it does not work as a right-click menu.

@ChemicalXandco ChemicalXandco changed the title Better, More intuitive Add node dialog WIP More intuitive Add node dialog Jan 29, 2019
@ChemicalXandco ChemicalXandco changed the title WIP More intuitive Add node dialog More intuitive add node menu + information on each node Mar 5, 2019
@fabiencastan
Copy link
Member

The graph should not have any knowledge of the nodes content, so the menu in the GraphEditor should be dynamically built based on the category names.

@fabiencastan
Copy link
Member

We will not merge an empty info widget. We don't merge intermediate steps into develop. The develop branch should be able to become a release if needed.
It will take a long time before we have full presentation for each node. To ease the merge, it would be better to split the problems into multiple PRs. So I would highly recommend to make a first PR only with the categories.

@natowi
Copy link
Member

natowi commented Mar 29, 2019

@ChemicalXandco nice work. I can help you out with the missing node descriptions.
Have a look at my WIP documentation. In the "complete node list" chapter you can find descriptions and references for the missing nodes.

I noted the Info window does not rescale with the window size.

Getting some information on existing nodes is helpful, but I think I would expect to get this information while browsing through the nodes. I would not want to insert each node to check the Info.
Idea: at the moment, when we open the add new node menu, the node panel is empty.
Maybe we could display the information in there, while we are browsing through the nodes?
This might also be a good place to place the node Info content (in a new tab inside the node pane instead of a separate window?).

- Categories are now strings in the node definitons
- Categories are now dynamically created
- Info widget removed, probably will be added later
@ChemicalXandco ChemicalXandco changed the title More intuitive add node menu + information on each node More intuitive add node menu Mar 31, 2019
@natowi natowi mentioned this pull request Jul 9, 2019
...when category is not set in the node definition script
...after the searchbar loses focus and has text
@natowi natowi added ready feature new feature (proposed as PR or issue planned by dev) labels Aug 16, 2019
@simogasp simogasp added the type:pr pull request issue label Sep 9, 2019
@natowi
Copy link
Member

natowi commented Oct 11, 2019

new nodes #639 and #390

category = 'Utils'

@ChemicalXandco
Copy link
Contributor Author

ChemicalXandco commented Nov 9, 2019

Would be good to get this PR merged for the next release

@natowi natowi mentioned this pull request Jan 24, 2020
15 tasks
@natowi
Copy link
Member

natowi commented Jan 24, 2020

@fabiencastan This would be really useful for the next release, as we have so many new nodes that do not fit all on screen with the old add node menu.

@natowi
Copy link
Member

natowi commented Apr 27, 2020

@ChemicalXandco fyi, we now have node documentation 0e606ee and 64a4c94

@natowi natowi mentioned this pull request Jun 29, 2020
@natowi
Copy link
Member

natowi commented Feb 12, 2021

@fabiencastan with the increasing number of nodes it would be great to get this PR included in the next release...

@fabiencastan
Copy link
Member

The PR needs to be updated. I have not tested it yet.

@ChemicalXandco
Copy link
Contributor Author

I can update it if we need this feature

@fabiencastan
Copy link
Member

Yes, it would be great to have categories in this menu, but we need to find a way to keep the search bar.

@ChemicalXandco
Copy link
Contributor Author

we need to find a way to keep the search bar

Last I checked this doesn't remove the search bar.

I shall make a new branch based on upstream develop because this one is too old.

@fabiencastan
Copy link
Member

Yes, I don't remember exactly how this version was working... :s
Thanks for doing it again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new feature (proposed as PR or issue planned by dev) ready type:pr pull request issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants