diff --git a/NEWS.md b/NEWS.md index c6bcb50..b7731e9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,10 @@ # patchwork (development version) +* Fixed `simplify_gt.gtable_patchwork()` indexing `widths` instead of + `heights` when computing the height of a nested patchwork, which threw + "index out of bounds" whenever every height in the nested layout was an + absolute unit + # patchwork 1.3.2 * Fixed a load-time bug that could throw spurious warnings diff --git a/R/plot_patchwork.R b/R/plot_patchwork.R index 1d4e180..5cbc3fe 100644 --- a/R/plot_patchwork.R +++ b/R/plot_patchwork.R @@ -404,7 +404,7 @@ simplify_gt.gtable_patchwork <- function(gt) { new_width <- unit(1, 'null') } if (all(is_abs_unit(gt$heights[panel_pos$t:panel_pos$b]))) { - new_height <- sum(convertHeight(gt$widths[panel_pos$t:panel_pos$b], 'mm')) + new_height <- sum(convertHeight(gt$heights[panel_pos$t:panel_pos$b], 'mm')) } else { new_height <- unit(1, 'null') } diff --git a/tests/testthat/test-layout.R b/tests/testthat/test-layout.R index 464a5b1..06a0d4c 100644 --- a/tests/testthat/test-layout.R +++ b/tests/testthat/test-layout.R @@ -101,3 +101,22 @@ test_that("various flavours of free() works", { free(free(p1 + p2 + p3 + p4, "label", "l"), "panel", "t") - p4 + p5 + plot_spacer() }) }) + +test_that("Nested patchworks with all-absolute-unit heights don't error", { + # simplify_gt.gtable_patchwork() used to index gt$widths instead of + # gt$heights when computing new_height, which threw "index out of bounds" + # as soon as a nested block's row count exceeded its column count - which + # happens whenever every height in the nested layout is an absolute unit + # (grid::unit(..., "cm")/"pt"/"in") rather than a relative one. + make_block <- function(n) { + wrap_elements(grid::textGrob(strrep("x\n", n))) + p1 + + plot_layout( + ncol = 1, + heights = grid::unit.c(grid::unit(n, "cm"), grid::unit(5, "cm")) + ) + } + + nested <- wrap_plots(list(make_block(1), make_block(3)), ncol = 1) + + expect_no_error(patchworkGrob(nested)) +})