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

wb_page_setup() argument fit_to_width does not apply scaling #965

Closed
asadow opened this issue Mar 7, 2024 · 4 comments
Closed

wb_page_setup() argument fit_to_width does not apply scaling #965

asadow opened this issue Mar 7, 2024 · 4 comments

Comments

@asadow
Copy link

asadow commented Mar 7, 2024

After running the below, I go to File, Print, and see that no scaling is applied. The solution to fit all columns on one page is to use scale = 80 instead. But I was expecting fit_to_width = TRUE to work.

library(openxlsx2)
library(glue)
.data <- structure(list(`Bldg/Eqp` = "5", `WO Num` = "123421", type = "BE", 
                        category = "", 
                        Description = "I am a description..............", 
                        date = "02-22-24", status = "S", est_time = 1), 
                   row.names = c(NA, -1L), 
                   class = c("tbl_df", "tbl", "data.frame"))


wb_backlog <- function(.data) {
  nm <- names(.data)
  letter_desc <- letters[which(nm == "Description")] |> toupper()
  letter_last <- letters[ncol(.data)] |> toupper()
  
  wb_workbook() |> 
    wb_add_worksheet("Sheet1") |> 
    wb_add_data("Sheet1", .data) |> 
    wb_add_cell_style(
      dims = glue("{letter_desc}2:{letter_desc}{nrow(.data) + 1}"),
      wrap_text = "1"
    ) |> 
    wb_set_col_widths(cols = which(nm == "Description"), widths = 75) |> 
    wb_add_filter(rows = 1, cols = which(nm != "Description")) |> 
    wb_add_border(
      dims = glue("A1:{letter_last}1"),
      bottom_color = wb_color(theme = 1), bottom_border = "thin",
      top_color = wb_color(theme = 1), top_border = "double",
      left_border = NULL, right_border = NULL
    ) |> 
    wb_page_setup(orientation = "landscape", fit_to_width = TRUE)
}

.data |> wb_backlog() |> wb_open()
#> Warning: will not open file when not interactive

Created on 2024-03-07 with reprex v2.0.2

@JanMarvin
Copy link
Owner

JanMarvin commented Mar 7, 2024

Hi @asadow ,

thanks for the report. I could be that we are simply assigning the page setup xml string for fit to page to the wrong worksheet argument.

openxlsx2/R/class-workbook.R

Lines 6118 to 6127 in fce092f

## Update ----
self$worksheets[[sheet]]$pageSetup <- sprintf(
'<pageSetup paperSize="%s" orientation="%s" scale = "%s" fitToWidth="%s" fitToHeight="%s" horizontalDpi="%s" verticalDpi="%s"/>',
paper_size, orientation, scale, as_xml_attr(fit_to_width), as_xml_attr(fit_to_height), hdpi, vdpi
)
if (fit_to_height || fit_to_width) {
self$worksheets[[sheet]]$sheetPr <- unique(c(self$worksheets[[sheet]]$sheetPr, '<pageSetupPr fitToPage="1"/>'))
}

Edit: on second thought, that does not seem to be the case. Will look into.

@JanMarvin
Copy link
Owner

I had a look unfortunately it does not look like there is something really going wrong, therefore we might have to look for another solution. The documentation for scale states that:

This setting is overridden when fitToWidth and/or fitToHeight are in use.
-- ECMA-376 Part 1 (p. 1670)

Unfortunately it look like either I do not understand this correctly or it is wrong. Setting fitToWidth and fitToHeight only seem to affect the number of pages preselected in the xlsx gui. Any scaling has to be applied manually. For the use case above MS365 calculates a scale of 90%. Probably we have to match this. While this might be rather straightforward for column case, it might be tricky for the row case and for a combination of both. It would be great if someone could provide us with some scaling values for maybe the palmerpenguins dataset..

In addition there is a typo and maybe a thinko in wb$worksheets[[1]]$sheetPr. When loading we read the full xml into this field, but we allow the sheetPR tag to be absent. In the wb_page_setup function we simply assume that this tag is missing, which obviously will break if it isn't. *sigh*

Therefore it looks like wb_page_setup() might require a general overhaul, in the meantime the following should provide a scaling factor:

library(openxlsx2)
library(glue)
.data <- structure(list(`Bldg/Eqp` = "5", `WO Num` = "123421", type = "BE", 
                        category = "", 
                        Description = "I am a description..............", 
                        date = "02-22-24", status = "S", est_time = 1), 
                   row.names = c(NA, -1L), 
                   class = c("tbl_df", "tbl", "data.frame"))


