Adds zoom fit selected and zoom fit all functionalities. #509
Conversation
Integrate upstream
…o upstream_zoom_fit
Llcoolsouder
left a comment
Llcoolsouder
left a comment
There was a problem hiding this comment.
The previous version of contextMenuEvent allows subclasses to completely customize what shows in the context menu. This change would make it so every subclass gets the zoom actions menu (but only if connected to a DataFlowGraphModel).
It might be more in the spirit to the original to keep the new zoom actions as public methods, but leave them disconnected, allowing end users to connect them where they see fit. (We could have a whole collection of inherited default impl actions that could be useful in this way.)
| { | ||
| if (itemAt(event->pos())) { | ||
| QGraphicsView::contextMenuEvent(event); | ||
| return; | ||
| } | ||
| QGraphicsView::contextMenuEvent(event); | ||
| QMenu *menu = nullptr; | ||
|
|
||
| if (!nodeScene()) return; | ||
| bool isZoomFitMenu = false; | ||
|
|
||
| auto const scenePos = mapToScene(event->pos()); | ||
| auto *dfModel = &nodeScene()->graphModel(); | ||
| auto n = qgraphicsitem_cast<NodeGraphicsObject *>(itemAt(event->pos())); | ||
|
|
||
| QMenu *menu = nodeScene()->createSceneMenu(scenePos); | ||
| if (dfModel && n) { | ||
| isZoomFitMenu = dfModel->nodeZoomFitMenu(n->nodeId()); | ||
| } | ||
|
|
||
| if (itemAt(event->pos()) && isZoomFitMenu) { | ||
| menu = nodeScene()->createZoomMenu(mapToScene(event->pos())); | ||
| } else if (!itemAt(event->pos())) { | ||
| menu = nodeScene()->createSceneMenu(mapToScene(event->pos())); | ||
| } |
There was a problem hiding this comment.
A couple thoughts here:
- There's already a signal we could connect to for node-specific context menus (i.e. on node right clicked) so this might not actually be the best place for checking for
itemAt()and calling our menu code.
https://github.com/search?q=repo%3Apaceholder%2Fnodeeditor%20nodeContextMenu&type=code - Zoom to fit selected (or all) aren't really specific to the node you're right clicking. Shouldn't these menu items be added to the more general scene menu? That way, right clicking empty space brings up the zoom options, but right clicking a node brings up the node-specific actions.
There was a problem hiding this comment.
Okay, I'll check that signal and try changing how the verification and call are made. Thanks for the suggestion!
Regarding the zoom fit options, the feature works as follows:
- Zoom Fit Selected zoom fits the selected nodes.
- Zoom Fit All zoom fits all the nodes in the canvas.
The right-click context menu is only triggered when the right button is on a selected node because right-clicking on the empty screen triggers the node context menu. Therefore, we thought it would be better to do this so as not to create an extensive context menu with multiple options (i.e., nodes and node-specific actions).
There was a problem hiding this comment.
Remember that this is just one example of how to use the Zoom Fit feature. Therefore, this feature can be implemented in the application in many ways by the developer.
4 participants
Type of change
Description
The zoom fit selected and zoom fit all features allow for the user to change focus between one, some or all exiting nodes, fitting them in the view accordingly. The video below demonstrates both functionalities:
Zoom_fit_test.mp4
Testing