daattali / timevis

📅 Create interactive timeline visualizations in R
http://daattali.com/shiny/timevis-demo/
Other
650 stars 157 forks source link

Updated to latest 7.7.0 version of vis-timevis and only fire the <ID>_window signal when the user adjusts the window #128

Closed justacec closed 1 year ago

justacec commented 1 year ago

In this PR I have went ahead and upgraded to the current version of vis-timevis (7.7.0) and have taken advantage of the byUser field of the rangechange(d) event properties. According to the documentation of visjs/timevis, the byUser flag is set to true if the event was fired because the user actually moved and adjusted the window. This better aligns the behavior with the current documentation which states that _window is emitted when the user adjusts the view. I originally read that as it is only fired when the user makes the change. :)

I am still curios about the extra emission of the _window in shiny when the widget is instantiated on lines 62-65 (in this branch) of inst/htmlwidgets/timevis.js. I might still be mis-understanding this code.

justacec commented 1 year ago

This is not ready for merge. I just noticed that it seems that the properties can be null sometimes. In which case the _window should not be fired. I need to fix this up this morning. Will get back to you.

justacec commented 1 year ago

Looks like my concerns was with something else. I fixed that up and this PR does seems to fix the original issue. So, I would say good for review and merge if happy with it.