Skip to content

Discrepancy between geom_point2 and geom_point_borderless (and similar)#435

Merged
strengejacke merged 5 commits into
mainfrom
strengejacke/issue395
May 22, 2026
Merged

Discrepancy between geom_point2 and geom_point_borderless (and similar)#435
strengejacke merged 5 commits into
mainfrom
strengejacke/issue395

Conversation

@strengejacke

Copy link
Copy Markdown
Member

Fixes #395

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces new 'halo' geoms, such as geom_point_halo and geom_jitter_halo, which provide a visual separation for overlapping points by adding a contour that matches the plot's background color. Additionally, it refactors *_borderless geoms as aliases and includes bug fixes for their rendering. Review feedback highlights an inaccuracy in the documentation regarding color aesthetics for the refactored geoms, a typo in the news file, and potential issues in the internal halo logic related to shape conflicts and background color detection.

Comment thread R/geom_point2.R Outdated
Comment thread NEWS.md

## Bug fixes

* Fixed the `*_borderless` aliases for `geom_point2()` or `geom_jitter2()`, which

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Typo in the function name alias: *_borless should be *_borderless.

Suggested change
* Fixed the `*_borderless` aliases for `geom_point2()` or `geom_jitter2()`, which
* Fixed the `*_borderless` aliases for `geom_point2()` or `geom_jitter2()`, which

Comment thread R/geom_point2.R Outdated
Comment on lines +134 to +137
dots$colour <- NULL
dots$color <- .get_theme_bg_color()

fun_args <- c(list(pch = 21), dots)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are two improvements possible here:

  1. Shape Conflict: If a user passes shape or pch in ..., it will conflict with the hardcoded pch = 21. It is safer to explicitly remove these from dots to ensure the halo effect (which requires a shape with a border and fill) is preserved.
  2. Background Color Logic: The .get_theme_bg_color() function currently retrieves the grid line color (panel.grid.major$colour) rather than the actual background color (panel.background$fill). This might result in a halo that doesn't blend with the background if they differ (e.g., grey grid lines on a white background).
  dots$colour <- dots$shape <- dots$pch <- NULL
  dots$color <- .get_theme_bg_color()

  fun_args <- c(list(pch = 21), dots)

@strengejacke strengejacke merged commit 4c246a7 into main May 22, 2026
13 of 27 checks passed
@strengejacke strengejacke deleted the strengejacke/issue395 branch May 22, 2026 07:25
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.

Discrepancy between geom_point2 and geom_point_borderless (and similar)

1 participant