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

Add snap and lock angle modifiers for handle dragging to the Path tool #2160

Merged
merged 8 commits into from
Dec 31, 2024

Conversation

bakayu
Copy link
Contributor

@bakayu bakayu commented Dec 24, 2024

Closes #1870 partly...
This PR aims to close the following parts of #1870 :

  • Shift locks the handle angle to 15° increments
    • (shouldn't interfere with Shift-clicking handles for multi-selecting points such as handles)
  • Ctrl locks the current angle of the handle

@Keavon
Copy link
Member

Keavon commented Dec 25, 2024

!build

Copy link

📦 Build Complete for acd7e1e
https://31e24e5f.graphite.pages.dev

@bakayu bakayu marked this pull request as ready for review December 26, 2024 14:15
@bakayu
Copy link
Contributor Author

bakayu commented Dec 27, 2024

@Keavon This PR is now ready for review!
Please produce a dev build for testing ^^

@Keavon
Copy link
Member

Keavon commented Dec 28, 2024

!build

@Keavon Keavon changed the title added snap and lock angle feature to path tool Add snap and lock angle modifiers to the Path tool Dec 28, 2024
Copy link

📦 Build Complete for c4b1bdf
https://541f50db.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Dec 28, 2024

Please check off the boxes in the issue description for what's implemented now with this PR.

@bakayu
Copy link
Contributor Author

bakayu commented Dec 28, 2024

By now, this PR has implemented all of the listed features. Updated the description as well.

Edit:
Though I'm still skeptical about this part-

Please let me know if anything comes up during testing

@Keavon
Copy link
Member

Keavon commented Dec 28, 2024

QA feedback:

  • If the handles are not colinear, releasing Alt should put the opposite handle back where it started before Alt was pressed so the user can have the freedom to press Alt any number of times without worrying about affecting things by changing your mind and releasing Alt.

@bakayu
Copy link
Contributor Author

bakayu commented Dec 28, 2024

QA feedback:

  • If the handles are not colinear, releasing Alt should put the opposite handle back where it started before Alt was pressed so the user can have the freedom to press Alt any number of times without worrying about affecting things by changing your mind and releasing Alt.

Noted.
For now, I have reverted the commit that aimed to fix this, I will address this later in a seperate PR.

Updated the description to reflect what this PR currently solves. This much should be ready for reviewing and merging.

@Keavon Keavon force-pushed the path-tool-update branch 2 times, most recently from b0b79be to 59635c8 Compare December 31, 2024 08:42
@Keavon
Copy link
Member

Keavon commented Dec 31, 2024

!build

Copy link

📦 Build Complete for 59635c8
https://c7313d54.graphite.pages.dev

bakayu and others added 7 commits December 31, 2024 11:10
- Previous implementation broke functionality of using Tab to swap the being-dragged handle to its opposing handle, Now fixed.
- Previous implementation broke functionality of using space to drag the manipulator group (anchor + handles) while dragging a handle, Now fixed.
Now, if `shift` is used to snap to a 15° increment, then `ctrl` is used to preserve the angle, releasing the `shift` key will still preserve the angle.
Now, temporarily converts selected handles to colinear if they are not already colinear.
@Keavon
Copy link
Member

Keavon commented Dec 31, 2024

!build

@Keavon
Copy link
Member

Keavon commented Dec 31, 2024

There's one more issue I found, but it can be fixed in the next PR you work on:

  • If you begin dragging a handle with Ctrl already pressed, it uses the angle of the previously selected handle rather than the being-dragged handle.

@bakayu
Copy link
Contributor Author

bakayu commented Dec 31, 2024

I will continue the work in a couple days, will fix it then.

@Keavon
Copy link
Member

Keavon commented Dec 31, 2024

Oh and one more:

  • If I'm holding Ctrl to lock the angle, then add Shift to snap it to 15°, then release Shift, it should return to its previously locked angle that was set when the currently held-down Ctrl was pressed down. This also needs to be true for the Line tool, which isn't currently the case. Also for the Pen tool, which currently doesn't even let you add Shift while Ctrl is held. What I'm describing should be its own PR.

Update per your feedback on Discord: instead of restoring the old angle, Ctrl being held down should make the addition of Shift be ignored, thus keeping the locking angle and giving it precedence over the 15 degree angle snapping, until Ctrl is released.

Copy link

📦 Build Complete for 192f314
https://75f6b714.graphite.pages.dev

@Keavon Keavon enabled auto-merge (squash) December 31, 2024 19:34
@Keavon Keavon disabled auto-merge December 31, 2024 19:34
@Keavon Keavon changed the title Add snap and lock angle modifiers to the Path tool Add snap and lock angle modifiers for handle dragging to the Path tool Dec 31, 2024
@Keavon Keavon enabled auto-merge (squash) December 31, 2024 19:34
@Keavon Keavon merged commit 39a7b76 into GraphiteEditor:master Dec 31, 2024
3 of 4 checks passed
bakayu added a commit to bakayu/Graphite that referenced this pull request Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue: Pen and Path tool improvements
3 participants