-
Notifications
You must be signed in to change notification settings - Fork 270
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
make diagonalNetwork and radialNetwork responsive (sort-of) #83
make diagonalNetwork and radialNetwork responsive (sort-of) #83
Conversation
… with backward support for original
I really like it and don't think there is any need in principle to dedicate an argument to this. One issue I just spotted and haven't had a chance to look into: If you knit the gh-pages .Rmd file you will see that it produces strange and undesirable behaviour. |
Thanks so much for the feedback and bug discovery. I'll work out the kinks and hopefully have a working pull momentarily. |
…alculation relative to svg node
I believe I have worked out the kinks in 1a9d8f5. @christophergandrud, do you mind verifying? |
Nice, merging into master now. |
oh great, what else for CRAN? anything you would like to implement? I'll keep going through the existing pulls. |
Generally, I'm a believer in relatively incremental CRAN updates to mimimise the risk of unanticipated code breaks at any given update. So I'm fine. But if you think there is an important pull to merge, then lets do that. |
#85 reminded me that the |
I'm also ready if you are. I'll make compile it now and send it up to CRAN. |
. . . submitted v0.2.4 |
This pull is a little more intrusive on the current behavior of
diagonalNetwork
andradialNetwork
, but I think it wisely lets theviewBox
attribute ofSVG
handle sizing and resizing. It seems theresize
method already is a little broken, so I don't think this causes any more damage. Using this approach also insures that the diagram fits within thehtmlwidget
containerdiv
, so aR
user does not need to fiddle withmargins
unless desired.At a minimum, I think this should exist as an argument to toggle on or off, but I'm very comfortable making this the default behavior especially given @christophergandrud focus on simplicity.