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 add style across #873

Merged
merged 8 commits into from
Dec 17, 2023
Merged

Wb add style across #873

merged 8 commits into from
Dec 17, 2023

Conversation

JanMarvin
Copy link
Owner

@JanMarvin JanMarvin commented Dec 12, 2023

library(openxlsx2)

wb <- wb_workbook() %>%
  wb_add_worksheet() %>%
  wb_set_col_widths(cols = "A:B", width = 10) %>% 
  wb_add_fill(dims = "C3", color = wb_color("yellow"))%>% 
  wb_set_cell_style_across(style = "C3", cols = "C:D", rows = 3:4)

if (interactive()) wb %>% wb_open()

@JanMarvin
Copy link
Owner Author

Could you have a look at the API @olivroy ? I'm not entirely happy with from, but we've used it at other places and dims seemed misleading. Maybe you have a better idea.

@olivroy
Copy link
Collaborator

olivroy commented Dec 12, 2023

@JanMarvin maybe dims_ref?

If I understand correctly (I checked very quickly), it is that we take the style from a reference cell, and apply it to rows and cols?

wb_copy_style(dims_ref = "C3", rows = 1:4)? similar to wb_copy_cells?

If not, I will take my time to think about it a bit more.

@JanMarvin
Copy link
Owner Author

Thanks! Yes precisely, pick a style from a cell and apply it columns-wise/row-wise. I avoided new arguments, but it looks like a good idea.

@JanMarvin
Copy link
Owner Author

I guess I rename and rework this function slightly. It might be helpful to rename this function to wb_set_cell_style_across() and make it behave similar to wb_set_cell_style(). This would reduce confusing functions in wb_add_style()/wb_add_style_across() which behave differently and do different things.

@olivroy
Copy link
Collaborator

olivroy commented Dec 13, 2023

@JanMarvin, maybe just add an argument to wb_set_cell_style(). like the user should supply either style or dims_ref. or style could accept a cell
wb_set_cell_style(dims = wb_dims(rows = 2:4, cols = 5:10), copy_style_from = "A1")

@JanMarvin
Copy link
Owner Author

@olivroy I changed the style argument of wb_set_cell_style() to accept both a style id as before and a cell, and the newly added wb_set_cell_style_across() function behaves similarly. I don't think there is much demand for this kind of cell styles and now we have everything in one place and the functions behave similarly, so users who were using the old function should have no problem understanding the new one.

I appreciate your input as always, but in the end I decided against dims_ref and I'm not a big fan of the very long arguments you suggested in your last comment, even though I see the benefits they bring.

@JanMarvin JanMarvin merged commit 5435941 into main Dec 17, 2023
9 checks passed
@JanMarvin JanMarvin deleted the wb_add_style_across branch December 17, 2023 22:48
@olivroy
Copy link
Collaborator

olivroy commented Dec 18, 2023

Thanks for your work! I think it is a great addition to use a cell as reference to style across the workbook!

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.

2 participants