search

annotorious / annotorious-v2-selector-pack

Additional selection tools for Annotorious and the Annotorious OpenSeadragon plugin
BSD 3-Clause "New" or "Revised" License
13 stars 19 forks source link

Console errors when drawing Ellipse #25

Open paulers opened 8 months ago

paulers commented 8 months ago

I read through the other issues, seems like this console error:

Error: <ellipse> attribute ry: Expected length, "undefined".
Ke @ index.js:43
Je @ index.js:43
e @ index.js:43
(anonymous) @ index.js:43
(anonymous) @ index.js:43
(anonymous) @ AnnotationLayer.js:161
index.js:43 Error: <ellipse> attribute ry: Expected length, "undefined".
Ke @ index.js:43
Je @ index.js:43
e @ index.js:43
(anonymous) @ index.js:43
(anonymous) @ index.js:43

Was mentioned in another issue, but not discussed. I dug deeper to see what the problem is, and found that on this line: https://github.com/annotorious/annotorious-v2-selector-pack/blob/bea78d6a7c60beecde20f609089c30dc1b27bc25/src/ellipse/RubberbandEllipse.js#L20 there are only 3 parameters passed to the drawEllipse method, when the method itself expects 4: https://github.com/annotorious/annotorious-v2-selector-pack/blob/bea78d6a7c60beecde20f609089c30dc1b27bc25/src/ellipse/Ellipse.js#L15

This then gets pushed into https://github.com/annotorious/annotorious-v2-selector-pack/blob/bea78d6a7c60beecde20f609089c30dc1b27bc25/src/ellipse/Ellipse.js#L4 which explodes on line 8: https://github.com/annotorious/annotorious-v2-selector-pack/blob/bea78d6a7c60beecde20f609089c30dc1b27bc25/src/ellipse/Ellipse.js#L8

Not sure what the 4th expected value is supposed to be, but figured this would make it easier to debug.

Thanks for looking!

rsimon commented 8 months ago

Oh, wow. Thanks for the detective work! I was vaguely aware of the issue, but never looked into it because it didn't seem to break crucial functionality. (And, also, I admit, because the Ellipse plugin was a 3rd party contribution.)

I made that tiny fix. Didn't publish an updated version of the package yet. But will do soon. For context: the drawEllipse call in line 20 of RubberbandEllipse simply creates an initial tiny ellipse when the user first starts drawing. This initial ellipse is supposed to have an (arbitrarily chosen) radius of 2px, and will then follow the mouse as the user draws. Looks like the initial call only set the x-radius, but should also have set a y-radius.

As I said: fixed now - many thinks for debugging this!