feat: leverage unused dots in `ph_with.*` methods for modifying location / placeholder object
Currently, the ... in ph_with.* methods are unused. We might use them to modify the location / placeholder object as in the example below. This would allow for a wide yet simple approach to achieve range of instant modifications.
@davidgohel Feel free to assign me the issue, if you agree with it :)
Example:
read_pptx() |>
add_slide("Two Content") |>
ph_with(location = "title", "The Title", ln = "red", rot = 2, geom = "horizontalScroll", bg = "#ff000015") |>
ph_with(location = "body[1]", "Body 1", geom = "star32", bg = "orange") |>
ph_with(location = "body[2]", letters[1:3], geom = "roundRect", bg = "grey95") |>
print(preview = TRUE)
This is an interesting approach!
I'm wondering if users might find it confusing to understand which parameters correspond to which function. Maybe we may need a piece of documentation that shows which parameters map to which function (I can help...)
Yes, that would be great if you like to help with that. :)
TODO
- [ ] Tests
- [ ] Documentation
- [ ] handle width and height for class
gg(plot_instr)
Notes
Using the dots is mostly unproblemetic as they are not used by most ph_with.* methods. Only two methods, for ggplot and plot_instr, pass them on to ragg::agg_png. This required some special handling to prevent agg_png() from throwing an error.
Examples
Version used for output below: devtools::install_github("markheckmann/officer", "aec62c3")
Run this gist for the two following examples like so:
source("https://gist.githubusercontent.com/markheckmann/a2b01b2906ea25c89f9835bcf5ad9e1c/raw/officer_issue_681_examples.R")
- Showing all
ph_with.*methods:
- Focusing
gg,plot_instr,ext_image:
As far as I can tell, the ph_with.* methods can process the following dot args:
- data.frame:
top,left,width,height, ~rotation~, ~ln~, ~bg~, ~geom~ - gg, plot_instr, external_img:
top,left,width,height,rotation,ln,bg, ~geom~ - all others:
top,left,width,height,rotation,ln,bg,geom
@davidgohel Does that seem correct to you?
I gave the docs the follwing first shot. Please feel free to suggest improvements to it:
@davidgohel , I was wondering if ph_location (i.e. its methods) should process the dots in the same way. After all, the dots in ph_with only modify the location object.
What do you think?
UPDATE: I thought about the following again and now tend to refrain from processing the dots in ph_location objects in the same way. It feels incoherent, at least to pass the args geom, ln, and bg.
As a result, the user would have two options to achieve the same result:
loc <- ph_location_label("Content Placeholder 2", rotation = 10, bg = "#ff000015")
layout <- "Title and Content"
x <- read_pptx() |>
# via ... in location
add_slide(layout, title = "via ... in location") |>
ph_with(ext_img, loc, use_loc_size = FALSE) |>
# via ... in ph_with
add_slide(layout, title = "via ... in ph_with") |>
ph_with(ext_img, "body", use_loc_size = FALSE, rotation = 10, bg = "#0000ff15") |>
print(preview = TRUE)
Hi
thanks. I have started to have a look at your branch... The implementation is elegant and well-structured.
I found one bug that should be fixed
ph_with.numeric <- function(x, value, location, format_fun = format, ..., .dots = .dots) {
slide <- x$slide$get_slide(x$cursor)
value <- format_fun(value, ...) # ... passed to format_fun
location <- fortify_location(location, doc = x)
location <- update_location_from_dots(location, ..., .dots = .dots) # same ... used again
I suggest something like if you don't have another solution in mind:
dots <- modifyList(list(...), .dots %||% list())
location_args <- c("left", "top", "width", "height", "ph_label", "ln", "bg", "rotation", "geom")
format_args <- dots[!names(dots) %in% location_args]
David
@davidgohel
-
Thanks for the hint. I will add this. Note, however, that the
widtharg is also part of fhe signature offormatand will be dropped, i.e. not passed toformat. But this is the tradeoff we have to make, I guess, when passing the dots to more than one functions downstream. -
Regarding the use of dots in
ph_location_*: are you for or against it? You gave my post a ❤️ but my latest UPDATE recommendatation was to refrain from implementing it, so I am a bit unsure what your take is here.
- you are right and I agree with your tradeoff argument (I don't think however ph_with.numeric is that used...)
- Misusage of imojs on my side :) I am not against.
I will have a deeper look tomorrow and give you feedback
@davidgohel Okay, then I will also make ph_location.* take the modifiers via the dots. :)