Skip to content

Properly handle pen tool undo and redo - #1587

Merged
0HyperCube merged 2 commits into
GraphiteEditor:masterfrom
zhiyuang:feature-pen-tool-undo
Feb 4, 2024
Merged

Properly handle pen tool undo and redo#1587
0HyperCube merged 2 commits into
GraphiteEditor:masterfrom
zhiyuang:feature-pen-tool-undo

Conversation

@zhiyuang

@zhiyuang zhiyuang commented Jan 26, 2024

Copy link
Copy Markdown
Contributor

Closes #971

refer to Figma

@github-actions

Copy link
Copy Markdown
📦 Build Complete for 5f4765d
https://dbdaca0b.graphite.pages.dev

@Keavon

Keavon commented Jan 27, 2024

Copy link
Copy Markdown
Member

This is a phenomenal improvement that was much needed! I spent 30 seconds testing it but I'll do a closer look tomorrow. I should hopefully get to reviewing both of your PRs tomorrow (I've been rather consumed by some code I've been working on which has been where all my attention has gone this past week).

@0HyperCube 0HyperCube left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this great improvement - the code looks good.

From a UX perspective I'm not sure how we want to deal with undo after drawing a complete path and confirming. Inkscape deletes the whole path. Figma switches back to the pen tool and does undo point by point. This PR currently just does undo point by point without selecting the path tool.

@Keavon

Keavon commented Jan 27, 2024

Copy link
Copy Markdown
Member

I'm also unsure. I was slightly surprised when it happened while I was testing it since that's not what I'm used to, but when I thought about it, I realized that might actually be desired. I'd suggest we merge as-is and think about the UX more over time and take into consideration the pros/cons throughout its usage and any potential future user feedback.

@0HyperCube
0HyperCube enabled auto-merge (squash) February 4, 2024 19:17
@0HyperCube
0HyperCube merged commit 05b4582 into GraphiteEditor:master Feb 4, 2024
Keavon added a commit that referenced this pull request Sep 6, 2026
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.

Undo/redo last point extended using the Pen tool

3 participants