chanind / hanzi-writer

Chinese character stroke order animations and practice quizzes
https://hanziwriter.org
MIT License
3.57k stars 557 forks source link

Fix: Ensure consistent stroke drawing in quiz mode on mobile #320

Closed plduhoux closed 1 month ago

plduhoux commented 2 months ago

Problem

When drawing a Hanzi character in quiz mode on mobile devices, certain strokes were being interrupted or stopped halfway through when wrote quickly. This was caused by the touchmove event ceasing to fire. The root issue was that a node within the SVG element was being removed during the stroke drawing process, disrupting the touch event stream. I found this issue which relates a similar problem : https://stackoverflow.com/questions/29384973/touchmove-event-stops-triggering-after-any-element-is-removed-from-dom

Solution

Instead of removing strokes immediately after they are drawn, I modified the logic to preserve the strokes by keeping their corresponding IDs. These strokes are now removed only at the end of the quiz session (on cancel or startQuiz). This prevents disruption of the touchmove event and allows for uninterrupted stroke drawing.

Testing

Tested on mobile device to ensure strokes are drawn smoothly and consistently. (in my app : https://hanziquest.plduhoux.fr/ where I frequently had the issue before these changes and no longer have these) Verified that the strokes are correctly removed when the quiz ends (I updated the tests so we test that strokes opacity is 0 instead of testing it was removed, I kept the test where the stroke have been removed after a cancel call)

codecov-commenter commented 2 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.36%. Comparing base (b5c0d39) to head (0f092f7). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/Quiz.ts 80.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #320 +/- ## ========================================== - Coverage 96.43% 96.36% -0.07% ========================================== Files 32 32 Lines 1149 1156 +7 Branches 214 216 +2 ========================================== + Hits 1108 1114 +6 - Misses 37 38 +1 Partials 4 4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

chanind commented 1 month ago

That sounds like it was hard to debug, but looks like a reasonable solution! Thanks for submitting this!

chanind commented 1 month ago

:tada: This PR is included in version 3.7.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

plduhoux commented 1 month ago

Thanks for merging it ! That was hard to debug effectively, but I really needed it 😅