diff --git a/R/export_tfl.R b/R/export_tfl.R index b74452e..ace8aae 100644 --- a/R/export_tfl.R +++ b/R/export_tfl.R @@ -77,9 +77,10 @@ #' #' In preview mode each page is drawn via `grid::grid.newpage()` (so knitr #' captures it as an inline graphic). Returns `NULL` invisibly. -#' @param ... Additional arguments passed to [writetfl::export_tfl_page()]. -#' These serve as defaults for all pages and are overridden by per-page -#' list elements in `x`. +#' @inheritDotParams export_tfl_page -x -page_i -preview +#' @details +#' Arguments forwarded via `...` serve as defaults for all pages and are +#' overridden by per-page list elements in `x`. #' #' @return #' - Normal mode (`preview = FALSE`): the normalized absolute path to the PDF diff --git a/R/export_tfl_page.R b/R/export_tfl_page.R index 4b5074f..4d184de 100644 --- a/R/export_tfl_page.R +++ b/R/export_tfl_page.R @@ -64,6 +64,22 @@ #' @param min_content_height Minimum acceptable content area height as a `unit` #' object. An error is raised if the computed content height falls below this #' value. +#' @param overflow_action One of `"error"` (default) or `"warn"`. Controls how +#' width-overflow conditions are reported when the content does not fit in +#' its allocated area: +#' - `"error"`: append the message to the layout-error vector and abort +#' before drawing (no PDF page is produced). +#' - `"warn"`: emit `rlang::warn()` and continue rendering. The PDF is +#' produced with the overflow visibly clipped by `grid`, which is useful +#' for diagnosing what is too wide. See issue #30. +#' +#' The same setting applies to all width-overflow detections: the +#' page-level content grob check (any `grob` content wider than the +#' content viewport), the [tfl_table()] total-width check (when +#' `allow_col_split = FALSE` and the column total still exceeds the +#' page after wrapping), and the [tfl_table()] per-column check (any +#' single column — or any data column combined with the row-header +#' group columns — wider than the page). #' @param page_i Integer page index, used to prefix layout error messages with #' `"Page : "`. Set automatically by [writetfl::export_tfl()]; #' not normally supplied when calling this function directly. @@ -101,6 +117,7 @@ export_tfl_page <- function( content_just = "left", margins = grid::unit(c(t = 0.5, r = 0.5, b = 0.5, l = 0.5), "inches"), min_content_height = grid::unit(3, "inches"), + overflow_action = c("error", "warn"), page_i = NULL, preview = FALSE, ... @@ -137,6 +154,7 @@ export_tfl_page <- function( content_just <- resolve_from_x(content_just, "content_just") padding <- resolve_from_x(padding, "padding") min_content_height <- resolve_from_x(min_content_height, "min_content_height") + overflow_action <- resolve_from_x(overflow_action, "overflow_action") # --------------------------------------------------------------------------- # 1c. Validate resolved inputs @@ -144,9 +162,10 @@ export_tfl_page <- function( checkmate::assert_class(padding, "unit", .var.name = "padding") checkmate::assert_class(margins, "unit", .var.name = "margins") checkmate::assert_class(min_content_height, "unit", .var.name = "min_content_height") - caption_just <- match.arg(caption_just, c("left", "right", "centre")) - footnote_just <- match.arg(footnote_just, c("left", "right", "centre")) - content_just <- match.arg(content_just, c("left", "right", "centre")) + caption_just <- match.arg(caption_just, c("left", "right", "centre")) + footnote_just <- match.arg(footnote_just, c("left", "right", "centre")) + content_just <- match.arg(content_just, c("left", "right", "centre")) + overflow_action <- match.arg(overflow_action, c("error", "warn")) # --------------------------------------------------------------------------- # 2. Normalize all text and rule inputs @@ -259,6 +278,29 @@ export_tfl_page <- function( content_h_in <- compute_content_height(vp_height_in, section_heights, present, padding_in) errors <- check_content_height(content_h_in, min_content_height, errors) + # Page-level width overflow check for non-ggplot, non-character content. + # ggplot scales to fit and character content is word-wrapped, so neither + # produces width overflow. Other grobs (gt::as_gtable, rtables textGrob, + # gridExtra::tableGrob, raw user grobs) have a natural width that + # grobWidth() can measure while outer_vp is active. + # + # tfl_table_grobs are skipped here because compute_col_widths() already + # ran a more precise per-column / group-aware overflow check during + # tfl_table_to_pagelist(). Re-checking the assembled grob would emit a + # duplicate (less informative) warning under overflow_action = "warn". + if (inherits(x$content, "grob") && + !inherits(x$content, "ggplot") && + !inherits(x$content, "tfl_table_grob")) { + content_w_in <- tryCatch( + .width_in(grid::grobWidth(x$content)), + error = function(e) NA_real_ + ) + if (is.finite(content_w_in)) { + errors <- check_content_width(content_w_in, vp_width_in, overflow_action, + errors, what = "Content") + } + } + if (length(errors) > 0) { grid::popViewport() page_prefix <- if (!is.null(page_i)) paste0("Page ", page_i, ": ") else "" diff --git a/R/layout.R b/R/layout.R index 6c3d3ac..c159676 100644 --- a/R/layout.R +++ b/R/layout.R @@ -1,4 +1,4 @@ -# layout.R — Content height computation and validation +# layout.R — Content height/width computation and validation #' Compute available content height after subtracting all other sections #' @@ -39,3 +39,48 @@ check_content_height <- function(content_h_in, min_content_height, errors) { } errors } + +# Shared dispatch for width-overflow events. Either appends `msg` to `errors` +# (when `overflow_action == "error"`) or emits an immediate rlang::warn() (when +# `"warn"`) and returns `errors` unchanged. Every overflow message ends with +# the diagnostic-mode hint so users always see the escape hatch. +.overflow_signal <- function(msg, overflow_action, errors) { + msg <- paste0( + msg, + "\n Set `overflow_action = \"warn\"` to convert this error to a ", + "warning and still produce output for diagnosis." + ) + if (identical(overflow_action, "warn")) { + rlang::warn(msg) + errors + } else { + c(errors, msg) + } +} + +#' Check content width against an upper bound and signal if too wide +#' +#' Mirrors [check_content_height()] but is a maximum-ceiling check rather than +#' a minimum-floor check, and accepts an `overflow_action` knob that downgrades +#' the error to a warning so output can still be produced for diagnosis (see +#' issue #30). +#' +#' @param content_w_in Natural width of the content in inches. +#' @param vp_width_in Available content viewport width in inches. +#' @param overflow_action One of `"error"` (default) or `"warn"`. +#' @param errors Character vector to append to (when action is `"error"`). +#' @param what Label for the source of the width (e.g. `"Content"`, +#' `"Column 'x'"`, `"Total column width"`). +#' @return Updated `errors` character vector. +#' @keywords internal +check_content_width <- function(content_w_in, vp_width_in, overflow_action, + errors, what = "Content") { + if (content_w_in > vp_width_in + 1e-6) { + msg <- sprintf( + "%s width (%.4g in) exceeds available content width (%.4g in)", + what, content_w_in, vp_width_in + ) + errors <- .overflow_signal(msg, overflow_action, errors) + } + errors +} diff --git a/R/table_columns.R b/R/table_columns.R index 75442f7..d04ed48 100644 --- a/R/table_columns.R +++ b/R/table_columns.R @@ -67,11 +67,20 @@ resolve_col_specs <- function(tbl) { #' Compute final column widths and column groups #' +#' @param overflow_action One of `"error"` (default) or `"warn"`. Controls how +#' width-overflow conditions are reported. See [export_tfl_page()]. +#' @param validate_overflow Logical (internal). When `FALSE`, skip the +#' per-column / group-aware / total-width overflow checks. The second +#' `cw_adj` pass in `.tfl_table_to_pagelist_default()` sets this to `FALSE` +#' so the same overflow is not re-signalled on every pass. #' @return A list with `$resolved_cols` (widths_in filled in) and #' `$col_groups` (list of integer vectors of column indices per group). #' @keywords internal compute_col_widths <- function(resolved_cols, data, content_width_in, - tbl, pg_width, pg_height, margins) { + tbl, pg_width, pg_height, margins, + overflow_action = c("error", "warn"), + validate_overflow = TRUE) { + overflow_action <- match.arg(overflow_action) n_cols <- length(resolved_cols) n_grp <- length(tbl$group_vars) min_in <- .width_in(tbl$min_col_width) @@ -163,20 +172,91 @@ compute_col_widths <- function(resolved_cols, data, content_width_in, } # --- Check feasibility --- - if (total_w > content_width_in + 1e-6) { - if (!tbl$allow_col_split) { - col_detail <- paste(vapply(seq_len(n_cols), function(j) { - sprintf(" %s: %.3g in", resolved_cols[[j]]$col, widths_in[[j]]) - }, character(1L)), collapse = "\n") - rlang::abort(sprintf(paste0( - "Total column width (%.3g in) exceeds available content width (%.3g in) ", - "after wrapping.\nColumn widths:\n%s\n", - "Set `allow_col_split = TRUE` to split columns across pages, ", - "or reduce column widths / enable wrap_cols." - ), total_w, content_width_in, col_detail)) + errors <- character(0) + + if (!validate_overflow) { + # Skip overflow validation entirely. The caller (typically the second + # cw_adj pass in .tfl_table_to_pagelist_default) is recomputing widths + # for layout reasons after a prior pass already validated the same + # configuration; re-signalling here would emit a duplicate warning. + resolved_cols <- lapply(seq_len(n_cols), function(j) { + cs <- resolved_cols[[j]] + cs$width_in <- widths_in[[j]] + cs + }) + col_groups <- paginate_cols(widths_in, content_width_in, n_grp, + tbl$allow_col_split, tbl$balance_col_pages) + return(list(resolved_cols = resolved_cols, + col_groups = col_groups, + col_cont_label_half_w = col_cont_label_half_w)) + } + + # Per-column / group-aware overflow check. Group columns repeat on every + # column-paginated page, so the available width for any single data column + # is content_width_in - grp_w. A group column itself must fit in the full + # content width (grp_w == 0 if there are no group columns, in which case the + # data-col rule reduces to `widths_in[j] > content_width_in`). + grp_w <- if (n_grp > 0L) sum(widths_in[seq_len(n_grp)]) else 0 + for (j in seq_len(n_cols)) { + cs <- resolved_cols[[j]] + if (j <= n_grp) { + # Group column j: must fit in content_width_in alone + if (widths_in[[j]] > content_width_in + 1e-6) { + errors <- .overflow_signal( + sprintf( + paste0("Group column '%s' width (%.3g in) exceeds available ", + "content width (%.3g in)"), + cs$col, widths_in[[j]], content_width_in + ), + overflow_action, errors + ) + } + } else { + # Data column j: must fit alongside the group columns on a single page. + # Use a tiny tolerance and avoid double-reporting when n_grp == 0 and + # the same overflow would also be caught by the (commented) total check. + if (grp_w + widths_in[[j]] > content_width_in + 1e-6) { + if (n_grp > 0L) { + msg <- sprintf( + paste0("Column '%s' (%.3g in) plus group columns (%.3g in) ", + "= %.3g in exceeds available content width (%.3g in); ", + "no column-paginated page can fit this column with the ", + "row headers"), + cs$col, widths_in[[j]], grp_w, + grp_w + widths_in[[j]], content_width_in + ) + } else { + msg <- sprintf( + paste0("Column '%s' width (%.3g in) exceeds available content ", + "width (%.3g in)"), + cs$col, widths_in[[j]], content_width_in + ) + } + errors <- .overflow_signal(msg, overflow_action, errors) + } } } + # Total-width check: only meaningful when allow_col_split = FALSE. When + # allow_col_split = TRUE, paginate_cols() handles the multi-page split and + # this is not an overflow event. + if (total_w > content_width_in + 1e-6 && !tbl$allow_col_split) { + col_detail <- paste(vapply(seq_len(n_cols), function(j) { + sprintf(" %s: %.3g in", resolved_cols[[j]]$col, widths_in[[j]]) + }, character(1L)), collapse = "\n") + msg <- sprintf(paste0( + "Total column width (%.3g in) exceeds available content width (%.3g in) ", + "after wrapping.\nColumn widths:\n%s\n", + "Set `allow_col_split = TRUE` to split columns across pages, ", + "or reduce column widths / enable wrap_cols." + ), total_w, content_width_in, col_detail) + errors <- .overflow_signal(msg, overflow_action, errors) + } + + if (length(errors) > 0L) { + rlang::abort(paste(errors, collapse = "\n")) + } + # --- Store final widths in resolved_cols --- resolved_cols <- lapply(seq_len(n_cols), function(j) { cs <- resolved_cols[[j]] diff --git a/R/table_pagelist.R b/R/table_pagelist.R index 8b39924..f080273 100644 --- a/R/table_pagelist.R +++ b/R/table_pagelist.R @@ -13,13 +13,14 @@ # Default values mirroring export_tfl_page() for use when dots are absent .tfl_page_defaults <- list( - margins = grid::unit(c(t = 0.5, r = 0.5, b = 0.5, l = 0.5), "inches"), - padding = grid::unit(0.5, "lines"), - header_rule = FALSE, - footer_rule = FALSE, - caption_just = "left", - footnote_just = "left", - gp = grid::gpar() + margins = grid::unit(c(t = 0.5, r = 0.5, b = 0.5, l = 0.5), "inches"), + padding = grid::unit(0.5, "lines"), + header_rule = FALSE, + footer_rule = FALSE, + caption_just = "left", + footnote_just = "left", + gp = grid::gpar(), + overflow_action = "error" ) # --------------------------------------------------------------------------- @@ -96,13 +97,15 @@ tfl_table_to_pagelist <- function(tbl, pg_width, pg_height, dots, .dot <- function(key) { if (!is.null(dots[[key]])) dots[[key]] else .tfl_page_defaults[[key]] } - margins <- .dot("margins") - padding <- .dot("padding") - header_rule <- .dot("header_rule") - footer_rule <- .dot("footer_rule") - cap_just <- .dot("caption_just") - fn_just <- .dot("footnote_just") - gp_page <- .dot("gp") + margins <- .dot("margins") + padding <- .dot("padding") + header_rule <- .dot("header_rule") + footer_rule <- .dot("footer_rule") + cap_just <- .dot("caption_just") + fn_just <- .dot("footnote_just") + gp_page <- .dot("gp") + overflow_action <- .dot("overflow_action") + overflow_action <- match.arg(overflow_action, c("error", "warn")) annot <- list( header_left = dots$header_left, @@ -138,7 +141,8 @@ tfl_table_to_pagelist <- function(tbl, pg_width, pg_height, dots, # Keep a pre-width copy of resolved_cols in case a second pass is needed. resolved_cols_0 <- resolved_cols col_result <- compute_col_widths( - resolved_cols, tbl$data, cw, tbl, pg_width, pg_height, margins + resolved_cols, tbl$data, cw, tbl, pg_width, pg_height, margins, + overflow_action = overflow_action ) resolved_cols <- col_result$resolved_cols # widths now set in inches col_groups <- col_result$col_groups # list of integer vectors @@ -157,7 +161,9 @@ tfl_table_to_pagelist <- function(tbl, pg_width, pg_height, dots, if (!is.null(tbl$col_cont_msg[[1L]])) cw_adj <- cw_adj - hw if (!is.null(tbl$col_cont_msg[[2L]])) cw_adj <- cw_adj - hw col_result <- compute_col_widths( - resolved_cols_0, tbl$data, cw_adj, tbl, pg_width, pg_height, margins + resolved_cols_0, tbl$data, cw_adj, tbl, pg_width, pg_height, margins, + overflow_action = overflow_action, + validate_overflow = FALSE # first pass already validated ) resolved_cols <- col_result$resolved_cols col_groups <- col_result$col_groups diff --git a/design/ARCHITECTURE.md b/design/ARCHITECTURE.md index e73fd28..19a5984 100644 --- a/design/ARCHITECTURE.md +++ b/design/ARCHITECTURE.md @@ -96,6 +96,10 @@ export_tfl_page(x, ...) [exported] ├── check_overlap(widths, vp_width, overlap_warn_mm) — overlap.R ├── compute_figure_height(...) — layout.R ├── check_figure_height(h, min_content_height, errors) — layout.R + ├── if grob content and not (ggplot/character/tfl_table_grob): — layout.R + │ check_content_width(grobWidth, vp_width, overflow_action, errors) + │ [tfl_table_grob is skipped: compute_col_widths() already ran a + │ more precise per-column check during tfl_table_to_pagelist()] ├── if errors: rlang::abort(paste(errors, collapse="\n")) │ ├── [DRAWING PHASE] @@ -317,7 +321,7 @@ export_tfl(x = list_of_table1, ...) [exported] | `R/normalize.R` | `normalize_text()`, `wrap_normalized_text()`, `normalize_rule()` | | `R/resolve_gp.R` | `resolve_gp()`, `merge_gpar()` | | `R/overlap.R` | `check_overlap()` | -| `R/layout.R` | `compute_figure_height()`, `check_figure_height()` | +| `R/layout.R` | `compute_figure_height()`, `check_figure_height()`, `check_content_width()`, `.overflow_signal()` | | `R/utils.R` | `validate_file_arg()`, `coerce_x_to_pagelist()`, `build_page_args()` | | `R/gt.R` | `export_tfl.gt_tbl()`, `gt_to_pagelist()`, `.extract_gt_annotations()`, `.clean_gt()`, `.gt_content_height()`, `.gt_grob_height()`, `.gt_row_groups()`, `.paginate_gt()`, `.rebuild_gt_subset()` | | `R/rtables.R` | `export_tfl.VTableTree()`, `rtables_to_pagelist()`, `.extract_rtables_annotations()`, `.clean_rtables()`, `.rtables_content_height()`, `.rtables_content_width()`, `.rtables_lpp_cpp()`, `.rtables_to_grob()` | @@ -571,16 +575,23 @@ resolve_col_specs(tbl) { col, label, width (unit/numeric/NULL), align, wrap, gp, is_group_col } -compute_col_widths(resolved_cols, data, content_width_in, tbl, ...) +compute_col_widths(resolved_cols, data, content_width_in, tbl, ..., + overflow_action = c("error", "warn")) 1. Measure auto-size columns: max string width over unique values (limited to max_measure_rows rows for efficiency) 2. Apply min_col_width floor to auto-sized columns 3. Allocate relative-weight columns proportionally from remaining width 4. If total > content_width_in and wrap_eligible cols exist: .apply_col_wrapping() narrows wrap cols iteratively - 5. If total still > content_width_in and allow_col_split: - paginate_cols() splits into column groups - 6. Each resolved col gets $width_in set (inches, scalar) + 5. Per-column / group-aware overflow check (issue #30): + For group cols j: widths_in[j] > content_width_in → signal + For data cols j: grp_w + widths_in[j] > content_width_in → signal + (signal = abort under "error", rlang::warn() under "warn") + 6. Total-width check, only when allow_col_split = FALSE: + total_w > content_width_in → signal + 7. paginate_cols() splits into column groups (always called; under "warn" + it gracefully paginates around the overflow) + 8. Each resolved col gets $width_in set (inches, scalar) paginate_cols(col_indices, col_widths_in, group_col_indices, content_width_in, balance_col_pages) diff --git a/design/DECISIONS.md b/design/DECISIONS.md index 3b56633..19d9617 100644 --- a/design/DECISIONS.md +++ b/design/DECISIONS.md @@ -800,3 +800,103 @@ existing helper. `.strip_sub_tfl_cols()`, and `.resolve_col_label()` (factored out of `resolve_col_specs()` so sub_tfl and the column-spec resolver share label fallback logic). + +--- + +## D-39: `overflow_action` for too-wide content (issue #30) + +**Decision:** Add a single user-facing parameter +`overflow_action = c("error", "warn")` (default `"error"`) on +`export_tfl_page()` and forward it through `export_tfl()` and the +`tfl_table_to_pagelist()` pipeline. The same knob controls three width-overflow +detection sites: a new page-level grob check in `export_tfl_page()`, the +existing `tfl_table` total-width abort in `compute_col_widths()`, and a new +group-aware per-column check (also in `compute_col_widths()`). + +**User need (from issue #30):** "When the content section is too wide for the +allocated area, give an error. That error should be convertible to a warning +so that output can be generated for diagnosis of the issue." + +**Behavior:** +- `"error"`: append the message to the `errors` accumulator and abort via + `rlang::abort(paste(errors, collapse = "\n"))`. No drawing occurs. +- `"warn"`: emit `rlang::warn()` immediately and continue. The PDF is + produced with overflow visibly clipped by `grid` so the user can see what + is too wide. + +**Three detection sites, one knob:** +1. **Page-level grob check** (`export_tfl_page()`, validation phase): when + `x$content` is a non-ggplot, non-character, non-`tfl_table_grob` grob, + measure `grid::grobWidth(x$content)` while `outer_vp` is active and + compare to `vp_width_in`. Catches `gt::as_gtable()`, `rtables` textGrobs, + `gridExtra::tableGrob`, and raw user grobs. `tfl_table_grob` is excluded + because the per-column check at site (3) below already validated the + layout with finer-grained per-column information; re-checking the + assembled grob would emit a redundant, less informative warning under + `overflow_action = "warn"`. +2. **`tfl_table` total-width** (`compute_col_widths()`): the existing + `allow_col_split = FALSE` abort now respects `overflow_action`. +3. **`tfl_table` group-aware per-column** (`compute_col_widths()`, new): for + each group column j ≤ `n_group_cols`, signal if + `widths_in[j] > content_width_in`; for each data column j > `n_group_cols`, + signal if `grp_w + widths_in[j] > content_width_in`. The data-column rule + is **group-aware** because group columns repeat on every column-paginated + page, so a data column that doesn't fit alongside the row headers can + never be rendered. Previously this case overflowed silently — grid + clipped the column with no warning. + +**Alternatives considered and rejected:** +- Boolean `strict_width = TRUE/FALSE`: less explicit and harder to extend + with future severity levels. +- Numeric threshold `width_warn_mm` (mirroring `overlap_warn_mm`): conflates + near-miss detection with the diagnostic-mode use case the user actually + asked for. +- Three-level `c("error", "warn", "silent")`: silent overflow is the bug + being fixed; leaving an opt-out re-opens it. +- Auto-promoting `allow_col_split = FALSE` → `TRUE` under `"warn"`: would + override an explicit user choice. The cleaner mental model is that + `allow_col_split` decides *whether* the total-width condition counts as an + overflow event, and `overflow_action` decides *how* it is signaled. +- Merging `check_content_height()` and the new `check_content_width()` into + a single `check_content_area()`: rejected because the two checks have + structurally different signatures (height takes a `min_content_height` + unit; width takes a viewport-width numeric and the action knob), different + semantics (min-floor vs max-ceiling), and naming the dimension precisely + reads better at the call site than `_area`. The shared part — the + warn-vs-error dispatch — is factored into a small private + `.overflow_signal()` helper instead. + +**API placement:** Top-level `export_tfl_page()` argument (next to +`min_content_height`) rather than in `...` (where `overlap_warn_mm` lives), +because `overflow_action` changes error-vs-warning semantics and deserves a +documented top-level slot. `export_tfl()` picks it up automatically via +`@inheritDotParams export_tfl_page` (also adopted in this change), so the +docs stay in sync without manual duplication. + +**Diagnostic hint in messages:** every overflow message — abort or warning — +ends with the literal hint +`Set `overflow_action = "warn"` to convert this error to a warning and still +produce output for diagnosis.` Centralized in the new private +`.overflow_signal()` helper in `R/layout.R`. + +**`sub_tfl` ordering:** the per-column / group-aware check is positioned +inside `compute_col_widths()` (called from `.tfl_table_to_pagelist_default()`), +which is reached from `.tfl_table_to_pagelist_sub_tfl()` *after* +`.strip_sub_tfl_cols()` has removed the `sub_tfl` columns from the data and +from `group_vars`. So a `tfl_table` whose original group columns would +overflow only because they include columns later stripped by `sub_tfl` is +correctly accepted. A regression test in `tests/testthat/test-sub_tfl.R` +locks this ordering in. + +**Backward compatibility:** the only behavior change for existing scripts is +that previously-silent overflow cases (single-column, group-aware, raw grob +content) now error by default. This is intentional — silent overflow is the +bug being fixed. Users who want the old behavior can pass +`overflow_action = "warn"` (or fix the underlying width issue). + +**Files touched:** +- New: `.overflow_signal()`, `check_content_width()` in `R/layout.R`. +- Modified: `R/export_tfl_page.R` (new arg, validation phase, page-level + grob check), `R/table_columns.R` (`compute_col_widths()` per-column + + total-width checks), `R/table_pagelist.R` (thread the arg through), + `R/export_tfl.R` (`@inheritDotParams`). diff --git a/design/DESIGN.md b/design/DESIGN.md index 46c4e9c..3d0e6fe 100644 --- a/design/DESIGN.md +++ b/design/DESIGN.md @@ -216,3 +216,40 @@ is required (e.g., a 0.5-inch margin requirement in a regulatory submission), `"inches"` or `"mm"` are more predictable. The `padding` argument uses `"lines"` deliberately, because inter-section spacing that scales with font size is usually the right behaviour. + +--- + +## Why does `overflow_action` default to `"error"` and not `"warn"`? + +Issue #30 calls out that too-wide content should produce a clear signal that +the user can downgrade to a warning *for diagnosis*. The reverse — silent +overflow with an opt-in error — was the prior status quo for several content +shapes (single-column overflow with group columns, raw user grobs wider than +the page, gt/rtables grobs that exceed the content area). Defaulting to +`"error"` reverses that: anything that cannot fit is surfaced immediately, +and the user explicitly opts into `"warn"` to inspect the broken layout. + +The error message always includes the literal hint +`Set \`overflow_action = "warn"\` to convert this error to a warning and +still produce output for diagnosis.` so the escape hatch is always one +keystroke away. We resisted introducing a third `"silent"` level: silent +overflow is the bug we're fixing, and an opt-out would re-open it. + +--- + +## Why one `overflow_action` knob across page-level, total, and per-column checks? + +Issue #30 frames the problem as a single user-facing concept ("content too +wide"); it would be confusing to expose three separate parameters with +similar but slightly different semantics. The three internal call sites +(`export_tfl_page()` page-level grob check, `compute_col_widths()` total +check, `compute_col_widths()` per-column check) collapse onto one knob via +the small private `.overflow_signal()` helper in `R/layout.R`, which is the +only function that actually knows the difference between `"error"` and +`"warn"`. Each call site composes its own message; the helper handles +dispatch and appends the diagnostic-mode hint. + +This also keeps the API surface aligned with `min_content_height` (a single +top-level argument controlling a single layout invariant) rather than with +`overlap_warn_mm` (a tuning knob in `...`). Width overflow is a +correctness-affecting condition, not a tuning detail. diff --git a/design/TESTING.md b/design/TESTING.md index 1393ec2..7be4bb2 100644 --- a/design/TESTING.md +++ b/design/TESTING.md @@ -22,7 +22,7 @@ One test file per source file — `tests/testthat/test-.R` covers | `test-draw.R` | `draw_content()`, `draw_rule()`, `draw_header_section()`, `draw_footer_section()`, `draw_caption_section()`, `draw_footnote_section()` | | `test-grob_builders.R` | `build_text_grob()`, `build_section_grobs()` | | `test-export_tfl.R` | `export_tfl()` — file validation, return values, preview mode, device lifecycle, tfl_table coercion, argument merging | -| `test-export_tfl_page.R` | `export_tfl_page()` — argument resolution from x, overlap_warn_mm, page_i prefix, section presence, rules | +| `test-export_tfl_page.R` | `export_tfl_page()` — argument resolution from x, overlap_warn_mm, page_i prefix, section presence, rules, page-level grob overflow under `overflow_action` (issue #30) | | `test-table_utils.R` | `.compute_group_sizes()`, `.collect_col_strings()`, `.measure_max_string_width()`, `.wrap_text()` | | `test-table_draw.R` | `build_table_grob()`, `drawDetails.tfl_table_grob()` (uncached fallback, wrap branch, rotated col_cont_msg labels, first_data fallback) | | `test-tfl_table.R` | `tfl_colspec()`, `tfl_table()`, column/row pagination, column width calculation, col_cont_msg flags, `tfl_table_to_pagelist()` | @@ -96,7 +96,7 @@ test_that("check_overlap handles all three absent (no-op)", ...) --- -## `test-layout.R` — `compute_figure_height()`, `check_figure_height()` +## `test-layout.R` — `compute_figure_height()`, `check_figure_height()`, `check_content_width()` ```r test_that("figure height equals vp_height when no other sections", ...) @@ -110,6 +110,15 @@ test_that("rules do not subtract from content height", ...) test_that("check_figure_height adds error when h < min_content_height", ...) test_that("check_figure_height does not add error when h >= min", ...) test_that("error message includes actual and minimum heights", ...) + +# check_content_width — issue #30 +test_that("check_content_width is a no-op when content fits", ...) +test_that("check_content_width appends to errors under \"error\" action", ...) +test_that("check_content_width emits rlang::warn under \"warn\" action and leaves errors unchanged", ...) +test_that("check_content_width error message includes the diagnostic-mode hint", ...) +test_that("check_content_width warning text includes the diagnostic-mode hint", ...) +test_that("check_content_width respects custom `what` label", ...) +test_that("check_content_width tolerates near-equal widths within 1e-6", ...) ``` --- @@ -233,6 +242,13 @@ Key areas covered: - `measure_row_heights_tbl()`: `max_measure_rows` sampling, wrap path - `tfl_table_to_pagelist()`: full pipeline smoke test, group validation, allow_col_split = FALSE error +- **`overflow_action` (issue #30)**: total too wide + `allow_col_split = FALSE` + errors by default and warns under `"warn"`; single non-group column wider + than the page errors / warns; group columns + a single data column that + overflow trigger the new group-aware error / warning; group column itself + wider than the page is reported with a `"Group column"` prefix; every + abort and warning message includes the literal `overflow_action = "warn"` + diagnostic hint. --- @@ -281,6 +297,12 @@ Key areas covered: carries the same caption suffix - long suffix that wraps to multiple lines: content height re-measured per group (no overflow on the worst-case group) + - **per-column overflow check runs after sub_tfl strips columns + (issue #30)**: a `tfl_table` whose original group columns + a data + column would overflow only because one group column appears in + `sub_tfl` is correctly accepted when `sub_tfl` is set, and rejected + when it is not. Locks in the ordering: `compute_col_widths()` runs + *after* `.strip_sub_tfl_cols()`. --- diff --git a/man/check_content_width.Rd b/man/check_content_width.Rd new file mode 100644 index 0000000..1809aa7 --- /dev/null +++ b/man/check_content_width.Rd @@ -0,0 +1,36 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/layout.R +\name{check_content_width} +\alias{check_content_width} +\title{Check content width against an upper bound and signal if too wide} +\usage{ +check_content_width( + content_w_in, + vp_width_in, + overflow_action, + errors, + what = "Content" +) +} +\arguments{ +\item{content_w_in}{Natural width of the content in inches.} + +\item{vp_width_in}{Available content viewport width in inches.} + +\item{overflow_action}{One of \code{"error"} (default) or \code{"warn"}.} + +\item{errors}{Character vector to append to (when action is \code{"error"}).} + +\item{what}{Label for the source of the width (e.g. \code{"Content"}, +\code{"Column 'x'"}, \code{"Total column width"}).} +} +\value{ +Updated \code{errors} character vector. +} +\description{ +Mirrors \code{\link[=check_content_height]{check_content_height()}} but is a maximum-ceiling check rather than +a minimum-floor check, and accepts an \code{overflow_action} knob that downgrades +the error to a warning so output can still be produced for diagnosis (see +issue #30). +} +\keyword{internal} diff --git a/man/compute_col_widths.Rd b/man/compute_col_widths.Rd index 44c3d0c..9a83bba 100644 --- a/man/compute_col_widths.Rd +++ b/man/compute_col_widths.Rd @@ -11,9 +11,20 @@ compute_col_widths( tbl, pg_width, pg_height, - margins + margins, + overflow_action = c("error", "warn"), + validate_overflow = TRUE ) } +\arguments{ +\item{overflow_action}{One of \code{"error"} (default) or \code{"warn"}. Controls how +width-overflow conditions are reported. See \code{\link[=export_tfl_page]{export_tfl_page()}}.} + +\item{validate_overflow}{Logical (internal). When \code{FALSE}, skip the +per-column / group-aware / total-width overflow checks. The second +\code{cw_adj} pass in \code{.tfl_table_to_pagelist_default()} sets this to \code{FALSE} +so the same overflow is not re-signalled on every pass.} +} \value{ A list with \verb{$resolved_cols} (widths_in filled in) and \verb{$col_groups} (list of integer vectors of column indices per group). diff --git a/man/export_tfl.Rd b/man/export_tfl.Rd index 1429e41..cf5bfda 100644 --- a/man/export_tfl.Rd +++ b/man/export_tfl.Rd @@ -91,9 +91,79 @@ pages. Set to \code{NULL} to disable.} In preview mode each page is drawn via \code{grid::grid.newpage()} (so knitr captures it as an inline graphic). Returns \code{NULL} invisibly.} -\item{...}{Additional arguments passed to \code{\link[=export_tfl_page]{export_tfl_page()}}. -These serve as defaults for all pages and are overridden by per-page -list elements in \code{x}.} +\item{...}{ + Arguments passed on to \code{\link[=export_tfl_page]{export_tfl_page}} + \describe{ + \item{\code{padding}}{Vertical space between adjacent present sections, as a +\code{unit} object. Separator rules (if enabled) are drawn at the midpoint +of this gap and do not consume additional space.} + \item{\code{header_left,header_center,header_right}}{Header text. Accepts +\code{NULL}, a single string, or a character vector (collapsed with \code{"\\\\n"}). +Horizontal justification follows the argument name (left/center/right). +Vertically top-justified. Overridden by \code{x$header_left} etc.} + \item{\code{caption}}{Caption text below the header and above the content. Accepts +\code{NULL}, a single string, or a character vector. Full-width; justification +controlled by \code{caption_just}. Overridden by \code{x$caption}.} + \item{\code{footnote}}{Footnote text below the content. Accepts \code{NULL}, a single +string, or a character vector. Full-width; justification controlled by +\code{footnote_just}. Overridden by \code{x$footnote}.} + \item{\code{footer_left,footer_center,footer_right}}{Footer text. Mirror of +header arguments. Vertically bottom-justified. Overridden by +\code{x$footer_left} etc.} + \item{\code{gp}}{Typography specification. Accepts either a single \code{gpar()} object +applied to all text, or a named list for section- or element-level +control. Resolution priority (highest first): element-level +(e.g. \code{gp$header_left}), section-level (e.g. \code{gp$header}), global +\code{gpar()}. Example: + +\if{html}{\out{
}}\preformatted{gp = list( + header = gpar(fontsize = 11, fontface = "bold"), + header_right = gpar(fontsize = 9, col = "gray50"), + caption = gpar(fontsize = 9, fontface = "italic"), + footer = gpar(fontsize = 8) +) +}\if{html}{\out{
}}} + \item{\code{header_rule}}{Separator rule drawn between the header and the next +section (caption or content), fitted within the \code{padding} gap. Accepts: +\itemize{ +\item \code{FALSE}: no rule +\item \code{TRUE}: full-width rule +\item A numeric in \verb{(0, 1]}: rule spanning that fraction of viewport width, +centered +\item A grob (typically a \code{linesGrob}): drawn as-is, centered vertically +in the padding gap. +}} + \item{\code{footer_rule}}{Separator rule between the last body section (footnote +or content) and the footer. Same specification as \code{header_rule}.} + \item{\code{caption_just}}{Horizontal justification for the caption.} + \item{\code{footnote_just}}{Horizontal justification for the footnote.} + \item{\code{content_just}}{Horizontal justification for character string content. +One of \code{"left"} (default), \code{"right"}, or \code{"centre"}. Ignored when +\code{x$content} is a ggplot or grob.} + \item{\code{margins}}{Outer page margins as a \code{unit} vector with elements +\code{t}, \code{r}, \code{b}, \code{l} (top, right, bottom, left).} + \item{\code{min_content_height}}{Minimum acceptable content area height as a \code{unit} +object. An error is raised if the computed content height falls below this +value.} + \item{\code{overflow_action}}{One of \code{"error"} (default) or \code{"warn"}. Controls how +width-overflow conditions are reported when the content does not fit in +its allocated area: +\itemize{ +\item \code{"error"}: append the message to the layout-error vector and abort +before drawing (no PDF page is produced). +\item \code{"warn"}: emit \code{rlang::warn()} and continue rendering. The PDF is +produced with the overflow visibly clipped by \code{grid}, which is useful +for diagnosing what is too wide. See issue #30. +} + +The same setting applies to all width-overflow detections: the +page-level content grob check (any \code{grob} content wider than the +content viewport), the \code{\link[=tfl_table]{tfl_table()}} total-width check (when +\code{allow_col_split = FALSE} and the column total still exceeds the +page after wrapping), and the \code{\link[=tfl_table]{tfl_table()}} per-column check (any +single column — or any data column combined with the row-header +group columns — wider than the page).} + }} } \value{ \itemize{ @@ -111,6 +181,10 @@ When \code{preview} is not \code{FALSE}, no PDF is written. Instead the selected are drawn to the currently open graphics device (useful in RStudio, Positron, or knitr chunks) and returned as a list of grid grobs. } +\details{ +Arguments forwarded via \code{...} serve as defaults for all pages and are +overridden by per-page list elements in \code{x}. +} \examples{ \dontrun{ library(ggplot2) diff --git a/man/export_tfl_page.Rd b/man/export_tfl_page.Rd index 6011f41..993c48f 100644 --- a/man/export_tfl_page.Rd +++ b/man/export_tfl_page.Rd @@ -23,6 +23,7 @@ export_tfl_page( content_just = "left", margins = grid::unit(c(t = 0.5, r = 0.5, b = 0.5, l = 0.5), "inches"), min_content_height = grid::unit(3, "inches"), + overflow_action = c("error", "warn"), page_i = NULL, preview = FALSE, ... @@ -102,6 +103,25 @@ One of \code{"left"} (default), \code{"right"}, or \code{"centre"}. Ignored when object. An error is raised if the computed content height falls below this value.} +\item{overflow_action}{One of \code{"error"} (default) or \code{"warn"}. Controls how +width-overflow conditions are reported when the content does not fit in +its allocated area: +\itemize{ +\item \code{"error"}: append the message to the layout-error vector and abort +before drawing (no PDF page is produced). +\item \code{"warn"}: emit \code{rlang::warn()} and continue rendering. The PDF is +produced with the overflow visibly clipped by \code{grid}, which is useful +for diagnosing what is too wide. See issue #30. +} + +The same setting applies to all width-overflow detections: the +page-level content grob check (any \code{grob} content wider than the +content viewport), the \code{\link[=tfl_table]{tfl_table()}} total-width check (when +\code{allow_col_split = FALSE} and the column total still exceeds the +page after wrapping), and the \code{\link[=tfl_table]{tfl_table()}} per-column check (any +single column — or any data column combined with the row-header +group columns — wider than the page).} + \item{page_i}{Integer page index, used to prefix layout error messages with \code{"Page : "}. Set automatically by \code{\link[=export_tfl]{export_tfl()}}; not normally supplied when calling this function directly.} diff --git a/tests/testthat/test-export_tfl_page.R b/tests/testthat/test-export_tfl_page.R index e6da69a..b9c6f36 100644 --- a/tests/testthat/test-export_tfl_page.R +++ b/tests/testthat/test-export_tfl_page.R @@ -312,3 +312,62 @@ test_that("gp$content controls character content typography", { ) ) }) + +# --------------------------------------------------------------------------- +# overflow_action — page-level grob width check (issue #30) +# --------------------------------------------------------------------------- + +test_that("non-table grob wider than the content viewport errors by default", { + f <- tempfile(fileext = ".pdf") + grDevices::pdf(f, width = 8.5, height = 11) + on.exit({ grDevices::dev.off(); unlink(f) }) + + wide_grob <- grid::rectGrob(width = grid::unit(20, "inches")) + expect_error( + export_tfl_page(list(content = wide_grob)), + "exceeds available content width" + ) +}) + +test_that("non-table grob overflow can be downgraded to a warning", { + f <- tempfile(fileext = ".pdf") + grDevices::pdf(f, width = 8.5, height = 11) + on.exit({ grDevices::dev.off(); unlink(f) }) + + wide_grob <- grid::rectGrob(width = grid::unit(20, "inches")) + expect_warning( + export_tfl_page(list(content = wide_grob), overflow_action = "warn"), + "exceeds available content width" + ) +}) + +test_that("ggplot content does not trigger the page-level width check (it scales)", { + skip_if_not_installed("ggplot2") + f <- tempfile(fileext = ".pdf") + grDevices::pdf(f, width = 8.5, height = 11) + on.exit({ grDevices::dev.off(); unlink(f) }) + + p <- ggplot2::ggplot(mtcars, ggplot2::aes(mpg, hp)) + ggplot2::geom_point() + expect_no_error(export_tfl_page(list(content = p))) +}) + +test_that("character content does not trigger the page-level width check (it word-wraps)", { + f <- tempfile(fileext = ".pdf") + grDevices::pdf(f, width = 8.5, height = 11) + on.exit({ grDevices::dev.off(); unlink(f) }) + + long <- paste(rep("word", 200), collapse = " ") + expect_no_error(export_tfl_page(list(content = long))) +}) + +test_that("page-level overflow message includes the diagnostic-mode hint", { + f <- tempfile(fileext = ".pdf") + grDevices::pdf(f, width = 8.5, height = 11) + on.exit({ grDevices::dev.off(); unlink(f) }) + + wide_grob <- grid::rectGrob(width = grid::unit(20, "inches")) + expect_error( + export_tfl_page(list(content = wide_grob)), + "overflow_action = \"warn\"" + ) +}) diff --git a/tests/testthat/test-layout.R b/tests/testthat/test-layout.R index 7b373c3..b1ba6a3 100644 --- a/tests/testthat/test-layout.R +++ b/tests/testthat/test-layout.R @@ -114,3 +114,88 @@ test_that("check_content_height appends to existing error vector", { expect_length(errors, 2) expect_equal(errors[[1]], "prior error") }) + +# --------------------------------------------------------------------------- +# check_content_width — issue #30 +# --------------------------------------------------------------------------- + +test_that("check_content_width is a no-op when content fits", { + errors <- check_content_width( + content_w_in = 5, + vp_width_in = 7, + overflow_action = "error", + errors = character(0) + ) + expect_length(errors, 0) +}) + +test_that("check_content_width appends to errors under \"error\" action", { + errors <- check_content_width( + content_w_in = 9, + vp_width_in = 7, + overflow_action = "error", + errors = character(0), + what = "Content" + ) + expect_length(errors, 1) + expect_match(errors[[1]], "exceeds available content width") + expect_match(errors[[1]], "9") + expect_match(errors[[1]], "7") +}) + +test_that("check_content_width emits rlang::warn under \"warn\" action and leaves errors unchanged", { + expect_warning( + out <- check_content_width( + content_w_in = 9, + vp_width_in = 7, + overflow_action = "warn", + errors = character(0) + ), + "exceeds available content width" + ) + expect_length(out, 0) +}) + +test_that("check_content_width error message includes the diagnostic-mode hint", { + errors <- check_content_width( + content_w_in = 9, + vp_width_in = 7, + overflow_action = "error", + errors = character(0) + ) + expect_match(errors[[1]], "overflow_action = \"warn\"", fixed = TRUE) +}) + +test_that("check_content_width warning text includes the diagnostic-mode hint", { + expect_warning( + check_content_width( + content_w_in = 9, + vp_width_in = 7, + overflow_action = "warn", + errors = character(0) + ), + "overflow_action = \"warn\"", + fixed = TRUE + ) +}) + +test_that("check_content_width respects custom `what` label", { + errors <- check_content_width( + content_w_in = 9, + vp_width_in = 7, + overflow_action = "error", + errors = character(0), + what = "Column 'foo'" + ) + expect_match(errors[[1]], "Column 'foo' width", fixed = TRUE) +}) + +test_that("check_content_width tolerates near-equal widths within 1e-6", { + errors <- check_content_width( + content_w_in = 7 + 1e-7, + vp_width_in = 7, + overflow_action = "error", + errors = character(0) + ) + expect_length(errors, 0) +}) diff --git a/tests/testthat/test-sub_tfl.R b/tests/testthat/test-sub_tfl.R index 5774a0a..16e27e4 100644 --- a/tests/testthat/test-sub_tfl.R +++ b/tests/testthat/test-sub_tfl.R @@ -333,3 +333,57 @@ test_that("ggtibble_to_pagelist rejects unknown sub_tfl columns", { class(x) <- c("ggtibble", class(x)) expect_error(ggtibble_to_pagelist(x, sub_tfl = "missing"), "not found") }) + +# --------------------------------------------------------------------------- +# Issue #30: per-column overflow check runs *after* sub_tfl strips columns +# --------------------------------------------------------------------------- + +test_that("per-column overflow check runs against the post-strip group_vars (sub_tfl ordering)", { + # Build a table where: + # - group_vars are (arm, visit), with arm fixed to 6 in + # - data col b fixed to 3 in + # - pg_width = 8.5, default 0.5-in margins → content ≈ 7.5 in + # Without sub_tfl: arm (6) + b (3) = 9 > 7.5 → group-aware overflow. + # With sub_tfl = "arm": arm is stripped from group_vars and from the data + # before compute_col_widths() runs, so the remaining group col 'visit' (~1 in + # auto) + b (3) = ~4 in fits comfortably and no error is raised. + df <- data.frame( + arm = c("A", "A", "B", "B"), + visit = c("V1", "V2", "V1", "V2"), + b = 1:4, + stringsAsFactors = FALSE + ) + df <- dplyr::group_by(df, arm, visit) + + # 1. Without sub_tfl: errors as expected. + tbl_no_sub <- tfl_table( + df, + cols = list( + tfl_colspec("arm", width = grid::unit(6, "inches")), + tfl_colspec("b", width = grid::unit(3, "inches")) + ) + ) + f1 <- tempfile(fileext = ".pdf"); on.exit(unlink(f1), add = TRUE) + expect_error( + export_tfl(tbl_no_sub, file = f1, pg_width = 8.5, pg_height = 11), + "plus group columns" + ) + + # 2. With sub_tfl = "arm": arm is stripped by .strip_sub_tfl_cols() before + # the per-column check runs, so the table renders without error. Locks in + # the ordering: per-column check happens *after* sub_tfl handling. + tbl_sub <- tfl_table( + df, + cols = list( + tfl_colspec("arm", width = grid::unit(6, "inches")), + tfl_colspec("b", width = grid::unit(3, "inches")) + ), + sub_tfl = "arm" + ) + f2 <- tempfile(fileext = ".pdf"); on.exit(unlink(f2), add = TRUE) + expect_no_error( + export_tfl(tbl_sub, file = f2, pg_width = 8.5, pg_height = 11) + ) + expect_true(file.exists(f2)) + expect_gt(file.info(f2)$size, 0) +}) diff --git a/tests/testthat/test-tfl_table.R b/tests/testthat/test-tfl_table.R index edfa647..ee72d7c 100644 --- a/tests/testthat/test-tfl_table.R +++ b/tests/testthat/test-tfl_table.R @@ -1014,18 +1014,21 @@ test_that("paginate_cols prepends multiple group cols to every page", { test_that("compute_col_widths handles a wrap col already at or below min_col_width", { # The wrap-eligible column auto-measures to a small width. # min_col_width = 4 in → the column is below min → not eligible for narrowing - # → .apply_col_wrapping hits the 'no eligible cols' break at line 190. - # allow_col_split = TRUE so the table still renders despite total exceeding content width. + # → .apply_col_wrapping hits the 'no eligible cols' break. + # content_width_in is set just below the total so the wrap loop is entered + # but each individual column still fits in content_width (so the per-column + # overflow check from issue #30 does not fire). tbl <- tfl_table( make_simple_df(), - wrap_cols = "label", - min_col_width = grid::unit(4, "inches"), + wrap_cols = "label", + min_col_width = grid::unit(4, "inches"), allow_col_split = TRUE ) expect_no_error( compute_col_widths( resolve_col_specs(tbl), tbl$data, - content_width_in = 3, + content_width_in = 11, # 3 cols × 4 in = 12 in > 11 → wrap loop entered; + # each col 4 in ≤ 11 → no per-col overflow tbl, pg_width = 11, pg_height = 8.5, margins = grid::unit(c(0.5, 0.5, 0.5, 0.5), "inches") ) @@ -1077,3 +1080,161 @@ test_that("tfl_table_to_pagelist respects explicit FALSE for header_rule/footer_ expect_true(file.exists(f)) expect_gt(file.info(f)$size, 0) }) + +# --------------------------------------------------------------------------- +# overflow_action — issue #30 +# --------------------------------------------------------------------------- + +# Helper: tfl_table with one fixed-width column wider than any reasonable +# page. Used to trigger the per-column overflow path without depending on +# text-measurement heuristics. +make_wide_col_table <- function(wide_w = grid::unit(20, "inches")) { + df <- data.frame( + a = c("hello", "world"), + b = 1:2, + stringsAsFactors = FALSE + ) + tfl_table( + df, + cols = list(tfl_colspec("a", width = wide_w)) + ) +} + +test_that("default overflow_action errors when total width exceeds page with allow_col_split = FALSE", { + # Backward-compat anchor: the existing pre-#30 abort still fires by default. + tbl <- tfl_table( + make_simple_df(), + cols = list( + tfl_colspec("label", width = grid::unit(4, "inches")), + tfl_colspec("value1", width = grid::unit(4, "inches")), + tfl_colspec("value2", width = grid::unit(4, "inches")) + ), + allow_col_split = FALSE + ) + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_error( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11), + "exceeds available content width" + ) +}) + +test_that("overflow_action = 'warn' downgrades the total-width abort and produces output", { + tbl <- tfl_table( + make_simple_df(), + cols = list( + tfl_colspec("label", width = grid::unit(4, "inches")), + tfl_colspec("value1", width = grid::unit(4, "inches")), + tfl_colspec("value2", width = grid::unit(4, "inches")) + ), + allow_col_split = FALSE + ) + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_warning( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11, + overflow_action = "warn"), + "exceeds available content width" + ) + expect_true(file.exists(f)) + expect_gt(file.info(f)$size, 0) +}) + +test_that("single non-group column wider than the page errors by default (issue #30)", { + tbl <- make_wide_col_table() + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_error( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11), + "Column 'a'" + ) +}) + +test_that("single non-group column wider than the page warns under overflow_action = 'warn'", { + tbl <- make_wide_col_table() + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_warning( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11, + overflow_action = "warn"), + "Column 'a'" + ) + expect_true(file.exists(f)) + expect_gt(file.info(f)$size, 0) +}) + +test_that("group cols + a single data col that overflow trigger the group-aware error", { + # Force group col 'g' to ~6 in and data col 'b' to ~3 in. Each individually + # fits a 8.5-in page (with 0.5-in margins → 7.5-in content), but together + # the row-header repeat plus the data column blows past content width. The + # group-aware check from issue #30 should catch this. + df <- dplyr::group_by( + data.frame(g = c("X", "X"), b = 1:2, stringsAsFactors = FALSE), + g + ) + tbl <- tfl_table( + df, + cols = list( + tfl_colspec("g", width = grid::unit(6, "inches")), + tfl_colspec("b", width = grid::unit(3, "inches")) + ) + ) + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_error( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11), + "plus group columns" + ) +}) + +test_that("group cols + a single data col that overflow can be downgraded to a warning", { + df <- dplyr::group_by( + data.frame(g = c("X", "X"), b = 1:2, stringsAsFactors = FALSE), + g + ) + tbl <- tfl_table( + df, + cols = list( + tfl_colspec("g", width = grid::unit(6, "inches")), + tfl_colspec("b", width = grid::unit(3, "inches")) + ) + ) + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_warning( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11, + overflow_action = "warn"), + "plus group columns" + ) + expect_true(file.exists(f)) + expect_gt(file.info(f)$size, 0) +}) + +test_that("group column itself wider than the page is reported with 'Group column'", { + df <- dplyr::group_by( + data.frame(g = c("X", "X"), b = 1:2, stringsAsFactors = FALSE), + g + ) + tbl <- tfl_table( + df, + cols = list(tfl_colspec("g", width = grid::unit(20, "inches"))) + ) + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_error( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11), + "Group column 'g'" + ) +}) + +test_that("overflow error messages include the diagnostic-mode hint", { + tbl <- make_wide_col_table() + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_error( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11), + "overflow_action = \"warn\"" + ) +}) + +test_that("overflow warning messages include the diagnostic-mode hint", { + tbl <- make_wide_col_table() + f <- tempfile(fileext = ".pdf"); on.exit(unlink(f)) + expect_warning( + export_tfl(tbl, file = f, pg_width = 8.5, pg_height = 11, + overflow_action = "warn"), + "overflow_action = \"warn\"" + ) +}) diff --git a/vignettes/v04-troubleshooting.Rmd b/vignettes/v04-troubleshooting.Rmd index 6614d1b..4be7a75 100644 --- a/vignettes/v04-troubleshooting.Rmd +++ b/vignettes/v04-troubleshooting.Rmd @@ -37,15 +37,27 @@ export_tfl(x, file = "out.pdf", min_content_height = grid::unit(2, "inches")) ``` -## Column width overflow +## Content too wide for the page -**Error:** `Total column width (X in) exceeds available content width (Y in) after wrapping.` +**Error message ends with:** +`Set \`overflow_action = "warn"\` to convert this error to a warning and still produce output for diagnosis.` -This error appears when table columns are too wide to fit on a single page -and `allow_col_split = FALSE`. +`writetfl` checks three width-overflow conditions and reports the first one +that fires: -The error message includes a per-column width breakdown so you can identify -which columns are consuming the most space. +1. **Page-level content grob too wide.** Any non-`ggplot`, non-character + content (e.g. `gt::as_gtable()`, an `rtables` text grob, + `gridExtra::tableGrob()`, a raw user grob) whose natural width exceeds + the content viewport. +2. **`tfl_table` total column width** (only when `allow_col_split = FALSE`). + The sum of column widths after wrapping still exceeds the content area. +3. **`tfl_table` per-column overflow.** Any single column wider than the + content area, or any data column whose width *plus* the row-header + group columns exceeds the page (group columns repeat on every + column-paginated page, so this case is unrenderable). + +The error message identifies which condition fired and includes a +per-column width breakdown for cases (2) and (3). **Solutions:** @@ -54,6 +66,30 @@ which columns are consuming the most space. `tfl_table(..., wrap_cols = c("wide_col1", "wide_col2"))` - Set narrower fixed widths on specific columns via `tfl_colspec()` - Increase page width or decrease margins +- Reduce the number of group columns (group columns repeat on every + column-paginated page and reserve space for themselves on every page) + +### Diagnosing by inspecting the broken layout + +When you can't immediately tell *why* the layout overflows, pass +`overflow_action = "warn"` to convert the error to a warning and still +produce a PDF. The PDF visibly shows the overflow (clipped by `grid` at +the page edge), which is often the fastest way to identify the offending +column or grob: + +```r +# Default: hard error, no PDF written +export_tfl(tbl, file = "out.pdf") +#> Error: Column 'description' width (12 in) exceeds available content width (7.5 in) +#> Set `overflow_action = "warn"` to convert this error to a warning ... + +# Warn-mode: PDF is written, warning printed for diagnosis +export_tfl(tbl, file = "out.pdf", overflow_action = "warn") +#> Warning: Column 'description' width (12 in) exceeds available content width (7.5 in) ... +``` + +Use `"warn"` for diagnosis only — it does not fix the layout; it just lets +you see what the renderer was forced to clip. ## Overlap warnings