Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with conditional formatting #664

Closed
Michiel91 opened this issue Jun 25, 2023 · 16 comments
Closed

Issues with conditional formatting #664

Michiel91 opened this issue Jun 25, 2023 · 16 comments

Comments

@Michiel91
Copy link

While testing several types of conditional formatting, I noticed that only one is applied to the actual workbook when more are set. Below is a detailed example showing what happens when two conditional formatting styles are applied:

# Create example dataset
data <- matrix(sample(c("A","B","C"), size = 20, replace = TRUE), 5, 4)

# Write data to workbook
workbook <- openxlsx2::wb_workbook() %>%
	openxlsx2::wb_add_worksheet("test") %>%
	openxlsx2::wb_add_data(x = data, 
												 sheet = "test")

# Apply some conditional formatting
xml_fill_code1 <- openxlsx2::read_xml(glue::glue("
								<gradientFill degree=\"45\">
									<stop position=\"0\"><color rgb=\"440154\"/></stop>
									<stop position=\"1\"><color rgb=\"31688E\"/></stop>
								</gradientFill>"),
																			pointer = FALSE)

formatting_style1 <- openxlsx2::create_dxfs_style(gradientFill = xml_fill_code1)

workbook %<>% openxlsx2::wb_add_conditional_formatting(sheet = "test",
																											 rows = 1:nrow(data),
																											 cols = 1:ncol(data),
																											 rule = '=="A"', 
																											 style = "formatting_style1") %>%
	openxlsx2::wb_add_style(formatting_style1)

# Apply additional conditional formatting
xml_fill_code2 <- openxlsx2::read_xml(glue::glue("
								<gradientFill degree=\"45\">
									<stop position=\"0\"><color rgb=\"35B779\"/></stop>
									<stop position=\"1\"><color rgb=\"FDE725\"/></stop>
								</gradientFill>"),
																			pointer = FALSE)

formatting_style2 <- openxlsx2::create_dxfs_style(gradientFill = xml_fill_code2)

workbook %<>% openxlsx2::wb_add_conditional_formatting(sheet = "test",
																											 rows = 1:nrow(data),
																											 cols = 1:ncol(data),
																											 rule = '=="B"', 
																											 style = "formatting_style2") %>%
	openxlsx2::wb_add_style(formatting_style2)

# Export workbook (confirmation that only one style is applied)
openxlsx2::wb_save(workbook, 
									 path = "test.xlsx", 
									 overwrite = TRUE)

# Check formatting (note the invalid NA dxfId's)
workbook$worksheets[[1]]$conditionalFormatting
workbook$styles_mgr$dxf
workbook$styles_mgr$styles$dxfs

The output Excel workbook shows that only one style works, while the other one is shown but not applied properly. The printed workbook properties hint at something going wrong with the dxfId's. Everything past the first style becomes NA (or all 0). Not sure if anything else is causing this bug, but this is what I could already find while debugging.

Thanks for looking into this! Let me know if you can use additional examples or info.

@JanMarvin
Copy link
Owner

Hi @Michiel91 , you have to assign the style to the workbook with a name. (That it works in the first place was a fluke).

See my adjustment below, I had only libreoffice to test and here the gradient did not work as expected:

library(openxlsx2)

## Create example dataset
data <- matrix(sample(c("A","B","C"), size = 20, replace = TRUE), 5, 4)

## create styles

# formatting_style1
xml_fill_code1 <- openxlsx2::read_xml(
  glue::glue("
    <gradientFill degree=\"45\">
        <stop position=\"0\"><color rgb=\"440154\"/></stop>
        <stop position=\"1\"><color rgb=\"31688E\"/></stop>
    </gradientFill>
  "),
  pointer = FALSE)

formatting_style1 <- openxlsx2::create_dxfs_style(gradientFill = xml_fill_code1)

# formatting_style2
xml_fill_code2 <- openxlsx2::read_xml(
  glue::glue("
  <gradientFill degree=\"45\">
    <stop position=\"0\"><color rgb=\"35B779\"/></stop>
    <stop position=\"1\"><color rgb=\"FDE725\"/></stop>
  </gradientFill>
  "),
  pointer = FALSE)

formatting_style2 <- openxlsx2::create_dxfs_style(gradientFill = xml_fill_code2)


## Create workbook
workbook <- openxlsx2::wb_workbook() %>%
  openxlsx2::wb_add_worksheet("test") %>%
  # add data
  openxlsx2::wb_add_data(
    x = data, 
    sheet = "test"
  ) %>% 
  # add conditional styles
  wb_add_style(formatting_style1, "formatting_style1") %>% 
  wb_add_style(formatting_style2, "formatting_style2") %>% 
  # add conditional formatting
  openxlsx2::wb_add_conditional_formatting(
    sheet = "test",
    dims = rowcol_to_dims(
      seq_len(nrow(data)),
      seq_along(data)
    ),
    rule = '=="A"', 
    style = "formatting_style1"
  ) %>% openxlsx2::wb_add_conditional_formatting(
    sheet = "test",
    dims = rowcol_to_dims(
      seq_len(nrow(data)),
      seq_along(data)
    ),
    rule = '=="B"', 
    style = "formatting_style2"
  )

# ## save
# workbook %>% 
#   wb_save(
#     path = "test.xlsx", 
#     overwrite = TRUE
#   )

## Checks
workbook$worksheets[[1]]$conditionalFormatting
#>                                                                                                 A1:T5 
#> "<cfRule type=\"expression\" dxfId=\"0\" priority=\"2\"><formula>A1=&quot;A&quot;</formula></cfRule>" 
#>                                                                                                 A1:T5 
#> "<cfRule type=\"expression\" dxfId=\"1\" priority=\"1\"><formula>A1=&quot;B&quot;</formula></cfRule>"
workbook$styles_mgr$dxf
#>   typ id              name
#> 1 dxf  0 formatting_style1
#> 2 dxf  1 formatting_style2
workbook$styles_mgr$styles$dxfs
#> [1] "<dxf><fill><gradientFill degree=\"45\"><stop position=\"0\"><color rgb=\"440154\"/></stop><stop position=\"1\"><color rgb=\"31688E\"/></stop></gradientFill></fill></dxf>"
#> [2] "<dxf><fill><gradientFill degree=\"45\"><stop position=\"0\"><color rgb=\"35B779\"/></stop><stop position=\"1\"><color rgb=\"FDE725\"/></stop></gradientFill></fill></dxf>"

@Michiel91
Copy link
Author

Hi @JanMarvin, thanks for the quick reply! Using the style_name together with swapping the order of wb_add_style and wb_add_conditional_formatting indeed solved the issue. I couldn't find this in the documentation or vignette, it appeared as if the style name was optional and the order of these two functions is arbitrary.

Coming from openxlsx where a style is added automatically when using 'conditionalFormatting', do you perhaps see a possibility to embed wb_add_style into the wb_add_conditional_formatting function and have it run automatically in the background?

@JanMarvin
Copy link
Owner

Hm, it is straight up there on top, but still using wb$styles_mgr$add(xml, "xml_name"). I will update this to use wb$add_style(xml, "xml_name").

wb <- wb_workbook()
negStyle <- create_dxfs_style(font_color = wb_color(hex = "FF9C0006"), bgFill = wb_color(hex = "FFFFC7CE"))
posStyle <- create_dxfs_style(font_color = wb_color(hex = "FF006100"), bgFill = wb_color(hex = "FFC6EFCE"))
wb$styles_mgr$add(negStyle, "negStyle")
wb$styles_mgr$add(posStyle, "posStyle")

Initially I had something like this in mind, where wb$add_style(xml) would be sufficient, to create a style with xml_name. In the end it did not like it enough. I want to avoid references to stylesObject, because this triggers some ptsd in me 😉 , but maybe wb_add_dxfs_style() similar to e.g. wb_add_fill() would complete our style wrappers.

@Michiel91
Copy link
Author

Thanks for improving the vignettes! I guess the styles_mgr instead of add_style caused me to miss it. I like your suggestion of wb_add_dxfs_style (and hope the word 'style' in there doesn't trigger the stylesObject PTSD 😜). I think having such a wrapper would be a great addition and reduces complexity.

Cheers,
Michiel

@JanMarvin
Copy link
Owner

#665 This should be sufficient. Needs an additional test, but should make it for 0.7.1 later next week.

@Michiel91
Copy link
Author

Awesome, thanks a lot for this nice addition!

@JanMarvin
Copy link
Owner

closed in 5e90271 🎉

@Michiel91
Copy link
Author

Hi @JanMarvin,

I might have been a bit too fast with replying everything works earlier today. Hopefully my thorough testing of openxlsx2 isn't giving you sleepless nights (or PTSD) yet... Just ran into what seems to be a downstream issue with conditional formatting, but I would like to double check with you before I open a new ticket.

Supplying a custom dxfs style containing an XML gradient fill works perfectly when not specifying a "type" in wb_add_conditional_formatting. However when using it in combination with for example type = "containsText" it breaks and returns Error: xml import unsuccessfull (minor spelling mistake btw, - last l 😉). This happens for all types. Other custom dxfs styles do work with all types. Could it be that something in the type argument doesn't yet recognize the recently implemented XML code?

@Michiel91
Copy link
Author

Please ignore my comment above. It does work, but I had to do a bit of tweaking to get it working. I was using containsText in combination with a string containing special characters. These required an additional escape character in the rule value. Not sure why it does work with the regular conditional formatting without a type, but I guess it's a fluke just like the example from this morning.

Cheers and thanks again for the implementation in #665 😉

@JanMarvin
Copy link
Owner

This might be something else. My debugging skills are best applied on code examples, but e.g. this works:

library(openxlsx2)

xml_fill_code2 <- openxlsx2::read_xml(
  glue::glue("
  <gradientFill degree=\"45\">
    <stop position=\"0\"><color rgb=\"35B779\"/></stop>
    <stop position=\"1\"><color rgb=\"FDE725\"/></stop>
  </gradientFill>
  "),
  pointer = FALSE)

formatting_style2 <- openxlsx2::create_dxfs_style(gradientFill = xml_fill_code2)

fn <- function(x) paste(sample(LETTERS, 10), collapse = "-")

wb <- wb_workbook()
wb$add_worksheet("containsText")
wb$add_data(x = sapply(1:10, fn))
wb$add_style(formatting_style2, "formatting_style2")
wb$add_conditional_formatting(
  cols = 1,
  rows = 1:10,
  type = "containsText",
  rule = "A",
  style = "formatting_style2"
)
Screenshot 2023-06-25 at 23 33 06

@Michiel91
Copy link
Author

Thanks for the debugging, it seems we just posted at the same time. I could also get it to work with simpler rules like "A" or "1" and then went on to see what was causing the other strings not to work. It turned out to be special characters also being wildcards in Excel.

@JanMarvin
Copy link
Owner

Typo is fixed in main, thanks. Could you give an example what "special characters also being wildcards in Excel." this means? Do we need to escape the rule?

@JanMarvin
Copy link
Owner

I might have a fix for the unescaped special characters #666

@Michiel91
Copy link
Author

Hi @JanMarvin,

I just tested your fix in #666 and this seems to work nicely for special characters like *, =, etc. Thanks again for the very fast implementation!

@JanMarvin
Copy link
Owner

Thanks, but * and = are not affected afaik. Only & ...

legal_chars <- function() { c("&", '"', "'", "<", ">", "\a", "\b", "\v", "\f") } # nolint
legal_sub <- function() { c("&amp;", "&quot;", "&apos;", "&lt;", "&gt;", "", "", "", "" ) } # nolint

Conditional formatting is one of the things I have spent literally days on to get it working and documenting and still things like this pass by, because I don't use it at all and no one else has reported them. Therefore all the reports help. If your unsure, open a discussion, so that the issues part is not bloated that much and creating minimal reproduceable examples is really helpful, otherwise I have to... 😄

@Michiel91
Copy link
Author

Hi @JanMarvin,

Not sure what exactly changed downstream, but the formatting at least now works when I escape special characters myself upfront. I think an interaction between these special characters and == or & not being escaped might have been causing the issues? From my side all good to go as the issues I mentioned in this ticket are resolved.

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

No branches or pull requests

2 participants