Skip to content

/ Graphite

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

Jump to bottom

Cancel in-progress drag with right click or escape key #119

Merged
merged 1 commit into from May 9, 2021
Merged

Cancel in-progress drag with right click or escape key #119

merged 1 commit into from May 9, 2021

Conversation

@pkupper
Copy link
Collaborator

@pkupper pkupper commented May 8, 2021 •
edited by Keavon

Closes #116 by allowing the user to cancel rectangle, ellipse, shape and line creation by either pressing the escape key or the right mouse button. On the pen tool the same action results in the already placed points being committed.

Closes #109 by not committing empty polylines or adding redundant points when clicking the same location twice in a row using the pen tool.

This change is Reviewable
@pkupper pkupper force-pushed the tool-improvements branch from dcdf567 to 30c3477 May 8, 2021
@cloudflare-pages
Copy link

@cloudflare-pages cloudflare-pages bot commented May 8, 2021 •
edited

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: b20d607
Status: ✅  Deploy successful!
Preview URL: https://09945588.graphite-master.pages.dev

View logs
@pkupper pkupper force-pushed the tool-improvements branch from 30c3477 to 5aa27a8 May 8, 2021
@pkupper pkupper requested review from Keavon and TrueDoctor May 8, 2021
@TrueDoctor
TrueDoctor requested changes May 8, 2021
View changes
Copy link
Member

@TrueDoctor TrueDoctor left a comment

Reviewed 9 of 9 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Keavon and @pkupper)

core/editor/src/tools/ellipse.rs, line 82 at r1 (raw file):
For now you could just add

(EllipseToolFsmState::LmbDown, Event::KeyUp(Key::KeyEscape)) | EllipseToolFsmState::LmbDown, Event::RmbDown(_)) => {
…
}
> ```

core/editor/src/tools/line.rs, line 89 at r1 (raw file):

			}
			// TODO - join match arms with or_patterns when available in stable rust (https://github.com/rust-lang/rust/issues/54883)
			(LineToolFsmState::LmbDown, Event::KeyUp(Key::KeyEscape)) => {

same thing here

core/editor/src/tools/pen.rs, line 65 at r1 (raw file):

			}
			(PenToolFsmState::LmbDown, Event::LmbUp(mouse_state)) => {
				if data.points.last() != Some(&mouse_state.position) {

comparing floats is not ideal. It should not make a difference in this case but there might be a case in which something changes between the calculations, so I'd suggest you calculate the distance instead and compare that to some epsilon

core/editor/src/tools/pen.rs, line 80 at r1 (raw file):

				PenToolFsmState::LmbDown
			}
			// TODO - join match arms with or_patterns when available in stable rust (https://github.com/rust-lang/rust/issues/54883)

See above

core/editor/src/tools/rectangle.rs, line 81 at r1 (raw file):

				RectangleToolFsmState::Ready
			}
			// TODO - join match arms with or_patterns when available in stable rust (https://github.com/rust-lang/rust/issues/54883)

You get the gist 😂
@pkupper pkupper force-pushed the tool-improvements branch from 5aa27a8 to 1b823ea May 8, 2021
@pkupper
pkupper reviewed May 8, 2021
View changes
Copy link
Collaborator Author

@pkupper pkupper left a comment

Reviewable status: 4 of 9 files reviewed, 5 unresolved discussions (waiting on @Keavon and @TrueDoctor)

core/editor/src/tools/ellipse.rs, line 82 at r1 (raw file):

Previously, TrueDoctor wrote…

For now you could just add

(EllipseToolFsmState::LmbDown, Event::KeyUp(Key::KeyEscape)) | EllipseToolFsmState::LmbDown, Event::RmbDown(_)) => {
…
}
> ```

</blockquote></details>

