Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for hidden crosshairs + new option + new feature #224

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

tr8dr
Copy link

@tr8dr tr8dr commented Dec 10, 2018

Fix
Crosshairs were hidden beneath any shading in a graph. This was due to the crosshair canvas being below the graph canvas. This was solved by changing the z-index, position type, and allowing events to pass through to lower canvas layers.

New Option
Also added a color option to dyCrosshairs(), to allow users to change the color of the crosshair.

New Feature
Added dyUpdate(session, id, data). This allows one to update an existing dygraph from shiny without rerendering the widget in the DOM. Added functionality in R, JS, and documentation.

- the cross-hair canvas was below the main canvas, such that if shading was applied, would be below the shading and not visible.
- added optional color setting for crosshairs
@przmv
Copy link
Collaborator

przmv commented Dec 10, 2018

LGTM!

Thanks, @tr8dr! Could you please also update the docs and the NEWS file?

@przmv przmv self-assigned this Dec 10, 2018
@tr8dr
Copy link
Author

tr8dr commented Dec 11, 2018

I have adjusted the Rmd and html docs for the new functionality. I did not find a "news" file anywhere in the distribution however. Let me know if there is something additional to be updated here.

@tr8dr tr8dr changed the title Fix for hidden crosshairs + added color option Fix for hidden crosshairs + added color option + in-situ data update from R -> browser Dec 12, 2018
@tr8dr tr8dr changed the title Fix for hidden crosshairs + added color option + in-situ data update from R -> browser Fix for hidden crosshairs + new option + new feature Dec 12, 2018
@tr8dr
Copy link
Author

tr8dr commented Dec 12, 2018

Let me know if you need additional changes. Have added a new feature to push data updates to an existing dygraph in the browser from R.

@przmv
Copy link
Collaborator

przmv commented Jan 11, 2019

Hey, @tr8dr, sorry for the delay. Could you please submit separate pull request for the independent changes? It would make the reviewing process quicker and easier. Thanks!

@tr8dr
Copy link
Author

tr8dr commented Jan 11, 2019

hmm, if you need it this way, I would have to unwind the second set of changes and wait for you to integrate the first set and then do a pull on that integrated set and put those changes back in. Not sure if there is a magical way to do this in github.

@przmv
Copy link
Collaborator

przmv commented Jan 14, 2019

@tr8dr
Copy link
Author

tr8dr commented Jan 15, 2019

Thanks, I will take a look at the above on the weekend ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants