Skip to content

Fix. Account for line style = "none". Closes #91.#99

Open
trekonom wants to merge 4 commits intoardata-fr:masterfrom
trekonom:issue-91
Open

Fix. Account for line style = "none". Closes #91.#99
trekonom wants to merge 4 commits intoardata-fr:masterfrom
trekonom:issue-91

Conversation

@trekonom
Copy link
Contributor

Hi David and Eli,

just in case you find the time. This PR proposes a fix for - the not very well reported - issue #91 .

Using the current CRAN and dev version setting the line style in a ms_linechart to "none" via chart_data_line_style does not work. Instead we get a solid line with the default settings for color, ....:

library(mschart)
library(officer)

dat <- data.frame(
  color = c(rep("green", 3), rep("unclear", 3), rep("gray", 3)),
  musician = c(rep(c("Robert Wyatt", "John Zorn", "Damon Albarn"), 3)),
  count = c(120, 101, 131, 200, 154, 187, 122, 197, 159),
  stringsAsFactors = FALSE
)

lty <- c(green = "none", unclear = "dashed", gray = "none")
lty <- lty[order(names(lty))]

chart <- ms_linechart(data = dat, x = "musician", y = "count", group = "color")
chart <- chart_data_line_style(chart, values = lty)

file <- tempfile(fileext = ".pptx")
doc <- read_pptx()
doc <- add_slide(doc, layout = "Title and Content", master = "Office Theme")
doc <- ph_with(doc, value = chart, location = ph_location_fullsize())
print(doc, target = file)

# doconv::to_miniature(file, fileout = "issue-91.png")

image

The issue is that currently an empty string is "added" for lines with a "none" style:

mschart/R/utils.R

Lines 51 to 53 in b69fbf6

if( is_transparent || x$width < 0.001 || x$style %in% "none" ){
return("")
}

instead of the required "<c:spPr><a:ln><a:noFill/></a:ln></c:spPr>" as is already done in
line_str <- "<c:spPr><a:ln><a:noFill/></a:ln></c:spPr>"

when the style of the linechart is set globally using e.g. chart_settings(x, style = "marker").

The proposed fix adds a none_as_empty argument to ooxml_fp_border so that one can choose between having an empty string (where this is fine) or a <a:ln><a:noFill/></a:ln> for a "none" style.

Using the proposed fix we get the correct result:

image

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.

1 participant