Merge pull request #458 from JanMarvin/gh_issue_457

fix add_drawing() sheet logic. closes #457
JanMarvin · Dec 10, 2022 · 7aa824f · 7aa824f
2 parents 18ef03e + 991bc07
commit 7aa824f
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 38 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,7 +1,7 @@
 Type: Package
 Package: openxlsx2
 Title: Read, Write and Edit 'xlsx' Files
-Version: 0.4
+Version: 0.4.0.9000
 Language: en-US
 Authors@R: c(
  person("Jordan Mark", "Barbone", email = "jmbarbone@gmail.com", role = "aut", comment = c(ORCID = "0000-0001-9788-3628")),

diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,15 @@
 # openxlsx2 (in development)
 
+## Fixes
+
+* Previously, adding mscharts to sheets was only possible if (1) the worksheet already contained a drawing (if the workbook was loaded) or (2) to the last sheet of the workbook. This has now been fixed. Adding mscharts to any worksheet in the workbook is now possible as intended. [458](https://github.com/JanMarvin/openxlsx2/pull/458)
+
+
+***************************************************************************
+
+
+# openxlsx2 0.4
+
 ## New features
 
 * Provide `rvg` support via `wb_add_drawing()`. This allows integrating `rvg` plots into xlsx files. [449](https://github.com/JanMarvin/openxlsx2/pull/449)

diff --git a/R/class-workbook.R b/R/class-workbook.R
@@ -1538,41 +1538,6 @@ wbWorkbook <- R6::R6Class(
  file.copy(self$vbaProject, xlDir)
  }
 
- # ## Drawings
- # for (draw in seq_along(self$drawings)) {
-
- # relship <- data.frame()
- # if (!all(identical(self$worksheets_rels[[draw]], character()))) {
- # relship <- rbindlist(xml_attr(unlist(self$worksheets_rels[[draw]]), "Relationship"))
- # relship$typ <- basename(relship$Type)
- # relship$tid <- as.numeric(gsub("\\D+", "", relship$Target))
- # }
-
- # rid <- max(0, relship$tid)
-
- # if (!any(relship$typ == "drawing")) {
-
- # # check if drawings required
- # drawing_rel <- NULL
- # if (length(self$drawings) != 0 && length(self$drawings[[draw]])) {
- # rid <- rid + 1
- # drawing_rel <- sprintf('<Relationship Id="rId%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing" Target="../drawings/drawing%s.xml"/>', rid, draw)
- # }
-
- # drawing_vml_rel <- NULL
- # # if drawings_vml is set or if file has comments. comments create drawings_vml when writing
- # if ((length(self$drawings_vml) > 0 && length(self$drawings_vml[[draw]]) > 0) || (length(self$comments) > 0 && length(self$comments[[draw]]) > 0)) {
- # rid <- rid + 1
- # drawing_vml_rel <- sprintf('<Relationship Id="rId%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/vmlDrawing" Target="../drawings/vmlDrawing%s.vml"/>', rid, draw)
- # }
-
- # self$worksheets_rels[[draw]] <- unique(c(self$worksheets_rels[[draw]], drawing_rel, drawing_vml_rel))
-
- # }
-
-
- # }
-
  ## write worksheet, worksheet_rels, drawings, drawing_rels
  ct <- private$writeSheetDataXML(
  ct,
@@ -3736,8 +3701,7 @@ wbWorkbook <- R6::R6Class(
  # if a drawing exisits, we already added ourself to it. Otherwise we
  # create a new drawing.
  if (has_no_drawing) {
- no_drawing <- length(self$drawings)
- self$worksheets_rels[[sheet]] <- sprintf("<Relationship Id=\"rId%s\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing\" Target=\"../drawings/drawing%s.xml\"/>", next_relship, no_drawing)
+ self$worksheets_rels[[sheet]] <- sprintf("<Relationship Id=\"rId%s\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing\" Target=\"../drawings/drawing%s.xml\"/>", next_relship, sheet)
  self$worksheets[[sheet]]$drawing <- sprintf("<drawing r:id=\"rId%s\"/>", next_relship)
  }
 

diff --git a/tests/testthat/test-class-workbook.R b/tests/testthat/test-class-workbook.R
@@ -534,4 +534,23 @@ test_that("add_drawing works", {
  got <- wb$drawings_rels
  expect_equal(exp, got)
 
+
+ # write data starting at B2
+ wb <- wb_workbook()$
+ add_worksheet()$add_data(x = mtcars)$
+ add_worksheet()$add_data(x = mtcars)$
+ add_worksheet()$add_data(x = mtcars)$
+ add_worksheet()$add_data(x = mtcars)$
+ add_mschart(dims = "F4:L20", 2, graph = chart_01)$
+ add_mschart(dims = "F4:L20", 3, graph = chart_01)
+
+ exp <- list(
+ character(0),
+ "<Relationship Id=\"rId1\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing\" Target=\"../drawings/drawing2.xml\"/>",
+ "<Relationship Id=\"rId1\" Type=\"http://schemas.openxmlformats.org/officeDocument/2006/relationships/drawing\" Target=\"../drawings/drawing3.xml\"/>",
+ character(0)
+ )
+ got <- wb$worksheets_rels
+ expect_equal(exp, got)
+
 })