Skip to content

Commit

Permalink
[drawings] improve relationship id selection (#838)
Browse files Browse the repository at this point in the history
* [drawing] use next_id()

* [set_col_width] speedup unfold_cols()

* update NEWS

* cleanup code

* add test
  • Loading branch information
JanMarvin committed Nov 2, 2023
1 parent 88c94b1 commit 128ff77
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 10 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

* Previously formulas written as data frames were not xml escaped. [834](https://github.com/JanMarvin/openxlsx2/pull/834)

* Improve drawing relationship id selection that could cause issues with unordered relationship ids in loaded workbooks. [838](https://github.com/JanMarvin/openxlsx2/pull/838)


***************************************************************************

Expand Down
6 changes: 3 additions & 3 deletions R/baseXML.R
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ genBaseStyleSheet <- function(dxfs = NULL, tableStyles = NULL, extLst = NULL) {
)
}

genBasePic <- function(imageNo) {
genBasePic <- function(imageNo, next_id) {
sprintf('<xdr:pic>
<xdr:nvPicPr>
<xdr:cNvPr id="%s" name="Picture %s"/>
Expand All @@ -252,7 +252,7 @@ genBasePic <- function(imageNo) {
</xdr:cNvPicPr>
</xdr:nvPicPr>
<xdr:blipFill>
<a:blip xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" r:embed="rId%s">
<a:blip xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships" r:embed="%s">
</a:blip>
<a:stretch>
<a:fillRect/>
Expand All @@ -263,7 +263,7 @@ genBasePic <- function(imageNo) {
<a:avLst/>
</a:prstGeom>
</xdr:spPr>
</xdr:pic>', imageNo, imageNo, imageNo)
</xdr:pic>', imageNo, imageNo, next_id)
}

genBaseTheme <- function() {
Expand Down
12 changes: 9 additions & 3 deletions R/class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -4677,6 +4677,12 @@ wbWorkbook <- R6::R6Class(
imageNo <- 1L
}

if (length(self$drawings_rels) >= sheet_drawing) {
next_id <- get_next_id(self$drawings_rels[[sheet_drawing]])
} else {
next_id <- "rId1"
}

## write file path to media slot to copy across on save
tmp <- file
names(tmp) <- stri_join("image", mediaNo, ".", imageType)
Expand All @@ -4688,7 +4694,7 @@ wbWorkbook <- R6::R6Class(
'<xdr:absoluteAnchor>',
pos,
sprintf('<xdr:ext cx="%s" cy="%s"/>', width, height),
genBasePic(imageNo),
genBasePic(imageNo, next_id),
"<xdr:clientData/>",
"</xdr:absoluteAnchor>"
)
Expand Down Expand Up @@ -4722,8 +4728,8 @@ wbWorkbook <- R6::R6Class(
self$drawings_rels[[sheet_drawing]] <- c(
old_drawings_rels,
sprintf(
'<Relationship Id="rId%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/image" Target="../media/image%s.%s"/>',
imageNo,
'<Relationship Id="%s" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/image" Target="../media/image%s.%s"/>',
next_id,
mediaNo,
imageType
)
Expand Down
9 changes: 5 additions & 4 deletions R/class-worksheet.R
Original file line number Diff line number Diff line change
Expand Up @@ -450,10 +450,11 @@ wbWorksheet <- R6::R6Class(
out <- NULL
for (i in seq_len(nrow(col_df))) {
z <- col_df[i, ]
for (j in seq(z$min, z$max)) {
z$key <- j
out <- rbind(out, z)
}
keys <- seq.int(z$min, z$max)

out_tmp <- z[rep(1L, length(keys)), ]
out_tmp$key <- keys
out <- rbind(out, out_tmp)
}

# merge and convert to character, remove key
Expand Down
1 change: 1 addition & 0 deletions inst/WORDLIST
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ pageBreakPreview
pageLayout
pageSetup
paperSize
params
patternFill
pcdata
ph
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/test-class-workbook.R
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ test_that("wb_set_col_widths", {
got <- wb$worksheets[[1]]$cols_attr
expect_equal(exp, got)

wb <- wb_workbook()$add_worksheet()
wb$worksheets[[1]]$cols_attr <- c(
"<col min=\"1\" max=\"17\" width=\"30.77734375\" style=\"16\" customWidth=\"1\"/>",
"<col min=\"18\" max=\"16384\" width=\"8.88671875\" style=\"16\"/>"
)

expect_silent(wb$set_col_widths(cols = 19, width = 9))

})


Expand Down

0 comments on commit 128ff77

Please sign in to comment.