bug: plot_layout_properties() labeling still incorrect for labels= FALSE

[markheckmann]

The assignments to the phs in plot_layout_properties(..., labels = FALSE) are (sometimes) still incorrect.

This bug has been around longer and was supposed to be fixed in PR #600, but this appears not to be the case. The problem went unnoticed, because the tests do not cover problematic cases.

Bug

The file four_content.pptx has one layout called Four Content with four body placeholders showing the problem. The bug exists in both, the CRAN and the dev version, although with differences.

layout_properties(x, "Four Content")

   master_name         name   type id                   ph_label   ...
1 Office Theme Four Content  title  2                    Title 1
2 Office Theme Four Content   body 10      Content Placeholder 1
3 Office Theme Four Content   body  9      Content Placeholder 2
4 Office Theme Four Content   body  3      Content Placeholder 3
5 Office Theme Four Content   body  4      Content Placeholder 4
6 Office Theme Four Content     dt  5         Date Placeholder 4
7 Office Theme Four Content    ftr  6       Footer Placeholder 5
8 Office Theme Four Content sldNum  7 Slide Number Placeholder 6

CRAN-Version (0.6.6)

DEV-Version

Analysis

The reason appears to lie in these lines from pptx_informations.R (see below): The labels are ordered and adjusted without reordering the other columns of dat, resulting in a mismatch.

labels <- dat$type[order(as.integer(dat$id))]
rle_ <- rle(labels)
labels <- sprintf("type: '%s' - id: %.0f", labels, unlist(lapply(rle_$lengths, seq_len)))

...

# later, this results in a mismatch between labels and the other columns (offx, cx, etc.) from dat
text(x = offx + cx/2, y = -(offy + cy/2), labels = labels, cex = 0.5, col = "red")

Additional UX note

Filling a ph using ph_location_type() works correctly (for CRAN and Dev). However, assigning the type id in the order of the internal placeholder id does not guarantee an intuitive order as shown in the image below for type body (the four main content phs).

for (id in 1:4) {
  x <- ph_with(x, paste("type = body, id =", id), ph_location_type("body", id = id))
}

Suggestions

1. Quick fix for incorrect assignments (I will submit a PR).
2. Refactoring regarding UX:
   • Add type id assignment to layout_properties() output, so it is transparent for the user.
   • Usage of a more intuitive type id ordering, for example, by position (top->bottom, left->right) as it is already the case in layout_properties() as of PR fix: layout_properties() returns all phs for multiple masters (close #597) #600
   • make usage of the term id consistent across functions layout_properties(), plot_layout_properties(), and ph_location_type().

@davidgohel, if okay for you, I will open an issue and make a suggestion.

[markheckmann]

The last suggestion has got its own issue now: #606

[markheckmann]

As of PR #608 , plot_layout_properties()'s labels argument has a different meaning, making this issue obsolete.