From 226b5826b7d4ee836412782e8d0db53e129440c6 Mon Sep 17 00:00:00 2001 From: Justin Yap Date: Wed, 19 Aug 2026 15:09:32 +1000 Subject: [PATCH 1/4] RS-23150: Keep category axis labels next to the axis in PPT exports getPPTSettings hardcoded LabelPosition = "Low" on the primary axis for every chart type except Pie and Donut. That exports as , which puts the tick labels at the low end of the crossing axis rather than beside the axis they belong to, leaving a gap between the labels and the bars that Displayr does not render. "NextTo" is what Q already defaults a category axis to, but it cannot be applied unconditionally: the category axis crosses at zero for every non-scatter type, so anything reaching below zero would run over the labels. "Low" was added deliberately in abd1845 to prevent that. So use "NextTo" only where nothing can be drawn below the axis, and keep "Low" otherwise, as Q does for its own negative stacked plots. Deciding that means allowing for several ways values reach below the axis without appearing in the data this sees: - the user's axis minimum, read as the chart reads it, since charToNumeric strips spaces and thousands separators and treats NA, "" and unparseable text as unset; - scatter and bubble floors, which setScatterAxesBounds pads below zero later, well after this runs; - StackedColumnWithStatisticalSignificance, which rejects negative input and negates the first n columns itself; - multi-statistic tables, where only the first plane is plotted and the significance planes must not decide the question. Every test errs towards "Low" when it cannot tell, since that is the behaviour this replaces and it cannot cause the overlap. Co-Authored-By: Claude Opus 5 (1M context) --- DESCRIPTION | 2 +- R/cchart.R | 51 +++++++++++++- tests/testthat/test-chartsettings.R | 102 +++++++++++++++++++++++++++- 3 files changed, 151 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index bf8d67e..c654afe 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: flipChart Type: Package Title: Single function for calling charts - CChart -Version: 1.12.15 +Version: 1.12.16 Author: Displayr Maintainer: Displayr Description: Wrapper for other chart functions, such that they can be access via a diff --git a/R/cchart.R b/R/cchart.R index c75d9fe..ca3b245 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -939,6 +939,9 @@ getPPTSettings <- function(chart.type, args, data) categories.axis.line <- if (!isTRUE(args$values.zero.line.width > 0)) list(width = args$categories.line.width, color = args$categories.line.color, crosses = default.cross) else list(width = args$values.zero.line.width, color = args$values.zero.line.color, dash = args$values.zero.line.dash, crosses = "AutoZero") + category.label.position <- categoryAxisLabelPosition(chart.type, args, data, + categories.axis.line$crosses) + res$PrimaryAxis = list(LabelsFont = list(color = args$categories.tick.font.color, family = args$categories.tick.font.family, size = px2pt(args$categories.tick.font.size)), @@ -954,7 +957,7 @@ getPPTSettings <- function(chart.type, args, data) MajorGridLine = list(Color = args$categories.grid.color, Width = px2pt(args$categories.grid.width), Style = getGridLineStyle(args$categories.grid.width, args$categories.grid.dash)), - LabelPosition = "Low") + LabelPosition = category.label.position) if (any(nzchar(args$categories.bounds.maximum))) res$PrimaryAxis$Maximum <- args$categories.bounds.maximum if (any(nzchar(args$categories.bounds.minimum))) @@ -1068,6 +1071,52 @@ getPPTSettings <- function(chart.type, args, data) } +# Where the category axis tick labels go in a PowerPoint export. +# +# PowerPoint draws "NextTo" labels against the axis line, which sits at value 0 whenever the axis +# crosses AutoZero, so anything reaching below zero runs over them. "Low" pins them to the low end +# instead, as Q does for its negative stacked plots (PptChartSettingsAndLabels). Values can reach +# below the axis without appearing in the data, so each test below errs towards "Low" when it +# cannot tell: that is the behaviour this replaced, and it cannot cause the overlap. +categoryAxisLabelPosition <- function(chart.type, args, data, crosses) +{ + # Anywhere else the axis line is already at the low end, so the two agree. + if (!identical(crosses, "AutoZero")) + return("NextTo") + + # data can be a list - grouped distribution charts, link-to-multiple-tables inputs - so + # flatten it rather than coerce. Dates go first; they unlist to day counts. + plotted <- if (inherits(data, c("Date", "POSIXt"))) NULL + else if (is.data.frame(data)) data[!vapply(data, inherits, logical(1), what = c("Date", "POSIXt"))] + else data + # A multi-statistic QTable is one plane per statistic and only the first is plotted, so + # significance planes must not decide this. removeSignifAndCharData picks the same plane, but + # not until well after this, and only it sets the statistic attribute. + if (length(dim(plotted)) == 3) + { + primary.statistic <- attr(plotted, "statistic") + plotted <- if (length(primary.statistic) == 1 && primary.statistic %in% dimnames(plotted)[[3]]) + plotted[, , primary.statistic] + else plotted[, , 1] + } + plotted.values <- suppressWarnings(as.numeric(unlist(plotted, use.names = FALSE))) + + # Read the bound as the chart does - charToNumeric strips spaces and thousands separators, and + # treats NA, "" and unparseable text as unset. Test the parsed number, never the text: a blank + # box arrives as NA, and nzchar(NA) is TRUE, so text would read as a floor. + values.minimum <- suppressWarnings(as.numeric(gsub("[ ,]", "", as.character(args$values.bounds.minimum)))) + # setScatterAxesBounds runs after this and pads the floor below zero for plenty of positive Y + # ranges - [5, 95] gives -10 - so an unpinned scatter floor could be anywhere. + scatter.floor.unknown <- isScatter(chart.type) && !isTRUE(values.minimum >= 0) + # StackedColumnWithStatisticalSignificance negates the first n columns itself, so those values + # sit below the axis without ever appearing in the data seen here. + categories.below.axis <- isTRUE(args$num.categories.below.axis > 0) + + plots.below.axis <- isTRUE(values.minimum < 0) || scatter.floor.unknown || categories.below.axis || + (any(plotted.values < 0, na.rm = TRUE) && !isTRUE(values.minimum >= 0)) + if (plots.below.axis) "Low" else "NextTo" +} + getLineStyle <- function (line) { if (is.null(line$width) || line$width <= 0) return ("None") diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index 46d2b5e..3122f82 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -64,7 +64,7 @@ test_that("Chart settings", NumberFormat = "General", AxisLine = list(Color = "#0000FF", Width = 1.5, Style = "Solid"), Crosses = "AutoZero", MajorGridLine = list(Color = "#BBBBBB", - Width = 0, Style = "None"), LabelPosition = "Low")) + Width = 0, Style = "None"), LabelPosition = "NextTo")) expect_equal(attr(res, "ChartSettings")$ValueAxis, list( LabelsFont = list(color = NULL, family = NULL, size = numeric(0)), ShowTitle = FALSE, @@ -101,7 +101,7 @@ test_that("Chart settings", NumberFormat = "General", AxisLine = list(Color = "#222222", Width = 1.5, Style = "Solid"), Crosses = "AutoZero", MajorGridLine = list(Color = "#BBBBBB", - Width = 0, Style = "None"), LabelPosition = "Low")) + Width = 0, Style = "None"), LabelPosition = "NextTo")) expect_equal(attr(res, "ChartSettings")$ValueAxis, list( LabelsFont = list(color = NULL, family = NULL, size = numeric(0)), ShowTitle = FALSE, @@ -529,3 +529,101 @@ test_that("Every plotly family PowerPoint has a style for is mapped", { "y-up", "line-ew", "arrow-up")), rep("Circle", 7)) }) + +test_that("Category axis labels drop to the low end when the plot goes below the axis", +{ + # The category axis crosses at zero, so "NextTo" labels would sit on top of anything + # drawn below it. dat.2d is rnorm and straddles zero; abs() of it does not. + negative <- CChart("Column", dat.2d, append.data = TRUE, colors = col.2d) + expect_equal(attr(negative, "ChartSettings")$PrimaryAxis$Crosses, "AutoZero") + expect_equal(attr(negative, "ChartSettings")$PrimaryAxis$LabelPosition, "Low") + + positive <- CChart("Column", abs(dat.2d), append.data = TRUE, colors = col.2d) + expect_equal(attr(positive, "ChartSettings")$PrimaryAxis$LabelPosition, "NextTo") + + # A user-set minimum below zero drops the plot below the axis even when the data doesn't. + stretched <- CChart("Column", abs(dat.2d), append.data = TRUE, colors = col.2d, + values.bounds.minimum = -3) + expect_equal(attr(stretched, "ChartSettings")$PrimaryAxis$LabelPosition, "Low") + + # ... and a minimum at or above zero clips the plot there, so negatives never reach below it. + clipped <- CChart("Column", dat.2d, append.data = TRUE, colors = col.2d, + values.bounds.minimum = 0) + expect_equal(attr(clipped, "ChartSettings")$PrimaryAxis$LabelPosition, "NextTo") + + # Distribution charts split by a group hand getPPTSettings a list of unequal-length vectors, + # which as.matrix turns into a list-matrix that as.numeric cannot coerce - an error, not a + # warning, so it would take CChart down with it rather than just the export settings. + grouped <- CChart("Histogram", list(x = 1:10, y = c(-1, 2, 3)), append.data = TRUE) + expect_equal(attr(grouped, "ChartSettings")$PrimaryAxis$LabelPosition, "Low") + + # The bound is read the way the chart reads it, so a thousands separator still counts as a + # floor below the axis. Plain as.numeric would give NA here and leave the labels in the plot. + separated <- CChart("Column", abs(dat.2d), append.data = TRUE, colors = col.2d, + values.bounds.minimum = "-5,000") + expect_equal(attr(separated, "ChartSettings")$PrimaryAxis$LabelPosition, "Low") + + # A blank bound box arrives as NA, and nzchar(NA) is TRUE, so testing the text rather than the + # parsed number would treat every blank bound as a floor and leave the labels at the low end. + blank <- CChart("Column", abs(dat.2d), append.data = TRUE, colors = col.2d, + values.bounds.minimum = NA) + expect_equal(attr(blank, "ChartSettings")$PrimaryAxis$LabelPosition, "NextTo") + + # The chart discards a bound it cannot parse and ranges from the data, so treat it as not set. + unreadable <- CChart("Column", abs(dat.2d), append.data = TRUE, colors = col.2d, + values.bounds.minimum = "not a number") + expect_equal(attr(unreadable, "ChartSettings")$PrimaryAxis$LabelPosition, "NextTo") + + + # StackedColumnWithStatisticalSignificance requires all-positive input and negates the first n + # columns itself, so the values that end up below the axis are never visible in the data the + # axis guard inspects - only in the ChartData the chart function exports. Asserting on that + # ChartData is what ties the two together: if the argument stopped reaching the chart, the + # negatives would disappear and this would fail rather than quietly still passing. + below.axis <- CChart("StackedColumnWithStatisticalSignificance", abs(dat.2d), + append.data = TRUE, num.categories.below.axis = 2) + expect_true(any(attr(below.axis, "ChartData") < 0, na.rm = TRUE)) + expect_equal(attr(below.axis, "ChartSettings")$PrimaryAxis$LabelPosition, "Low") + + none.below.axis <- CChart("StackedColumnWithStatisticalSignificance", abs(dat.2d), + append.data = TRUE, num.categories.below.axis = 0) + expect_false(any(attr(none.below.axis, "ChartData") < 0, na.rm = TRUE)) + expect_equal(attr(none.below.axis, "ChartSettings")$PrimaryAxis$LabelPosition, "NextTo") +}) + +test_that("Category axis label position handles the shapes data arrives in", +{ + position <- function(data, minimum = NULL, chart.type = "Column", below = NULL) + flipChart:::categoryAxisLabelPosition(chart.type, + list(values.bounds.minimum = minimum, num.categories.below.axis = below), + data, "AutoZero") + + # A list of unequal-length vectors, as grouped distribution charts supply. + expect_equal(position(list(x = 1:10, y = c(-1, 2, 3))), "Low") + expect_equal(position(list(x = 1:10, y = c(1, 2, 3))), "NextTo") + + # Dates unlist to day counts, negative before 1970, and say nothing about the value axis. + expect_equal(position(data.frame(d = as.Date(c("1960-01-01", "1985-06-01")), v = c(1, 2))), "NextTo") + expect_equal(position(as.Date("1960-01-01")), "NextTo") + + # Only the first plane of a multi-statistic table is plotted; z-Statistic is not. + stats <- array(c(10, 20, 30, 40, -1.5, 2, -0.3, 1.1), dim = c(2, 2, 2), + dimnames = list(c("r1", "r2"), c("c1", "c2"), c("Column %", "z-Statistic"))) + expect_equal(position(stats), "NextTo") + expect_equal(position(structure(stats, statistic = "z-Statistic")), "Low") + expect_equal(position(structure(stats, statistic = c("two", "names"))), "NextTo") + + # The bound is read as the chart reads it, and anything it cannot read counts as unset. + expect_equal(position(matrix(1:6, 2), "-5,000"), "Low") + expect_equal(position(matrix(1:6, 2), "5 000"), "NextTo") + expect_equal(position(matrix(1:6, 2), NA), "NextTo") + expect_equal(position(matrix(1:6, 2), "abc"), "NextTo") + expect_equal(position(matrix(c(1, -2, 3, 4, 5, 6), 2), 0), "NextTo") + + # An unpinned scatter floor is padded below zero later, so it cannot be ruled out. + expect_equal(position(data.frame(x = c(1, 2), y = c(5, 95)), NA, "Scatter"), "Low") + expect_equal(position(data.frame(x = c(1, 2), y = c(5, 95)), 0, "Scatter"), "NextTo") + + # An axis the chart draws elsewhere is already at the low end, so the two agree. + expect_equal(flipChart:::categoryAxisLabelPosition("Column", list(), matrix(c(-1, 2), 1), "Minimum"), "NextTo") +}) From 598508ae8f48038d46d60f6cba14fdf85b930fe7 Mon Sep 17 00:00:00 2001 From: Justin Yap Date: Thu, 20 Aug 2026 10:07:21 +1000 Subject: [PATCH 2/4] Untracked: Move the CircleCI flipStandardCharts pin to a tag that exists BuildAndCheckPackage has failed to resolve its dependencies since 139730f pinned flipStandardCharts at 1.32.15, which was never tagged - the tags are 1.31.6, 1.32.6, 1.32.12 and 1.32.18 - so pak cannot find the ref. Nothing about this branch causes it; the GitHub Actions build stays green because it resolves through nixr-public rather than pak. 1.32.18 is the current tag and is what the DESCRIPTION floor of >= 1.32.15 wants. Dropping the pins altogether was the first thing tried, on the grounds that the nightly executor already carries current versions, but that sent pak off building every dependency from source and the step timed out. Suggested by Carmen on the PR. --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3a47253..36a54c4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,7 +7,7 @@ parameters: default: "" remote-deps: type: string - default: Displayr/rhtmlCombinedScatter@1.2.2,Displayr/flipStandardCharts@1.32.15 + default: Displayr/rhtmlCombinedScatter@1.2.2,Displayr/flipStandardCharts@1.32.18 plugins-branch: type: string default: "" From 4f2b8a3ede5fdfb752b180604b18577dd325e478 Mon Sep 17 00:00:00 2001 From: Justin Yap Date: Thu, 20 Aug 2026 12:20:28 +1000 Subject: [PATCH 3/4] Update R/cchart.R Co-authored-by: Carmen Chan --- R/cchart.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/cchart.R b/R/cchart.R index ca3b245..1709e42 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -1103,7 +1103,9 @@ categoryAxisLabelPosition <- function(chart.type, args, data, crosses) # Read the bound as the chart does - charToNumeric strips spaces and thousands separators, and # treats NA, "" and unparseable text as unset. Test the parsed number, never the text: a blank - # box arrives as NA, and nzchar(NA) is TRUE, so text would read as a floor. + # box arrives as NA, and nzchar(NA) is TRUE, so text would read as a floor. The arg is a single + # text box (NULL or length 1), so isTRUE() on the parsed number is enough - the any(nzchar()) + # reads in getPPTSettings() guard length 0, not longer vectors. values.minimum <- suppressWarnings(as.numeric(gsub("[ ,]", "", as.character(args$values.bounds.minimum)))) # setScatterAxesBounds runs after this and pads the floor below zero for plenty of positive Y # ranges - [5, 95] gives -10 - so an unpinned scatter floor could be anywhere. From 20749a2c611b4761d83e862505c1422b5a742dd0 Mon Sep 17 00:00:00 2001 From: Justin Yap Date: Thu, 20 Aug 2026 12:23:05 +1000 Subject: [PATCH 4/4] RS-23150: Read the plotted plane directly, and say where the invariant lives Two things from review. The multi-statistic branch preferred whichever plane the statistic attribute named, falling back to the first. Nothing sets that attribute on the arrays that reach here - addStatTesting clears it after building one, and ConvertQTableToArray only converts 2-D input and never assigns it - so the preference was unreachable, and under the rule that only the first plane is drawn it would have read the wrong plane had it ever fired. Read plane 1 and drop the two tests that only passed by synthesising the attribute the pipeline strips. The comment above it now states why plane 1 is the right one, rather than pointing at removeSignifAndCharData, which answers a different question and keeps every surviving plane in two of its three paths. Also note against the bound parse that the argument is a single text box, which is what makes isTRUE() sufficient - that invariant lives in the plugin layer, where all eight call sites pass get0("formValuesMin"). Co-Authored-By: Claude Opus 5 (1M context) --- R/cchart.R | 14 ++++++-------- tests/testthat/test-chartsettings.R | 2 -- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/R/cchart.R b/R/cchart.R index 1709e42..df9b1bf 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -1089,15 +1089,13 @@ categoryAxisLabelPosition <- function(chart.type, args, data, crosses) plotted <- if (inherits(data, c("Date", "POSIXt"))) NULL else if (is.data.frame(data)) data[!vapply(data, inherits, logical(1), what = c("Date", "POSIXt"))] else data - # A multi-statistic QTable is one plane per statistic and only the first is plotted, so - # significance planes must not decide this. removeSignifAndCharData picks the same plane, but - # not until well after this, and only it sets the statistic attribute. + # Only the primary (first) plane is drawn as bars/markers - flipStandardCharts uses the later + # planes for annotations - so nothing outside plane 1 can reach below the axis. The later + # planes are still exported (removeSignifAndCharData keeps them for those annotations), so + # read plane 1 rather than the whole array. if (length(dim(plotted)) == 3) { - primary.statistic <- attr(plotted, "statistic") - plotted <- if (length(primary.statistic) == 1 && primary.statistic %in% dimnames(plotted)[[3]]) - plotted[, , primary.statistic] - else plotted[, , 1] + plotted <- plotted[, , 1] } plotted.values <- suppressWarnings(as.numeric(unlist(plotted, use.names = FALSE))) @@ -1105,7 +1103,7 @@ categoryAxisLabelPosition <- function(chart.type, args, data, crosses) # treats NA, "" and unparseable text as unset. Test the parsed number, never the text: a blank # box arrives as NA, and nzchar(NA) is TRUE, so text would read as a floor. The arg is a single # text box (NULL or length 1), so isTRUE() on the parsed number is enough - the any(nzchar()) - # reads in getPPTSettings() guard length 0, not longer vectors. + # calls in getPPTSettings() are there to survive length 0, not to handle longer vectors. values.minimum <- suppressWarnings(as.numeric(gsub("[ ,]", "", as.character(args$values.bounds.minimum)))) # setScatterAxesBounds runs after this and pads the floor below zero for plenty of positive Y # ranges - [5, 95] gives -10 - so an unpinned scatter floor could be anywhere. diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index 3122f82..0f0037a 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -610,8 +610,6 @@ test_that("Category axis label position handles the shapes data arrives in", stats <- array(c(10, 20, 30, 40, -1.5, 2, -0.3, 1.1), dim = c(2, 2, 2), dimnames = list(c("r1", "r2"), c("c1", "c2"), c("Column %", "z-Statistic"))) expect_equal(position(stats), "NextTo") - expect_equal(position(structure(stats, statistic = "z-Statistic")), "Low") - expect_equal(position(structure(stats, statistic = c("two", "names"))), "NextTo") # The bound is read as the chart reads it, and anything it cannot read counts as unset. expect_equal(position(matrix(1:6, 2), "-5,000"), "Low")