From 22cafa679a8fe3836713fed5e81e008e8da2f07d Mon Sep 17 00:00:00 2001 From: Polo Date: Sun, 22 Sep 2024 20:13:03 +0200 Subject: [PATCH] fix: Ensure consistent stroke drawing in quiz mode on mobile (#320) * Fix touchmove stops in the middle of a stroke * Fix tests with new way of removing strokes --- src/Quiz.ts | 18 ++++++++++++------ src/__tests__/Quiz-test.ts | 25 +++++++++++++------------ src/quizActions.ts | 14 ++++++++++++-- 3 files changed, 37 insertions(+), 20 deletions(-) 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 | 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/__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 () => { 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): GenericMutation[] => { + return userStrokeIds?.map(userStrokeId => + new Mutation(`userStrokes.${userStrokeId}`, null, { force: true }) + ) || []; +}; + export const highlightCompleteChar = ( character: Character, color: ColorObject | null,