Skip to content

Commit 35e5d61

Browse files
committed
Clean up redundant comments in ggplotly fixes
1 parent 4ecd29e commit 35e5d61

File tree

2 files changed

+6
-26
lines changed

2 files changed

+6
-26
lines changed

R/ggplotly.R

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,6 @@ gg2list <- function(p, width = NULL, height = NULL,
411411
# of each non-positional scale for display in tooltips
412412
for (sc in npscales$scales) {
413413
data <- lapply(data, function(d) {
414-
# Only process aesthetics that actually exist in this layer's data
415414
present_aes <- intersect(sc$aesthetics, names(d))
416415
if (length(present_aes) > 0) {
417416
d[paste0(present_aes, "_plotlyDomain")] <- d[present_aes]
@@ -573,12 +572,9 @@ gg2list <- function(p, width = NULL, height = NULL,
573572
tr$hoverinfo <- tr$hoverinfo %||%"text"
574573
tr
575574
})
576-
# show only one legend entry per legendgroup
577-
# For deduplication, skip invisible traces (like GeomBlank) so they don't
578-
# "claim" the legendgroup and prevent visible traces from showing in legend
575+
# show only one legend entry per legendgroup (skip invisible traces for dedup)
579576
grps <- sapply(traces, "[[", "legendgroup")
580577
is_visible <- sapply(traces, function(tr) !isFALSE(tr$visible))
581-
# Only consider visible traces for deduplication - invisible traces shouldn't claim legendgroup
582578
grps_for_dedup <- ifelse(is_visible, grps, paste0(grps, "_invisible_", seq_along(grps)))
583579
traces <- Map(function(x, y) {
584580
if (!is.null(x[["frame"]])) return(x)
@@ -990,7 +986,7 @@ gg2list <- function(p, width = NULL, height = NULL,
990986
font = text2font(theme$legend.text)
991987
)
992988

993-
# Translate legend.position from ggplot2 theme to plotly layout (fixes #2407, #2187)
989+
# Translate legend.position to plotly layout
994990
legend_pos <- theme$legend.position %||% theme[["legend.position"]]
995991
if (!is.null(legend_pos) && !identical(legend_pos, "none")) {
996992
if (is.character(legend_pos)) {
@@ -1005,7 +1001,6 @@ gg2list <- function(p, width = NULL, height = NULL,
10051001
x = -0.15, y = 0.5, xanchor = "right", yanchor = "middle"
10061002
)),
10071003
"inside" = {
1008-
# In ggplot2 >= 3.5.0, numeric position is stored in legend.position.inside
10091004
inside_pos <- theme$legend.position.inside %||% theme[["legend.position.inside"]]
10101005
if (is.numeric(inside_pos) && length(inside_pos) == 2) {
10111006
modifyList(gglayout$legend, list(
@@ -1015,10 +1010,9 @@ gg2list <- function(p, width = NULL, height = NULL,
10151010
gglayout$legend
10161011
}
10171012
},
1018-
gglayout$legend # "right" is default, no change needed
1013+
gglayout$legend
10191014
)
10201015
} else if (is.numeric(legend_pos) && length(legend_pos) == 2) {
1021-
# Handle numeric position like c(0.8, 0.2) for older ggplot2 versions
10221016
gglayout$legend <- modifyList(gglayout$legend, list(
10231017
x = legend_pos[1], y = legend_pos[2], xanchor = "left", yanchor = "bottom"
10241018
))
@@ -1431,7 +1425,6 @@ make_strip_rect <- function(xdom, ydom, theme, side = "top") {
14311425
# theme(panel.border) -> plotly.js rect shape
14321426
make_panel_border <- function(xdom, ydom, theme) {
14331427
border <- theme[["panel.border"]]
1434-
# Don't draw anything if panel.border is blank or NULL
14351428
if (is.null(border) || is_blank(border)) {
14361429
return(list())
14371430
}

R/layers2traces.R

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,7 @@ layers2traces <- function(data, prestats_data, layout, p) {
5656
if (!aesName %in% names(x)) next
5757
# TODO: should we be getting the name from scale_*(name) first?
5858
varName <- y[[i]]
59-
# Skip auto-generated group aesthetic (where both aes name and var name are "group"),
60-
# but allow: (1) variables named "group" mapped to other aesthetics like colour,
61-
# and (2) other variables mapped to the "group" aesthetic
59+
# Skip auto-generated group aesthetic, but keep explicit group mappings
6260
if (identical(aesName, "group") && identical(varName, "group")) next
6361
# add a line break if hovertext already exists
6462
if ("hovertext" %in% names(x)) x$hovertext <- paste0(x$hovertext, br())
@@ -77,11 +75,7 @@ layers2traces <- function(data, prestats_data, layout, p) {
7775
x
7876
}, data, hoverTextAes)
7977

80-
# draw legends only for discrete scales
81-
# Register each aesthetic separately for proper legend matching (fixes #2467)
82-
# When a scale has multiple aesthetics (e.g., c("colour", "fill")), we need
83-
# individual entries so "colour_plotlyDomain" matches discreteScales[["colour"]]
84-
# Skip identity scales (guide = "none") as they don't produce legends or trace splitting
78+
# draw legends only for discrete scales (skip scales with guide = "none")
8579
discreteScales <- list()
8680
for (sc in p$scales$non_position_scales()$scales) {
8781
if (sc$is_discrete() && !identical(sc$guide, "none")) {
@@ -713,8 +707,6 @@ geom2trace <- function(data, params, p) {
713707

714708
#' @export
715709
geom2trace.GeomBlank <- function(data, params, p) {
716-
# GeomBlank should never show in legend - it would claim the legendgroup
717-
# and cause visible traces with the same group to not show their legend entry
718710
list(visible = FALSE, showlegend = FALSE)
719711
}
720712

@@ -879,11 +871,7 @@ geom2trace.GeomBoxplot <- function(data, params, p) {
879871
# marker styling must inherit from GeomPoint$default_aes
880872
# https://github.com/hadley/ggplot2/blob/ab42c2ca81458b0cf78e3ba47ed5db21f4d0fc30/NEWS#L73-L7
881873
point_defaults <- GeomPoint$use_defaults(NULL)
882-
883-
# Determine if outliers should be hidden
884-
# Hide outliers if: outliers = FALSE, or outlier.shape = NA (via outlier_gp$shape)
885-
hide_outliers <- isFALSE(params$outliers) ||
886-
isTRUE(is.na(params$outlier_gp$shape))
874+
hide_outliers <- isFALSE(params$outliers) || isTRUE(is.na(params$outlier_gp$shape))
887875

888876
compact(list(
889877
x = data[["x"]],
@@ -898,7 +886,6 @@ geom2trace.GeomBoxplot <- function(data, params, p) {
898886
aes2plotly(data, params, "fill"),
899887
aes2plotly(data, params, "alpha")
900888
),
901-
# Control whether outlier points are shown
902889
boxpoints = if (hide_outliers) FALSE,
903890
# markers/points
904891
marker = list(

0 commit comments

Comments
 (0)