Add option suppress_default_hovertext for plot_method = "ggplot" #301
Conversation
| @@ -887,6 +890,7 @@ heatmaply.heatmapr <- function(x, | |||
| point_size_name = "Point size", | |||
| label_format_fun = function(...) format(..., digits = 4), | |||
| custom_hovertext = x[["matrix"]][["custom_hovertext"]], | |||
| suppress_default_hovertext = F, | |||
| col, ": ", mdf[[2]], "<br>", | ||
| val, ": ", label_format_fun(mdf[[3]]) | ||
| ) | ||
| if(!suppress_default_hovertext) |
There was a problem hiding this comment.
Use the same brace style as the rest of the code
| @@ -95,7 +101,13 @@ ggplot_heatmap <- function(xx, | |||
| } | |||
| } | |||
| if (!is.null(custom_hovertext)) { | |||
| mdf[["text"]] <- paste0(mdf[["text"]], "<br>", custom_hovertext) | |||
| if(!suppress_default_hovertext) | |||
There was a problem hiding this comment.
Invert this if/else
|
@alanocallaghan at least with v1.5, when using custom_hovertext with the plot_method = plotly method the default is not shown. |
|
What I mean is that the behavior should be equivalent |
|
This currently fails with: heatmaply(mtcars, suppress_default_hovertext=TRUE)
Error in ggplot_heatmap(data_mat, row_text_angle, column_text_angle, scale_fill_gradient_fun, :
object 'suppress_default_hovertext' not foundIf I add the arg to ggplot_heatmap, I get: heatmaply(mtcars, suppress_default_hovertext=TRUE)
Error in `.data$text`:
! Column `text` not found in `.data`.
Run `rlang::last_trace()` to see where the error occurred.Also hovertext is currently broken generally for plot_method="plotly" so I'd want to resolve that before merging |
|
Thanks @alanocallaghan. I added the arg to my fork and it works for me if I install heatmaply from it. Could there be other changes after merging? |
|
Please also add yourself as contributor to the DESCRIPTION file.
T
…On Sun, 24 Dec 2023, 20:07 mcsimenc, ***@***.***> wrote:
Thanks @alanocallaghan <https://github.com/alanocallaghan>. I added the
arg to my fork and it works for me if I install heatmaply from it. Could
there be other changes after merging?
—
Reply to this email directly, view it on GitHub
<#301 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHOJBVPA2J5GWVFFYSLPXTYLBVNNAVCNFSM6AAAAABBALV3YWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRYGU3DSNBXGU>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
Hi @mcsimenc |
|
Hey guys, I'm so sorry about the lag. I've been so busy with work and home life and haven't returned to this. They are simple changes and I will try to get to it this weekend! |
|
No worries, we all get it :)
Thanks 👍🙏
…On Mon, 20 May 2024, 22:13 mcsimenc, ***@***.***> wrote:
Hey guys, I'm so sorry about the lag. I've been so busy with work and home
life and haven't returned to this. They are simple changes and I will try
to get to it soon!
—
Reply to this email directly, view it on GitHub
<#301 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHOJBR2AB52W7DTT4WW7QTZDJRN7AVCNFSM6AAAAABBALV3YWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRRGIYTOMZZGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
closes #300