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

Placement of newly opened Component Browser dictated by the mouse pointer. #3301

Merged
merged 213 commits into from
Mar 31, 2022

Conversation

akavel
Copy link
Contributor

@akavel akavel commented Feb 24, 2022

Pull Request Description

Use a new algorithm for placement of new nodes in cases when:

  • a) there is no selected node, and the TAB key is pressed while the mouse pointer is near an existing node (especially in an area below an existing node);
  • b) a connection is dragged out from an existing node and dropped near the node (especially in an area below the node).

In both cases mentioned above, the new node will now be placed in a location suggested by an internal algorithm, aligned to existing nodes. Specifically, the placement algorithm used is similar to when pressing TAB with a node selected.

For more details, see: https://www.pivotaltracker.com/story/show/181076066

Visuals

A demo of TAB key and edge dropping behavior:

Screen.Recording.2022-03-03.at.23.20.48.mov

Important Notes

  • Visible visualizations enabled with the "eye icon" button are treated as part of a node. (In case of nodes with errors, visualizations are not visible, and are not treated as part of a node.)

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH ./run dist and ./run watch.

@akavel akavel changed the title Placement of newly opened Component Browser dictated by the mouse pointer. [WAITING FOR #3328 FIX] Placement of newly opened Component Browser dictated by the mouse pointer. Mar 17, 2022
@akavel akavel requested a review from farmaazon March 17, 2022 09:13
Comment on lines 8 to 9
use crate::free_place_finder::find_free_place;
use crate::free_place_finder::OccupiedArea;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the free_place_finder module can be made a submodule of new_node_position. So far, the finder is used solely for searching for new node positions.

Comment on lines 1428 to 1438
let position: Vector2 = match way {
AddNodeEvent => default(),
StartCreationEvent | ClickingButton if selection.is_some() =>
self.find_free_place_under(selection.unwrap()),
StartCreationEvent => mouse_position,
new_node_position::under(self, selection.unwrap()),
StartCreationEvent =>
new_node_position::at_mouse_aligned_to_close_nodes(self, mouse_position),
ClickingButton =>
self.find_free_place_for_node(screen_center, Vector2(0.0, -1.0)).unwrap(),
DroppingEdge { .. } => mouse_position,
new_node_position::on_ray(self, screen_center, Vector2(0.0, -1.0)).unwrap(),
DroppingEdge { edge_id } =>
new_node_position::at_mouse_aligned_to_source_node(self, edge_id, mouse_position),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

The separate module made it look so nice! Perhaps this whole match should also be refactored to function inside new_node_position.

@akavel
Copy link
Contributor Author

akavel commented Mar 18, 2022

(Process note: the code-review is now Approved, therefore the PR is waiting for Review by @farmaazon on Task https://www.pivotaltracker.com/story/show/181076066 before merging.)

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Mar 31, 2022
@mergify mergify bot merged commit b8a5e22 into develop Mar 31, 2022
@mergify mergify bot deleted the wip/akavel/cb-placement-by-mouse-181076066 branch March 31, 2022 14:16
mergify bot pushed a commit that referenced this pull request Mar 31, 2022
…3361)

When a new node is created with the <kbd>TAB</kbd> key or by clicking the `(+)` on-screen button while multiple nodes are selected, place the new node below all the selected nodes. (Previously, the new node was placed below the node that was selected earliest.)

Additionally, when placing a new node below an existing non-error node with a visualization enabled, place the new node below the visualization. (Previously, the new node was placed to the left of the visualization.)

https://www.pivotaltracker.com/story/show/180887079

#### Visuals

The following screencast demonstrates the feature on various arrangements of selected nodes, with visualization enabled and disabled.

https://user-images.githubusercontent.com/273837/159971452-148aa4d7-c0f3-4b48-871a-a2783989f403.mov

The following screencast demonstrates that new nodes created by double-clicking an output port of a node with visualization enabled are now placed below the visualization:

https://user-images.githubusercontent.com/273837/160107733-e3f7d0f9-0161-49d1-8cbd-06e18c843a20.mov

# Important Notes
- Some refactorings that were needed for this PR were ported from the #3301 PR:
- the code responsible for calculating the positions of new nodes was moved to a separate module (`new_node_position`);
- the `free_place_finder` module was made a submodule of the `new_node_position` module, due to the latter being its only user.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants