Skip to content

Commit 8344ddf

Browse files
authored
Merge pull request #2113 from rstudio/fix-body-styling-with-multicolumn-stub
fix: body styling with `columns = everything()` can miss columns when a multi-column stub is present
2 parents 982fa30 + d9c2602 commit 8344ddf

File tree

5 files changed

+302
-81
lines changed

5 files changed

+302
-81
lines changed

R/utils_render_grid.R

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -779,10 +779,11 @@ summary_rows_g <- function(data, group_id, side_grand_summary = "bottom") {
779779
}
780780
}
781781

782+
# For summary rows, use 1 stub column (the summary label column)
782783
row_styles <-
783784
build_row_styles(
784785
styles_resolved_row = styles_resolved_row,
785-
include_stub = TRUE,
786+
n_stub_cols = 1L,
786787
n_cols = n_data_cols
787788
)
788789

R/utils_render_html.R

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,15 @@ create_body_component_h <- function(data) {
11001100
has_stub_column <- "rowname" %in% stub_layout
11011101
has_two_col_stub <- "group_label" %in% stub_layout
11021102

1103+
# Calculate the actual number of stub columns for styling purposes
1104+
# This accounts for multi-column stubs (e.g., rowname_col = c("col1", "col2"))
1105+
stub_vars <- dt_boxhead_get_var_stub(data = data)
1106+
n_stub_cols <- if (has_stub_column && !all(is.na(stub_vars))) {
1107+
length(stub_vars)
1108+
} else {
1109+
0L
1110+
}
1111+
11031112
# Create ID components for every column that will be rendered
11041113
col_names_id <-
11051114
c(
@@ -1432,7 +1441,7 @@ create_body_component_h <- function(data) {
14321441
build_row_styles_with_stub_columns(
14331442
styles_resolved_row = styles_row,
14341443
stub_column_styles = stub_column_styles,
1435-
include_stub = has_stub_column,
1444+
n_stub_cols = n_stub_cols,
14361445
n_cols = n_data_cols,
14371446
data = data
14381447
)
@@ -2174,10 +2183,12 @@ summary_rows_for_group_h <- function(
21742183
}
21752184
}
21762185

2186+
# For summary rows, use 1 stub column (the summary label column)
2187+
# unless we have a two-column stub which is handled separately
21772188
row_styles <-
21782189
build_row_styles(
21792190
styles_resolved_row = styles_resolved_row,
2180-
include_stub = TRUE,
2191+
n_stub_cols = 1L,
21812192
n_cols = n_data_cols
21822193
)
21832194

@@ -2291,7 +2302,7 @@ summary_rows_for_group_h <- function(
22912302

22922303
build_row_styles <- function(
22932304
styles_resolved_row,
2294-
include_stub,
2305+
n_stub_cols,
22952306
n_cols
22962307
) {
22972308

@@ -2301,33 +2312,38 @@ build_row_styles <- function(
23012312
# styles_resolved_row, and it's OK for a column not to appear in
23022313
# styles_resolved_row, and it's OK for styles_resolved_row to have 0 rows.
23032314
#
2304-
# If `include_stub` is TRUE, then a row with column==0 will be used as the
2305-
# stub style.
2315+
# If `n_stub_cols` > 0, then rows with colnum <= 0 will be used for stub
2316+
# styles. For multi-column stubs, colnum values can be negative to indicate
2317+
# which stub column (with more negative = more leftward).
23062318

23072319
# This function's implementation can't tolerate colnum of NA, or illegal
23082320
# colnum values. Check and throw early.
23092321
if (
2310-
!isTRUE(all(styles_resolved_row$colnum %in% c(0, seq_len(n_cols)))) ||
2322+
!isTRUE(all(styles_resolved_row$colnum %in% c(seq(-n_stub_cols + 1, 0), seq_len(n_cols)))) ||
23112323
anyDuplicated(styles_resolved_row$colnum) > 0L
23122324
) {
23132325
cli::cli_abort(
23142326
"`build_row_styles()` was called with invalid `colnum` values."
23152327
)
23162328
}
23172329

2318-
n_cols <- n_cols + include_stub
2319-
result <- rep_len(NA_character_, n_cols)
2330+
n_total_cols <- n_cols + n_stub_cols
2331+
result <- rep_len(NA_character_, n_total_cols)
23202332

23212333
# The subset of styles_resolved_row that applies to data
23222334
idx <- styles_resolved_row$colnum > 0
2323-
result[styles_resolved_row$colnum[idx] + include_stub] <- styles_resolved_row$html_style[idx]
2335+
result[styles_resolved_row$colnum[idx] + n_stub_cols] <- styles_resolved_row$html_style[idx]
23242336

2325-
# If a stub exists, we need to prepend a style (or NULL) to the result.
2326-
if (include_stub) {
2337+
# If stub columns exist, apply stub styles
2338+
if (n_stub_cols > 0) {
2339+
# Handle colnum == 0 (applies to all stub columns for backward compatibility)
23272340
idx_0 <- styles_resolved_row$colnum == 0
23282341
stub_style <- styles_resolved_row$html_style[idx_0]
23292342
if (!is_empty(stub_style)) {
2330-
result[1] <- stub_style
2343+
# Apply to all stub columns
2344+
for (i in seq_len(n_stub_cols)) {
2345+
result[i] <- stub_style
2346+
}
23312347
}
23322348
}
23332349

@@ -2337,20 +2353,20 @@ build_row_styles <- function(
23372353
build_row_styles_with_stub_columns <- function(
23382354
styles_resolved_row,
23392355
stub_column_styles,
2340-
include_stub,
2356+
n_stub_cols,
23412357
n_cols,
23422358
data
23432359
) {
23442360

23452361
# First, build normal row styles
23462362
row_styles <- build_row_styles(
23472363
styles_resolved_row = styles_resolved_row,
2348-
include_stub = include_stub,
2364+
n_stub_cols = n_stub_cols,
23492365
n_cols = n_cols
23502366
)
23512367

2352-
# If we have stub column styles and a stub exists, modify the stub styles
2353-
if (include_stub && nrow(stub_column_styles) > 0) {
2368+
# If we have stub column styles and stub columns exist, modify the stub styles
2369+
if (n_stub_cols > 0 && nrow(stub_column_styles) > 0) {
23542370

23552371
# Get stub variables to map column names to positions
23562372
stub_vars <- dt_boxhead_get_var_stub(data = data)
@@ -2385,7 +2401,7 @@ build_row_styles_with_stub_columns <- function(
23852401
}
23862402

23872403
# For multi-column stubs, modify border widths for internal columns
2388-
if (include_stub) {
2404+
if (n_stub_cols > 0) {
23892405
stub_vars <- dt_boxhead_get_var_stub(data = data)
23902406

23912407
if (length(stub_vars) > 1 && !all(is.na(stub_vars))) {

tests/testthat/test-fmt_date_time.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
skip_on_os("linux")
22

3-
library(lubridate)
3+
suppressPackageStartupMessages(library(lubridate))
44

55
test_that("fmt_date() works correctly", {
66

0 commit comments

Comments
 (0)