wb_backlog <- function(.data) {
  nm <- names(.data)
  letter_desc <- letters[which(nm == "Description")] |> toupper()
  letter_last <- letters[ncol(.data)] |> toupper()
  
  wb_workbook() |> 
    wb_add_worksheet("Sheet1") |> 
    wb_add_data("Sheet1", .data) |> 
    wb_add_cell_style(
      dims = glue("{letter_desc}2:{letter_desc}{nrow(.data) + 1}"),
      wrap_text = "1"
    ) |> 
    wb_set_col_widths(cols = which(nm == "Description"), widths = 75) |> 
    wb_add_filter(rows = 1, cols = which(nm != "Description")) |> 
    wb_add_border(
      dims = glue("A1:{letter_last}1"),
      bottom_color = wb_color(theme = 1), bottom_border = "thin",
      top_color = wb_color(theme = 1), top_border = "double",
      left_border = NULL, right_border = NULL
    ) |> 
    wb_page_setup(orientation = "landscape", fit_to_width = TRUE)
}

wb <- .data |> wb_backlog()

# get the scaling factor (or get some scaling factor?)
cols <- wb$worksheets[[1]]$unfold_cols()
num_pages <- 1
col_width <- sum(as.numeric(cols$width)) / 100
scale <- round(num_pages/col_width * 100, digits = -1)

# apply scaling factor
wb <- wb |> wb_page_setup(orientation = "landscape", scale = scale, fit_to_width = num_pages)
# fix a typo "pageSetupPr != pageSetUpPr"
wb$worksheets[[1]]$sheetPr <- "<sheetPr><pageSetUpPr fitToPage=\"1\"/></sheetPr>"

if (interactive()) wb$open()

@JanMarvin
Copy link
Owner

Unfortunately the logic to create scaling is a bit more complex than I initially thought. It works in the example code above, but this is merely a fluke. I still intent to overhaul the wb_page_setup() code in #966, but scaling will still require manual adding values.

JanMarvin added a commit that referenced this issue Mar 14, 2024
* [pugi] old idea from #510

* [wb_page_setup] rework the function. In the future wb_set_page_setup() could be used

* another simpler approach

* Document that fit_to_{height,width} does not scale. #965

* typos and cleanup
@JanMarvin
Copy link
Owner

#966 is merged (wb$page_setup() now uses the new wb$set_page_style() function that allows a few new page style options, fixes a bug where setting a page setup to a file that was created with a tab color would result in broken xlsx files).

The following is now possible (I've replaced the glue code with @olivroy's wb_dims() magic):

library(openxlsx2)
.data <- structure(
  list(
    `Bldg/Eqp` = "5",
    `WO Num` = "123421",
    type = "BE", 
    category = "", 
    Description = "I am a description..............", 
    date = "02-22-24",
    status = "S",
    est_time = 1
  ), 
  row.names = c(NA, -1L), 
  class = c("tbl_df", "tbl", "data.frame")
)


wb_backlog <- function(.data) {
  nm <- names(.data)
  
  wb_workbook() |> 
    wb_add_worksheet("Sheet1") |> 
    wb_add_data("Sheet1", .data) |> 
    wb_add_cell_style(
      dims = wb_dims(x = .data, cols = "Description"),
      wrap_text = "1"
    ) |> 
    wb_set_col_widths(cols = which(nm == "Description"), widths = 75) |> 
    wb_add_filter(rows = 1, cols = which(nm != "Description")) |> 
    wb_add_border(
      dims = wb_dims(x = .data),
      bottom_color = wb_color(theme = 1), bottom_border = "thin",
      top_color = wb_color(theme = 1), top_border = "double",
      left_border = NULL, right_border = NULL
    ) |> 
    wb_page_setup(orientation = "landscape", scale = 90, fit_to_width = 1)
}

wb <- .data |> wb_backlog()

if (interactive()) wb$open()

As written previously, automatic scaling is not possible (the openxml documentation was a bit misleading in this regard). Excel apparently uses some GDI+ logic to scale a page. I haven't figured how to mimic this, therefore it's currently not something left to the user to figure out.

Currently there is no wrapper wb_set_page_setup(). I want to deprecate wb_page_setup() in the future, because it breaks our function names and while we could add new arguments to the existing function, I'd prefer a clean start with a better working function. Though, I haven't had much time to test this.

The page setup function is also not something I have any real use case for, therefore I'm a bit reluctant to invest much more time into this. If you are interested, please go ahead and test a few things and please report if they are working or if the implemented logic checks out. The missing real life reason is also why #929 is not yet a feature of the package.

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