Js fixes #286
Js fixes #286
Conversation
…factor into two handlers based on button press.
…code behavior was always correct.
whitequark
force-pushed
the
master
branch
3 times, most recently
from
371f33b to
fb1065d
Compare
| .multiplyScalar(1 / object.zoomScale); | ||
| object.rotate(-0.3 * Math.PI / 180 * incDiff.x * object.zoomScale, | ||
| -0.3 * Math.PI / 180 * incDiff.y * object.zoomScale); | ||
| _changed = true; | ||
| _rotatePrev.copy(_rotateCur); | ||
| } | ||
|
|
||
| /* eslint-disable no-unused-vars */ | ||
| function panstart(event) { |
There was a problem hiding this comment.
Can this be replaced with e.g. function panstart(_event) { ?
There was a problem hiding this comment.
You would think this would be simple, and eslint has an argsIgnorePattern option, but it doesn't work for functions only taking one argument.
The correct way to handle this case is create a custom rule. This implies creating a plugin, which is a node package, and also supplying my .eslintrc (which I prob should supply anyway in the repo, since the linter requires it). I don't actually have node installed atm tho- eslint is provided through atom. So, it's a bit of work :(.
Even as-is, I can reduce the scope of the lint disables by doing the following for each event handler:
/* eslint-disable no-unused-vars */
function panstart(_event) {
/* eslint-enable no-unused-vars */
/* TODO: Dynamically enable pan function? */
_rotateOrig.copy(_rotateCur);
}
Friendly ping--have you had any time to look at this issue? |
|
|
Merged manually, thanks. |
Is this issue still there? I can't fix right now for reasons I discussed, but I'm happy to try once I'm a bit better. |
|
Yeah, still there in master. |
This pull request fixes bugs in the THREE.js viewer, including:
mousemovehandler no longer relies onevent.buttonhaving a valid value (per MDN, it was a happy accident that the handler ever worked).I have verified the fixes work on Windows. Please let me know whether a freshly compiled Solvespace exports the fixed viewer before accepting.