fix: layout_properties() returns all phs for multiple masters (close #597)

[markheckmann]

Closes #597

• layout_properties() now returns all placeholders in case of multiple master.
• Also, the internal xfrmize() now sorts the resulting data by placeholder position. This yields an intuitive order, with placeholders sorted from top to bottom and left to right.

@davidgohel Please find a question for you at the end. Sorry, this will be quite long ...

---

EXAMPLE

url <- "https://github.com/user-attachments/files/16829213/three_identical_masters.pptx"
file <- tempfile(fileext = ".pptx")
download.file(url, file)
x <- read_pptx(file)
df <- layout_properties(x)
df[, c("master_name", "name", "type", "ph_label", "ph")]

Before

   master_name        name     type                   ph_label                                          ph
1     Master_1 Title Slide       dt         Date Placeholder 3        <p:ph type="dt" sz="half" idx="10"/>
2     Master_1 Title Slide      ftr       Footer Placeholder 4    <p:ph type="ftr" sz="quarter" idx="11"/>
3     Master_1 Title Slide   sldNum Slide Number Placeholder 5 <p:ph type="sldNum" sz="quarter" idx="12"/>
4     Master_1 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
5     Master_1 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>
6     Master_2 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
7     Master_2 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>
11    Master_3 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
12    Master_3 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>

Now

Shows all phs and has a more natural sorting by ph position.

   master_name        name     type                   ph_label                                          ph
1     Master_1 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
2     Master_1 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>
3     Master_1 Title Slide       dt         Date Placeholder 3        <p:ph type="dt" sz="half" idx="10"/>
4     Master_1 Title Slide      ftr       Footer Placeholder 4    <p:ph type="ftr" sz="quarter" idx="11"/>
5     Master_1 Title Slide   sldNum Slide Number Placeholder 5 <p:ph type="sldNum" sz="quarter" idx="12"/>
6     Master_2 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
7     Master_2 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>
8     Master_2 Title Slide       dt         Date Placeholder 3        <p:ph type="dt" sz="half" idx="10"/>
9     Master_2 Title Slide      ftr       Footer Placeholder 4    <p:ph type="ftr" sz="quarter" idx="11"/>
10    Master_2 Title Slide   sldNum Slide Number Placeholder 5 <p:ph type="sldNum" sz="quarter" idx="12"/>
11    Master_3 Title Slide ctrTitle                    Title 1                     <p:ph type="ctrTitle"/>
12    Master_3 Title Slide subTitle                 Subtitle 2             <p:ph type="subTitle" idx="1"/>
13    Master_3 Title Slide       dt         Date Placeholder 3        <p:ph type="dt" sz="half" idx="10"/>
14    Master_3 Title Slide      ftr       Footer Placeholder 4    <p:ph type="ftr" sz="quarter" idx="11"/>
15    Master_3 Title Slide   sldNum Slide Number Placeholder 5 <p:ph type="sldNum" sz="quarter" idx="12"/>

This looks fine to me, but ...

---

NOTE

The test snapshot plot-twocontent-layout-nolabel.png had to be updated. I have some doubts about the correctness, however. It was altered in the following three commits:

git log --oneline -- tests/testthat/_snaps/pptx-info/plot-twocontent-layout-nolabel.png

4c83218 (origin/issue_595_plot_layout_properties_improvements) tests: fix test snapshots for plot_layout_properties()
9f9148f tests: fix plot snapshot
321e5a6 tests: complete with few tests

Before

From 321e5a6

From 9f9148f

Now

It appears the snapped layout changed from "Two Content" to "Title Slide" in 9f9148f. The name of the snap, however, does not yet reflect this. Relevant code:

png2 <- tempfile(fileext = ".png")
png(png2, width = 7, height = 6, res = 150, units = "in")
plot_layout_properties(
    x = x, layout = "Title Slide",
    master = "Office Theme",
    labels = FALSE
)
dev.off()
expect_snapshot_doc(name = "plot-twocontent-layout-nolabel", x = png2, engine = "testthat")

@davidgohel :

• The ph types in the plots appear not have been correctly assigned before (ctrTitle in the bottom ph etc.), but seem okay now.
• I would suggest to also change the snapshot names for consistency.
• However, I am unsure about the id=1 part through all snaps, as I do not quite understand the goal of these lines in plot_layout_properties(). Is id=1 what is always expected?

Please let me know how to proceed from here :)

Cheers
Mark

[davidgohel]

The ph types in the plots appear not have been correctly assigned before (ctrTitle in the bottom ph etc.), but seem okay now.

I agree, probably an error I did because I worked too late. It seems you fixed it, another "many thanks"! So let's keep your work as it is.

---

I would suggest to also change the snapshot names for consistency.

Yes, totally, I did that mistake, but I really think it was not intentional. Let's change the snapshot name then.

---

Is id=1 what is always expected?

No, see with the following example:

x <- read_pptx()
plot_layout_properties(
  x = x, layout = "Two Content",
  master = "Office Theme",
  labels = FALSE
)

I do not quite understand the goal of these lines

    # order by id : these id are coming from MS powerpoint, we have no control on their values as they are set by PowerPoint.
    # they are incremented starting from 2 (I don't know why)
    labels <- dat$type[order(as.integer(dat$id))]
    # the following two lines create OUR id (used by ph_location_type for example)... Sorry for the same name than the one above, it's probably what we can call a design error 
    rle_ <- rle(labels)
    labels <- sprintf("type: '%s' - id: %.0f", labels, unlist(lapply(rle_$lengths, seq_len)))

I think the code is fine as it is actually. We want to see the id that we can use with ph_location_type(id = ...).

[davidgohel]

thanks @markheckmann, it looks good to me