Allow multi-line group labels to flow into suppressed rows#32
Merged
Conversation
When a group column has a multi-line value (e.g. "B\nC") that is then suppressed on subsequent rows of the same group, the label now flows visually through the blanked cells like an HTML rowspan instead of forcing the first row of the group to be tall enough to fit every line on its own. Implementation: * `measure_row_heights_tbl()` now returns a per-cell height matrix. * New helper `.compute_page_row_heights()` resolves per-row heights for a page by initialising rows from the non-group max and walking group columns innermost-first, growing the first row of each span only when the label exceeds the span's cumulative height. Innermost-first means outer spans borrow inner-pass growth. * `paginate_rows()` now drives the page-fit decision with a per-page tentative recompute. This is required because span heights are non-monotone in row count (adding a row to an open span can leave the page total unchanged) and because the orphan case — the first row of a multi-row group landing alone on a page boundary — must size that row to fit the full label by itself. `committed_rh` snapshots heights after each successful append so the flush captures the orphan-correct values. * `drawDetails.tfl_table_grob()` reads per-page committed heights from the page spec, computes a span-end matrix, and passes the cumulative span height to `.draw_cell_text()` for non-suppressed group cells so the clip viewport extends across the span. `row_rule` lines that would slice through a flowing label are suppressed. Closes /issues/29 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ize 1
With nested group_vars, the previous logic suppressed the group rule at
any transition where the new innermost group had only one row. That
hid legitimate outer-level boundaries — for example, with
group_vars = c("Cohort", "Visit"), the transition from the last Cohort 1
row to the first Cohort 2 row was suppressed whenever Cohort 2 happened
to start with a one-row Visit, leaving no visual separator at the
Cohort change.
New helper .compute_group_rule_sizes() returns, for each group_start
row, the size of the group at the *outermost* changing level
(group_vars[1..k] where k is the outermost level whose value changed
vs. the preceding row). drawDetails.tfl_table_grob() now uses this
size to decide whether to draw the rule.
Single-level grouping is unchanged: the outermost (and only) level is
the innermost level, so the size is the same as .compute_group_sizes().
Nested grouping where the outer level changes now correctly draws a
rule when the outer-level group has multiple rows, regardless of how
many rows the new innermost block happens to have.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p rules Adds a new tfl_table() argument simplify_rowspan, defaulting to FALSE so existing tables render byte-for-byte identically to the pre-issue-29 release. When simplify_rowspan = TRUE the four behaviour changes introduced in this branch all switch on together: * Span-aware row heights — multi-line group labels flow through the suppressed cells below them instead of inflating the first row. * Row-rule suppression within a multi-row span — a horizontal line that would slice a flowing label is skipped. * Partial-width group rules — the rule line starts at the column for the outermost group_var level that actually changed at the transition, so unchanged outer columns through which the label is flowing aren't visually divided. * Group rules drawn at every transition. The historical "skip rule when the new innermost group has size 1" check is bypassed; with partial widths and label-flow alignment, single-row transitions are visually unambiguous. Implementation: * tfl_table() gets the new argument plus checkmate flag validation; the print method reports it. * paginate_rows() gains simplify_rowspan and passes NULL to .compute_page_row_heights() when FALSE so the resolver short-circuits to the historical per-row max. * drawDetails.tfl_table_grob() reads tbl$simplify_rowspan and gates: the row-height resolver fallback path, span clipping, the row-rule suppression predicate, and the group-rule visibility/width logic. * New helper .compute_group_rule_info() returns both the outermost- changing-level size and the level index per group_start row; drawing reads the level to set the rule's left-edge column. Tests: existing row-span tests now pass simplify_rowspan = TRUE; new default-behaviour test verifies historical per-row max heights still apply when the flag is FALSE. .compute_group_rule_info() unit tests cover NA, single-level, multi-level, and empty inputs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
With simplify_rowspan = FALSE (the default), a multi-line group label
inflates only the row that displays it; the trailing rows where the
group cell is blanked by suppression are now sized only by their
non-group content. Previously each row's height was the per-row max
over every cell, including blanked group cells, so a 3-row group with
a 2-line label rendered as 3 rows of 2 lines each (6 lines) instead of
the new 2 + 1 + 1 = 4 lines.
Implementation: .compute_page_row_heights() gains a simplify_rowspan
parameter and three modes:
* No suppression — per-row max over all cells (unchanged).
* Suppression on, simplify_rowspan = FALSE — per-row max over the
non-suppressed cells (group cells contribute 0 when suppressed).
* Suppression on, simplify_rowspan = TRUE — span-aware flow as
introduced in the previous commit on this branch.
paginate_rows() and drawDetails.tfl_table_grob() now always pass
suppress_mat to the resolver and forward simplify_rowspan as an
explicit flag, so the resolver picks the right mode without callers
having to massage the suppress matrix.
Tables that don't use multi-line group labels render identically to
the pre-issue-29 release. Tables that did use them now produce
strictly more compact output and never cause overlap or clipping.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fy_rowspan flag Suppression and the rowspan flow are conceptually one thing: when a user asks for suppress_repeated_groups = TRUE they're asking for the visual where the multi-line label spans the suppressed cells beneath it, the row heights aren't inflated by the labelled-but-flowable column, the row rule doesn't slice through the flowing label, and the group rule starts at the column whose value actually changed. An earlier iteration of this branch added a simplify_rowspan flag to gate the flow + visual changes, but that left the package with three modes (suppression off / suppression on with flag off / suppression on with flag on) when one suffices. This commit removes the simplify_rowspan flag and makes the flow the sole behaviour whenever suppress_repeated_groups is TRUE. Setting suppress_repeated_groups = FALSE remains the way to opt out — every group cell renders on every row and the row max is taken over all cells (the strict per-row layout). * tfl_table() loses the simplify_rowspan parameter (and the validation, storage, and print-method line that went with it). * paginate_rows() and .compute_page_row_heights() lose the parameter; the resolver dispatches purely on whether suppress_mat is non-NULL. * drawDetails.tfl_table_grob() drops every isTRUE(tbl$simplify_rowspan) check; the span clipping, span-aware row-rule suppression, and the partial-width / always-drawn group rules all apply unconditionally when suppression is active. * Tests stop passing simplify_rowspan; the dedicated FALSE / TRUE test cases collapse into single-behaviour assertions. * Demo simplifies to one PDF per scenario (no off/on toggle). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a group column has a multi-line value (e.g. "B\nC") that is then suppressed on subsequent rows of the same group, the label now flows visually through the blanked cells like an HTML rowspan instead of forcing the first row of the group to be tall enough to fit every line on its own.
Implementation:
measure_row_heights_tbl()now returns a per-cell height matrix..compute_page_row_heights()resolves per-row heights for a page by initialising rows from the non-group max and walking group columns innermost-first, growing the first row of each span only when the label exceeds the span's cumulative height. Innermost-first means outer spans borrow inner-pass growth.paginate_rows()now drives the page-fit decision with a per-page tentative recompute. This is required because span heights are non-monotone in row count (adding a row to an open span can leave the page total unchanged) and because the orphan case — the first row of a multi-row group landing alone on a page boundary — must size that row to fit the full label by itself.committed_rhsnapshots heights after each successful append so the flush captures the orphan-correct values.drawDetails.tfl_table_grob()reads per-page committed heights from the page spec, computes a span-end matrix, and passes the cumulative span height to.draw_cell_text()for non-suppressed group cells so the clip viewport extends across the span.row_rulelines that would slice through a flowing label are suppressed.Closes #29