-
Notifications
You must be signed in to change notification settings - Fork 632
Add support for ggridges #2314
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
Add support for ggridges #2314
Conversation
Wow, incredible work, thank you! I'm going to add some nit-picky comments, but overall this is looking great
I wouldn't consider this a blocking issue, and I'm not sure if this is sensible to address here, but just throwing it out there that it'd be awesome if this worked better (more specifically, it seems highlight()'s opacityDim isn't having an effect on the non-highlighted ridgelines?)
p <- ggplot(iris |> highlight_key(~Species), aes(x = Sepal.Length, y = Species)) + geom_density_ridges(rel_min_height = 0.01) ggplotly(p) |> highlight("plotly_hover", opacityDim = 0.2)
I wouldn't consider this a blocking issue, and I'm not sure if this is sensible to address here, but just throwing it out there that it'd be awesome if this worked better (more specifically, it seems
highlight()'sopacityDimisn't having an effect on the non-highlighted ridgelines?)p <- ggplot(iris |> highlight_key(~Species), aes(x = Sepal.Length, y = Species)) + geom_density_ridges(rel_min_height = 0.01) ggplotly(p) |> highlight("plotly_hover", opacityDim = 0.2)
Didn't know about highlight. Any idea what could be affecting it? Or where I could look to debug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these need be doubled, like you did for x/y (this might be leading to #2314 (comment))
I'd consider doing something like (using with() here can be kinda dangerous since it'll error if any of the columns are missing
positions <- rbind(data, rev(data)) positions$y <- c(data$ymax, rev(data$ymin))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found the issue! Within the new to_basics, I was using to_basic.GeomDensity and to_basic.GeomLine which removed the crosstalk set. Attaching them back again similar to how it's done in layers2traces solves the highlight not working.
Before this, the to_basic.GeomLine would preserve the set attribute, but to_basic.GeomDensity would not, so we were getting highlight working only the line part and not density/area part.
I have tested the highlight functionality with different types of ridge plots, with different fills, points, lines within them, and they are all working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpsievert are there any pending action items on this, or is this good to go now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for letting this slip @AdroMine, looking good, would you mind just adding a NEWS.md item?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpsievert , added a line about adding support for ggridges package.
This comment was marked as off-topic.
This comment was marked as off-topic.
3297f8d to
a329f43
Compare
Awesome work, thank you!
Closes #1912
Add support for ggrides package
I took inspiration from the #1087 and have a version that supports most of the ggridges functionality. The test file has all the examples from ggridges introduction vignette
What's working:
What's not working:
ggridge functions that already working without new code:
Examples:
image
ggplotly(p)image
image
ggplotly(p2)image