Skip to content

Commit

Permalink
fix: Ensure consistent stroke drawing in quiz mode on mobile (#320)
Browse files Browse the repository at this point in the history
* Fix touchmove stops in the middle of a stroke

* Fix tests with new way of removing strokes
  • Loading branch information
plduhoux authored Sep 22, 2024
1 parent b5c0d39 commit 22cafa6
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 20 deletions.
18 changes: 12 additions & 6 deletions src/Quiz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand Down Expand Up @@ -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));
}

Expand All @@ -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,
),
Expand Down Expand Up @@ -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 ),
);
}
}
Expand Down
25 changes: 13 additions & 12 deletions src/__tests__/Quiz-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ describe('Quiz', () => {
clock.tick(1000);
await resolvePromises();

// should disappear
expect(renderState.state.userStrokes![currentStrokeId]).toBe(null);
});
});
Expand Down Expand Up @@ -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);
});
});

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
14 changes: 12 additions & 2 deletions src/quizActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 22cafa6

Please sign in to comment.