Done.
___
*[core/editor/src/tools/line.rs, line 89 at r1](https://reviewable.io/reviews/graphiteeditor/graphite/119#-M_Be5PACD1xTeE04mVP:-M_Bq6JWEPyHveouX7d4:b-896fix) ([raw file](https://github.com/graphiteeditor/graphite/blob/5aa27a841c3a98305be38236b2cbc9a4ad3c8b07/core/editor/src/tools/line.rs#L89)):*
<details><summary><i>Previously, TrueDoctor wrote…</i></summary><blockquote>

same thing here
</blockquote></details>

Done.
___
*[core/editor/src/tools/pen.rs, line 65 at r1](https://reviewable.io/reviews/graphiteeditor/graphite/119#-M_BeL2ZAS7OX28-KHcm:-M_Bq9az0pf4sn3MM3x8:b-dbus1r) ([raw file](https://github.com/graphiteeditor/graphite/blob/5aa27a841c3a98305be38236b2cbc9a4ad3c8b07/core/editor/src/tools/pen.rs#L65)):*
<details><summary><i>Previously, TrueDoctor wrote…</i></summary><blockquote>

comparing floats is not ideal. It should not make a difference in this case but there might be a case in which something changes between the calculations, so I'd suggest you calculate the distance instead and compare that to some epsilon
</blockquote></details>

These are still ints. Added TODO for after switching to canvas coordinates.
___
*[core/editor/src/tools/pen.rs, line 80 at r1](https://reviewable.io/reviews/graphiteeditor/graphite/119#-M_BfQKw1UhTgOdq4nBl:-M_BqMBz5q9oZhVu7Dq1:b-q64pkt) ([raw file](https://github.com/graphiteeditor/graphite/blob/5aa27a841c3a98305be38236b2cbc9a4ad3c8b07/core/editor/src/tools/pen.rs#L80)):*
<details><summary><i>Previously, TrueDoctor wrote…</i></summary><blockquote>

See above
</blockquote></details>

See above.
___
*[core/editor/src/tools/rectangle.rs, line 81 at r1](https://reviewable.io/reviews/graphiteeditor/graphite/119#-M_BfwE_3dAhB3cPbyvj:-M_BqSHZ5tFZ5mnjggPu:b-896fix) ([raw file](https://github.com/graphiteeditor/graphite/blob/5aa27a841c3a98305be38236b2cbc9a4ad3c8b07/core/editor/src/tools/rectangle.rs#L81)):*
<details><summary><i>Previously, TrueDoctor wrote…</i></summary><blockquote>

You get the gist :joy: 
</blockquote></details>

Done.


<!-- Sent from Reviewable.io -->
@TrueDoctor
TrueDoctor approved these changes May 8, 2021
View changes
Copy link
Member

@TrueDoctor TrueDoctor left a comment

Reviewed 5 of 5 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Keavon)
@Keavon
Keavon reviewed May 9, 2021
View changes
client/web/public/index.html Outdated Show resolved Hide resolved
@Keavon
Keavon reviewed May 9, 2021
View changes
core/editor/src/tools/ellipse.rs Outdated Show resolved Hide resolved
@Keavon
Keavon approved these changes May 9, 2021
View changes
@Keavon
Keavon approved these changes May 9, 2021
View changes
Copy link
Member

@Keavon Keavon left a comment

Reviewed 4 of 9 files at r1, 5 of 5 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pkupper)
@pkupper
Cancel in-progress drag with right click or escape key
Loading status checks…
b20d607
@pkupper pkupper force-pushed the tool-improvements branch from 1b823ea to b20d607 May 9, 2021
@Keavon
Keavon approved these changes May 9, 2021
View changes
Copy link
Member

@Keavon Keavon left a comment

Reviewed 7 of 7 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @pkupper)
@Keavon Keavon merged commit 57252fa into master May 9, 2021
2 of 3 checks passed
2 of 3 checks passed
@github-actions
build build
Details
code-review/reviewable 2 discussions left (pkupper)
Details
@cloudflare-pages
Cloudflare Pages Deployed successfully
Details
@Keavon Keavon deleted the tool-improvements branch May 9, 2021
0HyperCube added a commit that referenced this pull request May 14, 2021
@pkupper @0HyperCube
Cancel in-progress drag with right click or escape key (#119)
46c4266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Keavon Keavon

@TrueDoctor TrueDoctor

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Add the Escape and RMB inputs as ways to cancel the in-progress shape drag
The Pen Tool adds a layer when it is completed after only creating the starting point
3 participants
@pkupper @Keavon @TrueDoctor