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

feat: add s-pen button erasing #783

Merged
merged 2 commits into from
Jul 8, 2023
Merged

feat: add s-pen button erasing #783

merged 2 commits into from
Jul 8, 2023

Conversation

ZebraVogel94349
Copy link
Contributor

This adds the feature to switch to the eraser while the button on a stylus (tested with the Samsung S-Pen) is pressed and to switch back to the previous tool when the button is released, which is similar to the behavior in Samsung Notes.

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Merging #783 (26297f1) into main (1a42116) will increase coverage by 0.00%.
The diff coverage is 40.74%.

❗ Current head 26297f1 differs from pull request most recent head eab6a1a. Consider uploading reports for the commit eab6a1a to get more accurate results

@@           Coverage Diff           @@
##             main     #783   +/-   ##
=======================================
  Coverage   43.64%   43.64%           
=======================================
  Files          91       91           
  Lines        6384     6399   +15     
=======================================
+ Hits         2786     2793    +7     
- Misses       3598     3606    +8     
Impacted Files Coverage Δ
lib/components/toolbar/toolbar.dart 72.52% <0.00%> (+0.78%) ⬆️
lib/pages/editor/editor.dart 35.57% <31.57%> (-0.23%) ⬇️
lib/components/canvas/canvas_gesture_detector.dart 73.37% <71.42%> (+0.36%) ⬆️

... and 2 files with indirect coverage changes

@adil192 adil192 marked this pull request as draft July 8, 2023 17:42
@adil192
Copy link
Member

adil192 commented Jul 8, 2023

@ZebraVogel94349 Thanks for this! Would you be able to test the new changes (mostly the part where the user is already drawing a stroke and presses the eraser button)?

@ZebraVogel94349
Copy link
Contributor Author

ZebraVogel94349 commented Jul 8, 2023

@adil192 I tested all new changes with my Tab S7 FE and the S-Pen and it's working fine. If a the user is already drawing a stroke and presses the eraser button, the currently drawn stroke gets removed and the eraser is selected.
The only problem is that the button press only gets detected when the pen is already touching the screen, but not when hovering. I tried using the onPointerHover event for that, which worked for switching the tools, but you can't draw anything when pressing the button before touching the screen, due to this issue in flutter. I don't know if it only affects Samsung devices or also any other device with a stylus.

@adil192
Copy link
Member

adil192 commented Jul 8, 2023

I'll merge this in now. Hopefully the flutter issue is fixed soonish!
Thanks again :)

@adil192 adil192 marked this pull request as ready for review July 8, 2023 22:10
@adil192 adil192 merged commit 833fa4b into saber-notes:main Jul 8, 2023
@TomBPotochek
Copy link

I think this change made it so it is now impossible to manually erase with the eraser. When I select the eraser and try to use it, it immediately switches back to the pen tool. Happens with both s pen and finger drawing.

@adil192
Copy link
Member

adil192 commented Jul 9, 2023

I think this change made it so it is now impossible to manually erase with the eraser. When I select the eraser and try to use it, it immediately switches back to the pen tool. Happens with both s pen and finger drawing.

This should be fixed in #785

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.

3 participants