Repository: chanind/hanzi-writer
Pull request #320 (2024-09-13 10:04:28)

Fix: Ensure consistent stroke drawing in quiz mode on mobile
### 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: :warning: Please install the !['codecov app svg image'](https://github.com/codecov/engineering-team/assets/152432831/e90313f4-9d3a-4b63-8b54-cfe14e7ec20d) to ensure uploads and comments are reliably processed by Codecov.

## [Codecov](https://app.codecov.io/gh/chanind/hanzi-writer/pull/320?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin) 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`)](https://app.codecov.io/gh/chanind/hanzi-writer/commit/b5c0d3910b853862eeea777aa1587b95ba6ce4fe?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin) to head [(`0f092f7`)](https://app.codecov.io/gh/chanind/hanzi-writer/commit/0f092f73a8019ad28e694b9994ff2a09f7c5e582?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin).

| [Files with missing lines](https://app.codecov.io/gh/chanind/hanzi-writer/pull/320?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin) | Patch % | Lines |
|---|---|---|
| [src/Quiz.ts](https://app.codecov.io/gh/chanind/hanzi-writer/pull/320?src=pr&el=tree&filepath=src%2FQuiz.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin#diff-c3JjL1F1aXoudHM=) | 80.00% | [1 Missing :warning: ](https://app.codecov.io/gh/chanind/hanzi-writer/pull/320?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin) |

:exclamation: Your organization needs to install the [Codecov GitHub app](https://github.com/apps/codecov/installations/select_target) to enable full functionality.

<details><summary>Additional details and impacted files</summary>


```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              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/chanind/hanzi-writer/pull/320?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=David+Chanin).

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

Patch:
diff --git a/src/Quiz.ts b/src/Quiz.ts
index 61695b06..c452a68d 100644
--- a/src/Quiz.ts
+++ b/src/Quiz.ts
@@ -27,6 +27,7 @@ export default class Quiz {
   _mistakesOnStroke = 0;
   _totalMistakes = 0;
   _userStroke: UserStroke | undefined;
+  _userStrokesIds: Array<number> | undefined;
 
   constructor(character: Character, renderState: RenderState, positioner: Positioner) {
     this._character = character;
@@ -36,6 +37,13 @@ export default class Quiz {
   }
 
   startQuiz(options: ParsedHanziWriterOptions) {
+    if (this._userStrokesIds) {
+      this._renderState.run(
+        quizActions.removeAllUserStrokes( this._userStrokesIds ),
+      );
+    }
+    this._userStrokesIds = []
+
     this._isActive = true;
     this._options = options;
     const startIndex = fixIndex(
@@ -65,6 +73,7 @@ export default class Quiz {
     const point = this._positioner.convertExternalPoint(externalPoint);
     const strokeId = counter();
     this._userStroke = new UserStroke(strokeId, point, externalPoint);
+    this._userStrokesIds?.push(strokeId)
     return this._renderState.run(quizActions.startUserStroke(strokeId, point));
   }
 
@@ -88,7 +97,7 @@ export default class Quiz {
     if (!this._userStroke) return;
 
     this._renderState.run(
-      quizActions.removeUserStroke(
+      quizActions.hideUserStroke(
         this._userStroke.id,
         this._options!.drawingFadeDuration ?? 300,
       ),
@@ -152,12 +161,9 @@ export default class Quiz {
 
   cancel() {
     this._isActive = false;
-    if (this._userStroke) {
+    if (this._userStrokesIds) {
       this._renderState.run(
-        quizActions.removeUserStroke(
-          this._userStroke.id,
-          this._options!.drawingFadeDuration,
-        ),
+        quizActions.removeAllUserStrokes( this._userStrokesIds ),
       );
     }
   }
diff --git a/src/quizActions.ts b/src/quizActions.ts
index c88097c2..ef8865c1 100644
--- a/src/quizActions.ts
+++ b/src/quizActions.ts
@@ -53,16 +53,26 @@ export const updateUserStroke = (
   return [new Mutation(`userStrokes.${userStrokeId}.points`, points, { force: true })];
 };
 
-export const removeUserStroke = (
+export const hideUserStroke = (
   userStrokeId: string | number,
   duration: number,
 ): GenericMutation[] => {
   return [
     new Mutation(`userStrokes.${userStrokeId}.opacity`, 0, { duration }),
-    new Mutation(`userStrokes.${userStrokeId}`, null, { force: true }),
+    // Do not remove the stroke, keep it hidden until quiz ends
+    // This avoids a bug in which touchmove stops being triggered in the middle of a stroke
+    // the only doc i found https://stackoverflow.com/questions/29384973/touchmove-event-stops-triggering-after-any-element-is-removed-from-dom
+    // so if the user on his phone is too quick to start his new stroke, the new stroke may stops in mid air
+    //new Mutation(`userStrokes.${userStrokeId}`, null, { force: true }),
   ];
 };
 
+export const removeAllUserStrokes = (userStrokeIds: Array<number>): GenericMutation[] => {
+  return userStrokeIds?.map(userStrokeId =>
+    new Mutation(`userStrokes.${userStrokeId}`, null, { force: true })
+  ) || [];
+};
+
 export const highlightCompleteChar = (
   character: Character,
   color: ColorObject | null,


Test patch:
diff --git a/src/__tests__/Quiz-test.ts b/src/__tests__/Quiz-test.ts
index d2723e6d..3f28e7ff 100644
--- a/src/__tests__/Quiz-test.ts
+++ b/src/__tests__/Quiz-test.ts
@@ -206,6 +206,7 @@ describe('Quiz', () => {
       clock.tick(1000);
       await resolvePromises();
 
+      // should disappear
       expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
     });
   });
@@ -268,8 +269,8 @@ describe('Quiz', () => {
       clock.tick(1000);
       await resolvePromises();
 
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
   });
 
@@ -391,8 +392,8 @@ describe('Quiz', () => {
 
       expect(renderState.state.character.main.strokes[0].opacity).toBe(1);
       expect(renderState.state.character.main.strokes[1].opacity).toBe(0);
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
 
     it('accepts backwards stroke when allowed', async () => {
@@ -456,8 +457,8 @@ describe('Quiz', () => {
 
       expect(renderState.state.character.main.strokes[0].opacity).toBe(1);
       expect(renderState.state.character.main.strokes[1].opacity).toBe(0);
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
 
     it('notes backwards stroke when checking', async () => {
@@ -521,8 +522,8 @@ describe('Quiz', () => {
 
       expect(renderState.state.character.main.strokes[0].opacity).toBe(0);
       expect(renderState.state.character.main.strokes[1].opacity).toBe(0);
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
 
     it('ignores single point strokes', async () => {
@@ -561,8 +562,8 @@ describe('Quiz', () => {
 
       expect(renderState.state.character.main.strokes[0].opacity).toBe(0);
       expect(renderState.state.character.main.strokes[1].opacity).toBe(0);
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
 
     it('stays on the stroke if it was incorrect', async () => {
@@ -619,8 +620,8 @@ describe('Quiz', () => {
 
       expect(renderState.state.character.main.strokes[0].opacity).toBe(0);
       expect(renderState.state.character.main.strokes[1].opacity).toBe(0);
-      // should fade and disappear
-      expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
+      // should fade
+      expect(renderState.state.userStrokes![currentStrokeId]?.opacity).toBe(0);
     });
 
     it('highlights the stroke if the number of mistakes exceeds showHintAfterMisses', async () => {