From a14e842ad43ff4151ff3df9b7ff48f96b8392876 Mon Sep 17 00:00:00 2001 From: chschan Date: Thu, 16 Jul 2026 14:05:02 +1000 Subject: [PATCH 1/9] FS2-4532: per-series line type, marker symbol and size in PPT export Co-Authored-By: Claude Opus 4.8 (1M context) --- R/cchart.R | 38 +++++++++++++++++++++++++---- tests/testthat/test-chartsettings.R | 29 +++++++++++++++++++++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/R/cchart.R b/R/cchart.R index 03d6cf4..1d83292 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -634,6 +634,16 @@ scatterAxisWarning <- function(data, user.args) } +# FS2-4532: map plotly marker symbol names to PowerPoint marker styles. +markerSymbolToPPTStyle <- function(symbols) +{ + base <- sub("-open$", "", tolower(symbols)) + lookup <- c(circle = "Circle", square = "Square", diamond = "Diamond") + out <- unname(lookup[base]) + out[is.na(out)] <- "Circle" + out +} + getPPTSettings <- function(chart.type, args, data) { # Opacity is by default set to NULL in the javascript code @@ -664,6 +674,10 @@ getPPTSettings <- function(chart.type, args, data) else if (!is.null(args$marker.border.opacity)) tmp.line.style <- "Solid" + # FS2-4532: line type can be a per-series comma-separated string + tmp.line.style <- rep(ConvertCommaSeparatedStringToVector(tmp.line.style), + length = tmp.n) + tmp.line.thickness <- 1 if (chart.type %in% c("Line", "Radar", "Time Series")) tmp.line.thickness <- as.numeric(ConvertCommaSeparatedStringToVector(args$line.thickness)) @@ -682,6 +696,15 @@ getPPTSettings <- function(chart.type, args, data) tmp.line.color <- "#FFFFFF" tmp.line.color <- rep(tmp.line.color, length = tmp.n) + # FS2-4532: marker size / symbol can be per-series + tmp.marker.size <- if (is.null(args$marker.size)) 6 + else as.numeric(ConvertCommaSeparatedStringToVector(args$marker.size)) + tmp.marker.size <- rep(tmp.marker.size, length = tmp.n) + tmp.marker.symbols <- if (is.null(args$marker.symbols)) "Circle" + else markerSymbolToPPTStyle( + ConvertCommaSeparatedStringToVector(args$marker.symbols)) + tmp.marker.symbols <- rep(tmp.marker.symbols, length = tmp.n) + tmp.data.label.show <- isTRUE(args$data.label.show) tmp.data.label.show.category.labels <- FALSE if (isScatter(chart.type) && !isTRUE(args$scatter.labels.as.hovertext)) @@ -734,8 +757,8 @@ getPPTSettings <- function(chart.type, args, data) # When scatterplots use colors as a numerical scale # we can assume a single template series series.settings <- list(list( - CustomPoints = getColorsAsNumericScale(data, args$colors, tmp.opacity, args$marker.size), - Marker = list(Size = args$marker.size, OutlineStyle = "None"), + CustomPoints = getColorsAsNumericScale(data, args$colors, tmp.opacity, tmp.marker.size[1]), + Marker = list(Size = tmp.marker.size[1], OutlineStyle = "None"), ShowDataLabels = tmp.data.label.show, DataLabelsPosition = "Center", DataLabelsFont = list(family = args$data.label.font.family, @@ -761,7 +784,7 @@ getPPTSettings <- function(chart.type, args, data) DataLabelsPosition = tmp.data.label.position, OutlineColor = tmp.line.color[1], # style is none if no border color defined OutlineWidth = tmp.line.thickness[1], - OutlineStyle = tmp.line.style)) + OutlineStyle = tmp.line.style[1])) } else series.settings <- lapply(1:length(args$colors), @@ -775,15 +798,20 @@ getPPTSettings <- function(chart.type, args, data) DataLabelsPosition = tmp.data.label.position, OutlineColor = tmp.line.color[i], OutlineWidth = tmp.line.thickness[i], - OutlineStyle = tmp.line.style)}) + OutlineStyle = tmp.line.style[i])}) tmp.n <- length(series.settings) if ((isScatter(chart.type) && isTRUE(args$scatter.colors.as.categorical)) || chart.type == "Line") for (i in 1:tmp.n) - series.settings[[i]]$Marker = list(Size = args$marker.size, + { + marker.i <- list(Size = tmp.marker.size[i], OutlineStyle = "None", BackgroundColor = getHexCode(args$colors[i], tmp.opacity)) + if (chart.type == "Line") # FS2-4532: per-series marker symbol (Line only) + marker.i$Style <- tmp.marker.symbols[i] + series.settings[[i]]$Marker <- marker.i + } # Initialise return output res <- list() diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index c4248e3..169e1c5 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -130,7 +130,8 @@ test_that("Chart settings", expect_equal(attr(res, "ChartSettings")$TemplateSeries[[2]]$OutlineWidth, 1.5) expect_equal(attr(res, "ChartSettings")$TemplateSeries[[3]]$OutlineWidth, 2.25) expect_equal(attr(res, "ChartSettings")$TemplateSeries[[2]]$Marker, - list(Size = 10, OutlineStyle = "None", BackgroundColor = "#ED7D31FF")) + list(Size = 10, OutlineStyle = "None", BackgroundColor = "#ED7D31FF", + Style = "Circle")) res <- CChart("Radar", dat.2d, append.data = TRUE, colors = col.2d, line.thickness = 2) expect_equal(attr(res, "ChartSettings")$TemplateSeries[[1]]$BackgroundColor, "#5C9AD366") @@ -143,6 +144,32 @@ test_that("Chart settings", expect_equal(attr(res, "ChartSettings")$TemplateSeries[[1]]$ShowCategoryNames, FALSE) expect_equal(attr(res, "ChartSettings")$TemplateSeries[[1]]$ShowDataLabels, FALSE) +test_that("FS2-4532: Line PPT settings are per-series", { + res <- CChart("Line", dat.2d, append.data = TRUE, colors = col.2d, + line.type = "Solid,Dot", marker.show = TRUE, + marker.symbols = "circle,square", marker.size = "6,10,14") + ts <- attr(res, "ChartSettings")$TemplateSeries + expect_equal(ts[[1]]$OutlineStyle, "Solid") + expect_equal(ts[[2]]$OutlineStyle, "Dot") + expect_equal(ts[[3]]$OutlineStyle, "Solid") # recycled + expect_equal(ts[[1]]$Marker$Style, "Circle") + expect_equal(ts[[2]]$Marker$Style, "Square") + expect_equal(ts[[3]]$Marker$Style, "Circle") # recycled + expect_equal(ts[[1]]$Marker$Size, 6) + expect_equal(ts[[2]]$Marker$Size, 10) + expect_equal(ts[[3]]$Marker$Size, 14) +}) + +test_that("FS2-4532: scalar inputs still broadcast (old Plugins back-compat)", { + res <- CChart("Line", dat.2d, append.data = TRUE, colors = col.2d, + line.type = "Dash", marker.show = TRUE, marker.size = 8) + ts <- attr(res, "ChartSettings")$TemplateSeries + expect_equal(ts[[1]]$OutlineStyle, "Dash") + expect_equal(ts[[3]]$OutlineStyle, "Dash") + expect_equal(ts[[1]]$Marker$Size, 8) + expect_equal(ts[[3]]$Marker$Size, 8) +}) + res <- CChart("Palm", abs(dat.2d), append.data = TRUE, colors = col.2d) expect_equal(attr(res, "ChartSettings")$TemplateSeries[[1]]$BackgroundColor, "#5C9AD366") expect_equal(attr(res, "ChartSettings")$TemplateSeries[[1]]$OutlineStyle, "Solid") From 5a029daff2d5ceefe03cab3a43353e7c8853dea6 Mon Sep 17 00:00:00 2001 From: chschan Date: Thu, 16 Jul 2026 15:48:39 +1000 Subject: [PATCH 2/9] FS2-4532: scope per-series line type to Line charts only The comma-separated split of tmp.line.style ran for every chart type. Only Line supports per-series line type (Radar has no line.type parameter; other charts use a single style), and Radar/Time Series reach the per-series OutlineStyle loop - so a stray comma-separated line.type could wrongly apply per-series to them. Split only when chart.type == "Line"; still rep() to n series for all types. Mirrors how line.thickness's split is already scoped to its chart-type branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- R/cchart.R | 11 +++++++---- tests/testthat/test-chartsettings.R | 11 +++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/R/cchart.R b/R/cchart.R index 1d83292..06e7267 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -634,7 +634,8 @@ scatterAxisWarning <- function(data, user.args) } -# FS2-4532: map plotly marker symbol names to PowerPoint marker styles. +# Map plotly marker symbol names to PowerPoint marker styles +# https://wiki.q-researchsoftware.com/wiki/PptMarkerSettings#Style markerSymbolToPPTStyle <- function(symbols) { base <- sub("-open$", "", tolower(symbols)) @@ -674,9 +675,11 @@ getPPTSettings <- function(chart.type, args, data) else if (!is.null(args$marker.border.opacity)) tmp.line.style <- "Solid" - # FS2-4532: line type can be a per-series comma-separated string - tmp.line.style <- rep(ConvertCommaSeparatedStringToVector(tmp.line.style), - length = tmp.n) + # FS2-4532: line type is per-series (comma-separated) for Line charts only; every + # other chart type uses a single style for all series. + if (chart.type == "Line") + tmp.line.style <- ConvertCommaSeparatedStringToVector(tmp.line.style) + tmp.line.style <- rep(tmp.line.style, length = tmp.n) tmp.line.thickness <- 1 if (chart.type %in% c("Line", "Radar", "Time Series")) diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index 169e1c5..476b5b8 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -280,6 +280,17 @@ test_that("FS2-4532: scalar inputs still broadcast (old Plugins back-compat)", { expect_equal(attr(res, "ChartSettings")$ValueAxis$Crosses, "Minimum") }) +test_that("FS2-4532: line type is only split per-series for Line charts", { + # Only Line supports per-series line type; a comma-separated line.type on any other + # chart must be treated as a single style for every series, not split across them. + # Radar has no line.type parameter, so CChart warns it does not match - expected here. + res <- suppressWarnings(CChart("Radar", dat.2d, append.data = TRUE, colors = col.2d, + line.type = "Solid,Dot")) + styles <- vapply(attr(res, "ChartSettings")$TemplateSeries, + function(s) s$OutlineStyle, character(1)) + expect_equal(length(unique(styles)), 1L) # all series share one style, not Solid/Dot/... +}) + test_that("Scatter axes bounds", { dat1 <- structure(list(` ` = c(16.5292618516667, 0.479370604963302, 19.8251578509455, From 0ed29d149c721ff6c9541442a88c63fdab5e9ae1 Mon Sep 17 00:00:00 2001 From: chschan Date: Thu, 16 Jul 2026 16:07:47 +1000 Subject: [PATCH 3/9] Bump version; fix comments --- DESCRIPTION | 2 +- R/cchart.R | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 3d8ca98..44b6827 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: flipChart Type: Package Title: Single function for calling charts - CChart -Version: 1.12.12 +Version: 1.12.13 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 06e7267..47a186f 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -675,8 +675,8 @@ getPPTSettings <- function(chart.type, args, data) else if (!is.null(args$marker.border.opacity)) tmp.line.style <- "Solid" - # FS2-4532: line type is per-series (comma-separated) for Line charts only; every - # other chart type uses a single style for all series. + # Line type is per-series (comma-separated) for Line charts only + # Handle this separately in case args$line.type is null (from old gui controls) if (chart.type == "Line") tmp.line.style <- ConvertCommaSeparatedStringToVector(tmp.line.style) tmp.line.style <- rep(tmp.line.style, length = tmp.n) @@ -699,10 +699,10 @@ getPPTSettings <- function(chart.type, args, data) tmp.line.color <- "#FFFFFF" tmp.line.color <- rep(tmp.line.color, length = tmp.n) - # FS2-4532: marker size / symbol can be per-series tmp.marker.size <- if (is.null(args$marker.size)) 6 else as.numeric(ConvertCommaSeparatedStringToVector(args$marker.size)) tmp.marker.size <- rep(tmp.marker.size, length = tmp.n) + tmp.marker.symbols <- if (is.null(args$marker.symbols)) "Circle" else markerSymbolToPPTStyle( ConvertCommaSeparatedStringToVector(args$marker.symbols)) From 286c01bb5422b46b68a9e7de163fc19d0b4ef08f Mon Sep 17 00:00:00 2001 From: chschan Date: Fri, 31 Jul 2026 13:48:49 +1000 Subject: [PATCH 4/9] Draw markers only where the line chart shows them getPPTSettings gave every line chart a marker on every point, ignoring marker.show entirely, so an exported deck disagreed with the chart on screen wherever markers were only at the ends of a series. flipStandardCharts now reports the points showing a marker. Where that is not every point, the series turns its own marker off and those points switch it back on again, keeping the series' symbol. A list of points numbered across the whole chart is CombinedScatter's annotation borders, which say nothing about visibility, so it is left alone; so is a chart from a flipStandardCharts that reports nothing, which keeps the old behaviour. Co-Authored-By: Claude Opus 5 (1M context) --- R/cchart.R | 45 ++++++++++++++++++-- tests/testthat/test-markerppt.R | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 tests/testthat/test-markerppt.R diff --git a/R/cchart.R b/R/cchart.R index 47a186f..23cb7e2 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -367,7 +367,9 @@ CChart <- function(chart.type, x, small.multiples = FALSE, result <- do.call(fun.and.pars$chart.function, eval(parse(text = args))) chart.warning <- attr(result, "ChartWarning") result <- addLabels(result, chart.type, user.args$title, categories.title, values.title, user.args$data.label.format) - chart.settings <- updateChartSettingsWithLabels(chart.settings, attr(result, "ChartLabels"), attr(result, "CustomPoints")) + chart.settings <- updateChartSettingsWithLabels(chart.settings, attr(result, "ChartLabels"), + attr(result, "CustomPoints"), + markers.at.every.point = identical(attr(result, "ChartType"), "Line Markers")) if (isScatter(chart.type)) { @@ -495,7 +497,8 @@ addLabels <- function(x, chart.type, chart.title, categories.title, values.title return(x) } -updateChartSettingsWithLabels <- function(chart.settings, chart.labels, custom.points) +updateChartSettingsWithLabels <- function(chart.settings, chart.labels, custom.points, + markers.at.every.point = FALSE) { if (!is.null(chart.labels)) { @@ -514,9 +517,29 @@ updateChartSettingsWithLabels <- function(chart.settings, chart.labels, custom.p chart.settings$TemplateSeries[[i]]$ShowDataLabels <- FALSE } + # Points numbered within their own series carry per-point marker visibility: the series + # turns its marker off and the points below switch it back on again. A globally numbered + # list is CombinedScatter's annotation borders, which say nothing about visibility, so it + # is left alone. A chart whose every point has a marker says so at series level instead, + # rather than repeating itself once per point. + # The absence of the attribute is how a flipStandardCharts without this feature presents, + # and it has to keep the old behaviour of a marker on every point. + per.series.markers <- identical(attr(custom.points, "IndexBase"), "series") && + !markers.at.every.point + series.marker.style <- list() + if (per.series.markers) + for (i in seq_along(chart.settings$TemplateSeries)) + { + style.i <- chart.settings$TemplateSeries[[i]]$Marker$Style + series.marker.style[[i]] <- if (is.null(style.i)) NA_character_ else style.i + if (!is.null(chart.settings$TemplateSeries[[i]]$Marker)) + chart.settings$TemplateSeries[[i]]$Marker$Style <- "None" + } + # Update ChartSettings to incorporate annotation info from flipStandardCharts # that is stored in the CustomPoints attribute - # Currently this is only used to add annotation marker borders in CombinedScatter + # Used for annotation marker borders in CombinedScatter, and for per-point marker + # visibility in Line charts if (!is.null(custom.points) && any(sapply(custom.points, Negate(is.null)))) { n.series <- min(length(chart.settings$TemplateSeries), length(custom.points)) @@ -550,6 +573,14 @@ updateChartSettingsWithLabels <- function(chart.settings, chart.labels, custom.p chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$BackgroundColor <- chart.settings$TemplateSeries[[i]]$BackgroundColor } + # The series' own symbol, which was replaced by "None" above so that only + # the points listed here show a marker + if (per.series.markers && !is.na(series.marker.style[[i]]) && + is.null(chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$Style)) + { + chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$Style <- + series.marker.style[[i]] + } k <- k + 1 next } @@ -564,6 +595,14 @@ updateChartSettingsWithLabels <- function(chart.settings, chart.labels, custom.p chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$BackgroundColor <- chart.settings$TemplateSeries[[i]]$Marker$BackgroundColor } + # The series' own symbol, which was replaced by "None" above so that only + # the points listed here show a marker + if (per.series.markers && !is.na(series.marker.style[[i]]) && + is.null(chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$Style)) + { + chart.settings$TemplateSeries[[i]]$CustomPoints[[k]]$Marker$Style <- + series.marker.style[[i]] + } k <- k + 1 } } diff --git a/tests/testthat/test-markerppt.R b/tests/testthat/test-markerppt.R new file mode 100644 index 0000000..7dcf04f --- /dev/null +++ b/tests/testthat/test-markerppt.R @@ -0,0 +1,74 @@ +context("Per-point markers in the PPT export") + +# flipStandardCharts reports which points of a Line chart show a marker, in the CustomPoints +# attribute. These check that the export turns that into per-point PptMarkerSettings rather +# than putting a marker on every point. + +set.seed(12345) +dat.2d <- matrix(rnorm(5 * 2), 5, 2, dimnames = list(letters[1:5], c("A", "B"))) +col.2d <- c("#5C9AD3", "#ED7D31") + +seriesOf <- function(...) + attr(CChart("Line", dat.2d, append.data = TRUE, colors = col.2d, ...), + "ChartSettings")$TemplateSeries + +indicesOf <- function(s) vapply(s$CustomPoints, function(p) p$Index, numeric(1)) +stylesOf <- function(s) vapply(s$CustomPoints, function(p) p$Marker$Style, character(1)) + +test_that("Markers everywhere stay a series-level setting", { + ts <- seriesOf(marker.show = TRUE, marker.size = 8) + expect_equal(ts[[1]]$Marker$Style, "Circle") + expect_equal(ts[[1]]$Marker$Size, 8) + expect_length(ts[[1]]$CustomPoints, 0) + expect_length(ts[[2]]$CustomPoints, 0) +}) + +test_that("A Line chart with no markers turns them off at series level", { + ts <- seriesOf(marker.show = FALSE) + expect_equal(ts[[1]]$Marker$Style, "None") + expect_equal(ts[[2]]$Marker$Style, "None") + expect_length(ts[[1]]$CustomPoints, 0) +}) + +test_that("Markers at ends are off by default and switched on at the end points", { + ts <- seriesOf(marker.show.at.ends = TRUE, marker.size = 8) + expect_equal(ts[[1]]$Marker$Style, "None") + expect_equal(indicesOf(ts[[1]]), c(0, 4)) + expect_equal(stylesOf(ts[[1]]), c("Circle", "Circle")) + expect_equal(ts[[1]]$CustomPoints[[1]]$Marker$Size, 8) + + # Numbered within the series, so series 2 repeats 0 and 4 rather than continuing + expect_equal(indicesOf(ts[[2]]), c(0, 4)) +}) + +test_that("Markers at the last end switch on a single point per series", { + ts <- seriesOf(marker.show.at.last.end = TRUE, marker.size = 8) + expect_equal(ts[[1]]$Marker$Style, "None") + expect_equal(indicesOf(ts[[1]]), 4) + expect_equal(indicesOf(ts[[2]]), 4) +}) + +test_that("Each series keeps its own marker symbol on its custom points", { + ts <- seriesOf(marker.show.at.ends = TRUE, marker.symbols = "circle,square") + expect_equal(stylesOf(ts[[1]]), c("Circle", "Circle")) + expect_equal(stylesOf(ts[[2]]), c("Square", "Square")) +}) + +test_that("A chart from an older flipStandardCharts keeps markers on every point", { + # No CustomPoints attribute at all is how a build without this feature presents, and it + # must not be read as "no markers anywhere" + settings <- list(TemplateSeries = list( + list(Marker = list(Style = "Circle", Size = 6), BackgroundColor = "#5C9AD3"))) + out <- updateChartSettingsWithLabels(settings, NULL, NULL) + expect_equal(out$TemplateSeries[[1]]$Marker$Style, "Circle") +}) + +test_that("Globally numbered custom points are left alone", { + # CombinedScatter numbers its points across the chart and has no per-point marker + # visibility; only a list marked as series-numbered drives markers off at series level. + settings <- list(TemplateSeries = list( + list(Marker = list(Style = "Circle", Size = 6), BackgroundColor = "#5C9AD3"))) + scatter.points <- list(list(list(Index = 2, OutlineColor = "#FF0000", OutlineWidth = 2))) + out <- updateChartSettingsWithLabels(settings, NULL, scatter.points) + expect_equal(out$TemplateSeries[[1]]$Marker$Style, "Circle") +}) From 0fc0e3fef3e63ef88f979365fe26cbc476db2f72 Mon Sep 17 00:00:00 2001 From: chschan Date: Fri, 31 Jul 2026 14:38:12 +1000 Subject: [PATCH 5/9] Support line type in Radar --- DESCRIPTION | 2 +- R/cchart.R | 6 +++-- tests/testthat/test-chartsettings.R | 40 ++++++++++++++++++++++------- 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 44b6827..614abf4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,7 +23,7 @@ Imports: flipChartBasics, flipData (>= 1.2.7), flipFormat, - flipStandardCharts (>= 1.32.12), + flipStandardCharts (>= 1.32.15), flipStatistics, flipTables (>= 2.8.8), flipTime, diff --git a/R/cchart.R b/R/cchart.R index 23cb7e2..fdd4340 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -714,9 +714,11 @@ getPPTSettings <- function(chart.type, args, data) else if (!is.null(args$marker.border.opacity)) tmp.line.style <- "Solid" - # Line type is per-series (comma-separated) for Line charts only + # Line type is per-series (comma-separated) for the charts whose line type is per series. + # Time Series takes one line type for the whole chart, so splitting it there would turn a + # single setting into a per-series one. # Handle this separately in case args$line.type is null (from old gui controls) - if (chart.type == "Line") + if (chart.type %in% c("Line", "Radar")) tmp.line.style <- ConvertCommaSeparatedStringToVector(tmp.line.style) tmp.line.style <- rep(tmp.line.style, length = tmp.n) diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index 476b5b8..53294b7 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -160,6 +160,24 @@ test_that("FS2-4532: Line PPT settings are per-series", { expect_equal(ts[[3]]$Marker$Size, 14) }) +test_that("Radar PPT settings take a line type per series", { + # Radar reaches the OutlineStyle loop the same way Line does, so a comma-separated + # line type has to be split for it too rather than reaching PowerPoint as one string + res <- CChart("Radar", dat.2d, append.data = TRUE, colors = col.2d, + line.type = "Solid,Dot") + ts <- attr(res, "ChartSettings")$TemplateSeries + expect_equal(ts[[1]]$OutlineStyle, "Solid") + expect_equal(ts[[2]]$OutlineStyle, "Dot") + expect_equal(ts[[3]]$OutlineStyle, "Solid") # recycled +}) + +test_that("Radar line type reaches the chart as well as the export", { + # It used to be read for PowerPoint but dropped on the way to the chart, so a dotted + # radar exported dotted and rendered solid + expect_warning(CChart("Radar", dat.2d, append.data = TRUE, colors = col.2d, + line.type = "Dot"), NA) +}) + test_that("FS2-4532: scalar inputs still broadcast (old Plugins back-compat)", { res <- CChart("Line", dat.2d, append.data = TRUE, colors = col.2d, line.type = "Dash", marker.show = TRUE, marker.size = 8) @@ -280,15 +298,19 @@ test_that("FS2-4532: scalar inputs still broadcast (old Plugins back-compat)", { expect_equal(attr(res, "ChartSettings")$ValueAxis$Crosses, "Minimum") }) -test_that("FS2-4532: line type is only split per-series for Line charts", { - # Only Line supports per-series line type; a comma-separated line.type on any other - # chart must be treated as a single style for every series, not split across them. - # Radar has no line.type parameter, so CChart warns it does not match - expected here. - res <- suppressWarnings(CChart("Radar", dat.2d, append.data = TRUE, colors = col.2d, - line.type = "Solid,Dot")) - styles <- vapply(attr(res, "ChartSettings")$TemplateSeries, - function(s) s$OutlineStyle, character(1)) - expect_equal(length(unique(styles)), 1L) # all series share one style, not Solid/Dot/... +test_that("FS2-4532: line type is only split per-series for the charts that support it", { + # Line and Radar both take a line type per series, so a comma-separated value is split + # across them. Time Series takes one line type for the whole chart, and splitting it + # there would turn a single setting into a per-series one. + dat <- matrix(1:6, 3, 2, dimnames = list(letters[1:3], c("A", "B"))) + args <- list(colors = c("#FF0000", "#00AA00"), line.type = "Solid,Dot") + stylesFor <- function(chart.type) + vapply(getPPTSettings(chart.type, args, dat)$TemplateSeries, + function(s) s$OutlineStyle, character(1)) + + expect_equal(stylesFor("Line"), c("Solid", "Dot")) + expect_equal(stylesFor("Radar"), c("Solid", "Dot")) + expect_equal(stylesFor("Time Series"), c("Solid,Dot", "Solid,Dot")) }) test_that("Scatter axes bounds", From 8fc18e2e00228f269df30cec9781fe6222444223 Mon Sep 17 00:00:00 2001 From: chschan Date: Mon, 3 Aug 2026 08:37:29 +1000 Subject: [PATCH 6/9] Relax version requirements to allow install --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 614abf4..44b6827 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,7 +23,7 @@ Imports: flipChartBasics, flipData (>= 1.2.7), flipFormat, - flipStandardCharts (>= 1.32.15), + flipStandardCharts (>= 1.32.12), flipStatistics, flipTables (>= 2.8.8), flipTime, From 3c64b81e3bd3724cb2bb74387260a2136d670305 Mon Sep 17 00:00:00 2001 From: chschan Date: Mon, 3 Aug 2026 15:48:46 +1000 Subject: [PATCH 7/9] Read the line shape per series when deciding to smooth PowerPoint takes one smoothing setting for the whole chart, and it was worked out by comparing the shape argument against "Curved". Now that the shape can name one per series, that comparison reads "Curved,Curved" as no series being curved at all, so a chart drawn entirely with curves exported straight. The value is parsed first and the first series decides, which is the series the other whole-chart settings here are taken from. Curved is the name the controls send and spline is plotly's; the chart draws either as a curve, so the export now treats them alike. The numeric settings needed nothing: they already read a comma-separated string, a number, or one number per series, so charts saved from the old text box and from the new numeric controls both export the same. Tests cover that rather than leaving it to be rediscovered. Co-Authored-By: Claude Opus 5 (1M context) --- R/cchart.R | 10 +++++- tests/testthat/test-chartsettings.R | 48 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/R/cchart.R b/R/cchart.R index fdd4340..34cc97c 100644 --- a/R/cchart.R +++ b/R/cchart.R @@ -973,7 +973,15 @@ getPPTSettings <- function(chart.type, args, data) res$GapWidth = min(5.0, args$bar.gap / (1 - args$bar.gap)) * 100 } if (chart.type == "Line") - res$Smooth = isTRUE(args$shape == "Curved") + { + # The shape can name one per series, so the whole-chart setting PowerPoint takes is + # decided by the first series, as the other collapsed settings here are. Comparing + # the argument as it arrived would read "Curved,Curved" as no series curved at all. + # Curved is what the controls send and spline is plotly's own name for it; the chart + # draws either as a curve, so the export has to treat them the same way. + tmp.shape <- ConvertCommaSeparatedStringToVector(args$shape) + res$Smooth = isTRUE(tolower(tmp.shape[1]) %in% c("curved", "spline")) + } if (chart.type %in% c("BarMultiColor", "ColumnMultiColor", "Pyramid", "Bar Pictograph") || (isScatter(chart.type) && !isTRUE(args$scatter.colors.as.categorical))) res$ShowLegend <- FALSE diff --git a/tests/testthat/test-chartsettings.R b/tests/testthat/test-chartsettings.R index 53294b7..3c0d612 100644 --- a/tests/testthat/test-chartsettings.R +++ b/tests/testthat/test-chartsettings.R @@ -441,3 +441,51 @@ test_that("getGridLineStyle handles missing/NA widths (RS-22447)", expect_equal(getGridLineStyle(NA, NULL), "Solid") expect_equal(getGridLineStyle(NULL, "Dot"), "Dot") }) + +test_that("Smooth follows the first series' shape, whatever form the shape arrives in", { + dat <- matrix(1:6, 3, 2, dimnames = list(letters[1:3], c("A", "B"))) + smoothFor <- function(shape) { + args <- list(colors = c("#FF0000", "#00AA00")) + if (!is.null(shape)) args$shape <- shape + getPPTSettings("Line", args, dat)$Smooth + } + + # PowerPoint takes one setting for the whole chart, so a per-series shape has to pick + # one; the first series is the same series other chart-wide settings are taken from. + expect_true(smoothFor("Curved")) + expect_false(smoothFor("Straight")) + + # The per-series forms: comma-separated as the Plugins send it, or a vector + expect_true(smoothFor("Curved,Curved")) + expect_true(smoothFor("Curved, Straight")) + expect_false(smoothFor("Straight,Curved")) + expect_true(smoothFor(c("Curved", "Straight"))) + expect_false(smoothFor(c("Straight", "Curved"))) + + # Unset stays unsmoothed, and case does not matter + expect_false(smoothFor(NULL)) + expect_true(smoothFor("curved")) + + # Curved and Straight are what the controls send, but the chart also takes plotly's own + # names, and a chart drawn curved has to export curved whichever name asked for it + expect_true(smoothFor("spline")) + expect_true(smoothFor("Spline")) + expect_false(smoothFor("linear")) + expect_true(smoothFor("spline,linear")) + expect_false(smoothFor("linear,spline")) +}) + +test_that("Numeric series settings export from every form a chart may have been saved with", { + # The controls used to be a text box taking "6, 10, 14", and are now numeric ones, so a + # deck exported today may come from either. Both have to keep working. + dat <- matrix(1:9, 3, 3, dimnames = list(letters[1:3], c("A", "B", "C"))) + sizesFor <- function(v) + vapply(getPPTSettings("Line", list(colors = c("#F00", "#0A0", "#00F"), + marker.size = v), dat)$TemplateSeries, + function(s) s$Marker$Size, numeric(1)) + + expect_equal(sizesFor("6,10,14"), c(6, 10, 14)) # old text box, per series + expect_equal(sizesFor(c(6, 10, 14)), c(6, 10, 14)) # new numeric controls, per series + expect_equal(sizesFor(10), c(10, 10, 10)) # new numeric control, chart wide + expect_equal(sizesFor("10"), c(10, 10, 10)) # old text box, chart wide +}) From 39fe25676e822e4b6cbc1ef46191cf40ad02b1cd Mon Sep 17 00:00:00 2001 From: chschan Date: Mon, 3 Aug 2026 17:51:36 +1000 Subject: [PATCH 8/9] Stop pinning the companion packages in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pinned versions decided what was installed, so the version requirement in DESCRIPTION only judged the result: relaxing it could not help while CI was still fetching flipStandardCharts 1.32.4. Both pins had fallen a long way behind — master is 1.32.15 and 1.2.1 — and rhtmlCombinedScatter 1.0.14 no longer satisfies what flipStandardCharts itself asks for. They were there to build against companion versions that had not been merged yet. Both are merged, and DESCRIPTION already lists flipStandardCharts under Remotes, so with no pin the dependencies come from master. Co-Authored-By: Claude Opus 5 (1M context) --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a8965b0..7ffc5d6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,7 +7,7 @@ parameters: default: "" remote-deps: type: string - default: Displayr/rhtmlCombinedScatter@1.0.14,Displayr/flipStandardCharts@1.32.4 + default: "" plugins-branch: type: string default: "" From 5ef486a739342d8319f89e8cb94484d6c4a2c9d7 Mon Sep 17 00:00:00 2001 From: chschan Date: Mon, 3 Aug 2026 21:25:54 +1000 Subject: [PATCH 9/9] Build against the merged companion versions The pinned versions decided what CI installed, so the requirement in DESCRIPTION only judged the result afterwards. Both pins had fallen well behind what the tests need: the per-point marker export reads a CustomPoints attribute that flipStandardCharts only reports from 1.32.14, and Radar's line type arrived around the same time, so building against 1.32.4 fails those tests whatever the requirement says. rhtmlCombinedScatter 1.0.14 also no longer satisfies what flipStandardCharts asks for. Both companions are merged, so the pins move up to their current versions rather than being dropped: emptying them made the build resolve a published flipStandardCharts older than the tests need. Co-Authored-By: Claude Opus 5 (1M context) --- .circleci/config.yml | 2 +- DESCRIPTION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 7ffc5d6..53b39eb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -7,7 +7,7 @@ parameters: default: "" remote-deps: type: string - default: "" + default: Displayr/rhtmlCombinedScatter@1.2.1,Displayr/flipStandardCharts@1.32.15 plugins-branch: type: string default: "" diff --git a/DESCRIPTION b/DESCRIPTION index 44b6827..614abf4 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -23,7 +23,7 @@ Imports: flipChartBasics, flipData (>= 1.2.7), flipFormat, - flipStandardCharts (>= 1.32.12), + flipStandardCharts (>= 1.32.15), flipStatistics, flipTables (>= 2.8.8), flipTime,