make plotly axes default to ggplots axes#156
Merged
Merged
Conversation
Contributor
|
I cant find an issue for this and I also cant seem to reproduce the weird axis ticks. I will test nevertheless with jaspDescriptives |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates convertGgplotToPlotly() so Plotly axes initially keep ggplot2’s explicit tick specification (tickmode array), while switching to Plotly auto ticks only during pan/zoom interactions and restoring the original ticks on axis reset. It also adjusts the rangeframe Plotly-side logic and updates tests/snapshots affected by the rendering changes.
Changes:
- Add an
adaptive_ggplotly_ticks()helper that injects JS to toggle axis tick behavior between ggplot-style ticks and Plotly auto ticks based on relayout events. - Rework the rangeframe JS so it aligns rangeframe extents to the rendered ticks (including after resets that restore ggplot tick arrays).
- Update PlotPieChart tests and refresh vdiffr snapshots impacted by the plotting changes.
Reviewed changes
Copilot reviewed 3 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/testthat/test-PlotPieChart.R | Updates the non-polar test path and makes the “many slices” input deterministic; adds a macOS skip for that case. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-sequential.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-points-main.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-points-legend.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-evidence-main.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-bf-main.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-arrow-plain.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotRobustnessSequential/plotrobustnesssequential-arrow-parse.svg | Updated snapshot after rendering changes. |
| tests/testthat/_snaps/PlotPieChart/plotpiechart-polar-no-ticks-no-legend.svg | Updated snapshot after PlotPieChart output changes. |
| tests/testthat/_snaps/PlotPieChart/plotpiechart-polar-default.svg | Updated snapshot after PlotPieChart output changes. |
| R/PlotPieChart.R | Adjusts documentation and deprecation warning text around polarAxis. |
| R/convertGgplotToPlotly.R | Adds adaptive tick JS injection and updates rangeframe JS to use rendered tick extents. |
| man/plotPieChart.Rd | Keeps generated Rd text consistent with the updated roxygen docs. |
Files not reviewed (1)
- man/plotPieChart.Rd: Generated file
Comment on lines
+539
to
+542
| function approximatelyEqual(a, b) { | ||
| const scale = Math.max(1, Math.abs(a), Math.abs(b)); | ||
| return Math.abs(a - b) <= 1e-9 * scale; | ||
| } |
Comment on lines
+121
to
+125
| for (ax in xaxes) | ||
| plotlyplotje <- adaptive_ggplotly_ticks(plotlyplotje, ax) | ||
|
|
||
| if (grepl("l", sides)) | ||
| for (ax in yaxes) | ||
| plotlyplotje$x$layout[[ax]]$tickmode <- "auto" | ||
| for (ax in yaxes) | ||
| plotlyplotje <- adaptive_ggplotly_ticks(plotlyplotje, ax) |
Comment on lines
+26
to
+28
| # set.seed(1) | ||
| # value_many <- rpois(25, 10) | ||
| # hardcoded output of rpois above after test failures on macos only |
juliuspfadt
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the issue we discussed over Mattermost (is there an issue for this?)
Reprex in R
The code is not very pretty, but the idea is:
array(ggplot2) default).This also required some changes to the rangeframe code.
According to chatgpt, there is no R way of doing this.