dygraphs
dygraphs copied to clipboard
Fix issue #611 in the drawing demo gallery
This pull request fixes the broken zoom functionality and console errors in the drawing demo. It simply adds the willDestroyContextMyself attribute, and removes the now unimplemented functions in the defaultInteractionModel. Comments have also been added to explain to users why calls are missing.
Sorry about the double commit, still getting a handle on pull request functionality. 13887ef corrects a problem where synchronizer.js would block the users drawCallback when updating the zoom of connected charts.
Coverage decreased (-0.06%) to 90.122% when pulling 13887ef95bb2c28849d56f8e59ddfbc0c55cd23e on mp035:master into da2a028fc41e5573868358b3d9eda9826211d217 on danvk:master.
Is this (and #611 therefore) still pertinent? It doesn’t throw any console warnings for me…
Hi, yep, the bug is still there. Select the zoom functionality in https://dygraphs.com/gallery/#g/drawing with the console open, and try a zoom, or move the mouse over the canvas.
@mp035 thank you for both retesting and the reproducer. I think I get what you’re doing here now.
The unrelated synchroniser change however, do you also have a comment/docs/reproducer/background info on that? I’ve no problem just merging this PR all in one, but I want to make sure I don’t break anything that works, at this point… (Debian is almost frozen, and I want to make a v2.2.1 with all those tiny fixes that arose since v2.2.0 before nothing new can go in any more).
@mirabilos , sorry, I missed your comment. It has been nearly 4 years since I worked on this so, I am a little fuzzy on the details. I think the issue was that the end-user could not rely on their callback being called on every redraw because sometimes synchronizer.js would block for its own purposes and never propagate to the next callback.
~~Looking at my own personal codebase, I have not included it so I would reject 13887ef if I were you.~~
Mark Pointing dixit:
@mirabilos , sorry, I missed your comment. It has been nearly 4 years @since I worked on this so, I am a little fuzzy on the details. I think
Don’t worry, I understand.
@the issue was that the end-user could not rely on their callback being @called on every redraw because sometimes synchronizer.js would block @for its own purposes and never propagate to the next callback. I see
I also think it fixed another bug, crosshairs<->synchroniser interop.
@you have merged it anyway, I just didn't want to leave you hanging.
Yes, thank you!
bye, //mirabilos
Solange man keine schmutzigen Tricks macht, und ich meine wirklich schmutzige Tricks, wie bei einer doppelt verketteten Liste beide Pointer XORen und in nur einem Word speichern, funktioniert Boehm ganz hervorragend. -- Andreas Bogk über boehm-gc in d.a.s.r
Thanks for the update, perhaps that's why I didn't pull 13887ef into my personal codebase. There was a massive amount of cursor customization in my application so I may not have needed it.