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 joining of path endpoints with Ctrl+J in the Path tool #2227

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

Sidharth-Singh10
Copy link
Contributor

fixes: https://discord.com/channels/731730685944922173/881073965047636018/1289168449117097994

Implements a keyboard shortcut (Ctrl+J) to close open paths, with support for multiple scenarios:

  1. With selected points: Connects two selected endpoints
  2. Without selection: Automatically closes single continuous subpaths by connecting their endpoints

The implementation:

  • Checks that selected points are endpoints before connecting
  • Handles points in different layers by copying to topmost layer
  • Only connects valid endpoints (points connected to single segment)
  • Creates straight line segments without handles

@Sidharth-Singh10 Sidharth-Singh10 changed the title feat(path-tool): ctrlJ to join endpoints feat(path-tool): ctrl-J to join endpoints Jan 26, 2025
@Keavon Keavon changed the title feat(path-tool): ctrl-J to join endpoints Add joining of path endpoints with Ctrl+J in the Path tool Jan 29, 2025
@Keavon
Copy link
Member

Keavon commented Jan 31, 2025

Please see the Pen tool for the behavior and code paths that occur when the endpoints of two layers are clicked on to draw a segment between the two. You'll notice how the two layers get joined into one. Check the graph to see the before and after of the node setup. Try reusing that code for when joining points across layers, which currently doesn't work (it just draws a line that happens to go to the location of the other one, on one of the pair of layers).

That'll be for a followup PR. I'm merging this now since that's somewhat involved and the PR works well in all the other cases at the moment. Thanks.

@Keavon Keavon enabled auto-merge (squash) January 31, 2025 05:37
@Keavon Keavon merged commit f462963 into GraphiteEditor:master Jan 31, 2025
4 checks passed
@Sidharth-Singh10
Copy link
Contributor Author

Got it! I’ll handle this in a follow-up PR

@Keavon
Copy link
Member

Keavon commented Jan 31, 2025

This also relates to some of the same concepts in #2184, particularly the video mentioned in part 4 at the very end of the issue.

@Sidharth-Singh10
Copy link
Contributor Author

Will look into it!!

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.

2 participants