Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

onChange behaviour is not as expected #117

Closed
DipanshKhandelwal opened this issue Mar 30, 2021 · 1 comment
Closed

onChange behaviour is not as expected #117

DipanshKhandelwal opened this issue Mar 30, 2021 · 1 comment

Comments

@DipanshKhandelwal
Copy link

1. triggerOnChange is never called on clear

clear = () => {
this.lines = [];
this.valuesChanged = true;
this.ctx.drawing.clearRect(
0,
0,
this.canvas.drawing.width,
this.canvas.drawing.height
);
this.ctx.temp.clearRect(
0,
0,
this.canvas.temp.width,
this.canvas.temp.height
);
};

2. triggerOnChange is called multiple times on calling undo: [ number of lines left on undo + 1]

triggerOnChange is called at the end of undo :

undo = () => {
const lines = this.lines.slice(0, -1);
this.clear();
this.simulateDrawingLines({ lines, immediate: true });
this.triggerOnChange();
};

But, simulateDrawingLines is also called on undo before triggerOnChange.

Now, simulateDrawingLines calls saveLine multiple times. Check line 276:

simulateDrawingLines = ({ lines, immediate }) => {
// Simulate live-drawing of the loaded lines
// TODO use a generator
let curTime = 0;
let timeoutGap = immediate ? 0 : this.props.loadTimeOffset;
lines.forEach(line => {
const { points, brushColor, brushRadius } = line;
// Draw all at once if immediate flag is set, instead of using setTimeout
if (immediate) {
// Draw the points
this.drawPoints({
points,
brushColor,
brushRadius
});
// Save line with the drawn points
this.points = points;
this.saveLine({ brushColor, brushRadius });
return;
}
// Use timeout to draw
for (let i = 1; i < points.length; i++) {
curTime += timeoutGap;
window.setTimeout(() => {
this.drawPoints({
points: points.slice(0, i + 1),
brushColor,
brushRadius
});
}, curTime);
}
curTime += timeoutGap;
window.setTimeout(() => {
// Save this line with its props instead of this.props
this.points = points;
this.saveLine({ brushColor, brushRadius });
}, curTime);
});
};

And saveLine calls triggerOnChange at the end. Check line 446:

saveLine = ({ brushColor, brushRadius } = {}) => {
if (this.points.length < 2) return;
// Save as new line
this.lines.push({
points: [...this.points],
brushColor: brushColor || this.props.brushColor,
brushRadius: brushRadius || this.props.brushRadius
});
// Reset points array
this.points.length = 0;
const width = this.canvas.temp.width;
const height = this.canvas.temp.height;
// Copy the line to the drawing canvas
this.ctx.drawing.drawImage(this.canvas.temp, 0, 0, width, height);
// Clear the temporary line-drawing canvas
this.ctx.temp.clearRect(0, 0, width, height);
this.triggerOnChange();
};

@embiem
Copy link
Owner

embiem commented Nov 7, 2021

thanks for the in-depth analysis. Seems to be related to #93, so I'll track this issue in there.

@embiem embiem closed this as completed Nov 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants