From e2211ad262931f1e6f4bf849bb093a33855e7d05 Mon Sep 17 00:00:00 2001 From: Gauarv Chaudhary Date: Sun, 4 Jan 2026 21:43:54 +0530 Subject: [PATCH 1/3] test: add regression test for issue #279 facet_wrap spacing Adds comprehensive test cases to verify that facet_wrap spacing scales proportionally with custom height parameter. Tests cover: - Vertical facet layout (ncol=1) with 2x and 3x custom heights - 2x2 grid layout with 1.5x custom height - Multiple height values to verify proportionality This ensures issue #279 (excessive spacing below facets) does not regress. All tests pass (16 assertions). Refs #279 --- ...sue-279-facet-wrap-custom-height-spacing.R | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R diff --git a/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R b/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R new file mode 100644 index 000000000..4394cefaf --- /dev/null +++ b/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R @@ -0,0 +1,171 @@ +context("Issue #279: facet_wrap spacing with custom height") + +test_that("facet_wrap vertical layout height scales proportionally with custom height parameter (#279)", { + skip_on_cran() + + # Create simple test data with 2 facets for vertical layout + # This setup is most likely to show excessive spacing issues + test_data <- data.frame( + x = c(1, 5, 10, 1, 5, 10), + y = c(1, 5, 10, 1, 5, 10), + category = c("A", "A", "A", "B", "B", "B") + ) + + # Test Case 1: Default height (baseline for comparison) + viz_default <- ggplot() + + geom_point(aes(x, y), data = test_data, size = 3) + + facet_wrap(~category, ncol = 1) + # Vertical layout + ggtitle("Default height") + + # Test Case 2: Custom height (2x default) + viz_custom_800 <- ggplot() + + geom_point(aes(x, y), data = test_data, size = 3) + + facet_wrap(~category, ncol = 1) + + theme_animint(height = 800) + # 2x default 400 + ggtitle("Custom height 800") + + # Test Case 3: Custom height (3x default) + viz_custom_1200 <- ggplot() + + geom_point(aes(x, y), data = test_data, size = 3) + + facet_wrap(~category, ncol = 1) + + theme_animint(height = 1200) + # 3x default 400 + ggtitle("Custom height 1200") + + # Render all three visualizations + dir_default <- tempfile() + dir_800 <- tempfile() + dir_1200 <- tempfile() + + info_default <- animint2dir( + list(plot = viz_default), + out.dir = dir_default, + open.browser = FALSE + ) + + info_800 <- animint2dir( + list(plot = viz_custom_800), + out.dir = dir_800, + open.browser = FALSE + ) + + info_1200 <- animint2dir( + list(plot = viz_custom_1200), + out.dir = dir_1200, + open.browser = FALSE + ) + + # Read the generated plot.json files to verify height settings + json_default <- jsonlite::fromJSON(file.path(dir_default, "plot.json")) + json_800 <- jsonlite::fromJSON(file.path(dir_800, "plot.json")) + json_1200 <- jsonlite::fromJSON(file.path(dir_1200, "plot.json")) + + # Verify that custom heights are correctly stored in plot.json + expect_equal(json_800$plots$plot$options$height, 800) + expect_equal(json_1200$plots$plot$options$height, 1200) + + # The bug in issue #279 was: excessive vertical spacing below facets + # when using custom height != 400 (default) + # Expected behavior: spacing should scale proportionally, not excessively + + # We verify this indirectly by checking that the layout information + # is consistent across different height values + + # Check that panel layout is identical (same number of rows/cols) + expect_equal(json_default$plots$plot$layout$ROW, json_800$plots$plot$layout$ROW) + expect_equal(json_default$plots$plot$layout$ROW, json_1200$plots$plot$layout$ROW) + expect_equal(json_default$plots$plot$layout$COL, json_800$plots$plot$layout$COL) + expect_equal(json_default$plots$plot$layout$COL, json_1200$plots$plot$layout$COL) + + # Check that height_proportion values are identical + # (these control relative spacing, not absolute) + expect_equal(json_default$plots$plot$layout$height_proportion, + json_800$plots$plot$layout$height_proportion) + expect_equal(json_default$plots$plot$layout$height_proportion, + json_1200$plots$plot$layout$height_proportion) +}) + +test_that("facet_wrap grid layout (2x2) with custom height has proportional spacing (#279)", { + skip_on_cran() + + # Test with a 2x2 grid layout (similar to ROC example in issue) + test_data_grid <- data.frame( + x = rep(1:10, 4), + y = rep(1:10, 4), + category = rep(c("A", "B", "C", "D"), each = 10) + ) + + # Default height + viz_default <- ggplot() + + geom_point(aes(x, y), data = test_data_grid) + + facet_wrap(~category, ncol = 2) + + # Custom height (1.5x default) + viz_custom <- ggplot() + + geom_point(aes(x, y), data = test_data_grid) + + facet_wrap(~category, ncol = 2) + + theme_animint(height = 600) + + dir_default <- tempfile() + dir_custom <- tempfile() + + info_default <- animint2dir(list(plot = viz_default), out.dir = dir_default, open.browser = FALSE) + info_custom <- animint2dir(list(plot = viz_custom), out.dir = dir_custom, open.browser = FALSE) + + # Verify custom height is applied + json_custom <- jsonlite::fromJSON(file.path(dir_custom, "plot.json")) + expect_equal(json_custom$plots$plot$options$height, 600) + + # Check that layout proportions are consistent + json_default <- jsonlite::fromJSON(file.path(dir_default, "plot.json")) + expect_equal(json_default$plots$plot$layout$height_proportion, + json_custom$plots$plot$layout$height_proportion) + + # Verify we have 2 rows and 2 columns as expected + expect_equal(max(json_custom$plots$plot$layout$ROW), 2) + expect_equal(max(json_custom$plots$plot$layout$COL), 2) +}) + +test_that("facet_wrap with multiple custom height values maintains proportional spacing (#279)", { + skip_on_cran() + + # Test that spacing proportions are maintained across various custom heights + test_data <- data.frame( + x = c(1, 5, 10, 1, 5, 10), + y = c(1, 5, 10, 1, 5, 10), + category = c("X", "X", "X", "Y", "Y", "Y") + ) + + heights_to_test <- c(400, 600, 800, 1000, 1200) + layout_proportions <- list() + + for (i in seq_along(heights_to_test)) { + h <- heights_to_test[i] + + if (h == 400) { + # Default height + viz <- ggplot() + + geom_point(aes(x, y), data = test_data) + + facet_wrap(~category, ncol = 1) + } else { + # Custom height + viz <- ggplot() + + geom_point(aes(x, y), data = test_data) + + facet_wrap(~category, ncol = 1) + + theme_animint(height = h) + } + + test_dir <- tempfile() + info <- animint2dir(list(plot = viz), out.dir = test_dir, open.browser = FALSE) + + # Extract layout proportions + json <- jsonlite::fromJSON(file.path(test_dir, "plot.json")) + layout_proportions[[i]] <- json$plots$plot$layout$height_proportion + } + + # All layout proportions should be identical + # (height should scale, but proportions should remain constant) + for (i in 2:length(layout_proportions)) { + expect_equal(layout_proportions[[1]], layout_proportions[[i]], + info = sprintf("Height %d proportions differ from default (400)", heights_to_test[i])) + } +}) From a6599292fa9624951b5a2a64a6fea838ea672034 Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Tue, 10 Mar 2026 21:00:18 +0530 Subject: [PATCH 2/3] fix #279: set SVG display:block to eliminate inline whitespace below facets; replace JSON-only test with browser-based regression test - Add .style("display","block") to SVG creation in animint.js so browsers do not add text-descender whitespace below inline SVG elements (fixes visible gap below facet_wrap/facet_grid at any custom height). - Replace the three animint2dir+JSON tests with a single animint2HTML test that renders in a real browser and asserts SVG height stays proportional (not height*num_facets) when using theme_animint(height=600) on a facet_grid(task_id ~ .) plot with sonar/spam/vowel/waveform/zip data, matching the ch20 examples that originally exposed the bug. --- inst/htmljs/animint.js | 3 +- ...sue-279-facet-wrap-custom-height-spacing.R | 197 +++--------------- 2 files changed, 36 insertions(+), 164 deletions(-) diff --git a/inst/htmljs/animint.js b/inst/htmljs/animint.js index bfcc4b07f..f49c74908 100644 --- a/inst/htmljs/animint.js +++ b/inst/htmljs/animint.js @@ -283,7 +283,8 @@ var animint = function (to_select, json_file) { var svg = tdLeft.append("svg") .attr("id", p_info.plot_id) .attr("height", p_info.options.height) - .attr("width", p_info.options.width); + .attr("width", p_info.options.width) + .style("display", "block"); // divvy up width/height based on the panel layout var nrows = Math.max.apply(null, p_info.layout.ROW); diff --git a/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R b/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R index 4394cefaf..08b91efbe 100644 --- a/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R +++ b/tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R @@ -1,171 +1,42 @@ -context("Issue #279: facet_wrap spacing with custom height") +acontext("Issue #279: facet_grid spacing with custom height") -test_that("facet_wrap vertical layout height scales proportionally with custom height parameter (#279)", { +test_that("facet_grid SVG height is proportional to theme_animint height, no excess space (#279)", { skip_on_cran() - - # Create simple test data with 2 facets for vertical layout - # This setup is most likely to show excessive spacing issues - test_data <- data.frame( - x = c(1, 5, 10, 1, 5, 10), - y = c(1, 5, 10, 1, 5, 10), - category = c("A", "A", "A", "B", "B", "B") - ) - - # Test Case 1: Default height (baseline for comparison) - viz_default <- ggplot() + - geom_point(aes(x, y), data = test_data, size = 3) + - facet_wrap(~category, ncol = 1) + # Vertical layout - ggtitle("Default height") - - # Test Case 2: Custom height (2x default) - viz_custom_800 <- ggplot() + - geom_point(aes(x, y), data = test_data, size = 3) + - facet_wrap(~category, ncol = 1) + - theme_animint(height = 800) + # 2x default 400 - ggtitle("Custom height 800") - - # Test Case 3: Custom height (3x default) - viz_custom_1200 <- ggplot() + - geom_point(aes(x, y), data = test_data, size = 3) + - facet_wrap(~category, ncol = 1) + - theme_animint(height = 1200) + # 3x default 400 - ggtitle("Custom height 1200") - - # Render all three visualizations - dir_default <- tempfile() - dir_800 <- tempfile() - dir_1200 <- tempfile() - - info_default <- animint2dir( - list(plot = viz_default), - out.dir = dir_default, - open.browser = FALSE - ) - - info_800 <- animint2dir( - list(plot = viz_custom_800), - out.dir = dir_800, - open.browser = FALSE - ) - - info_1200 <- animint2dir( - list(plot = viz_custom_1200), - out.dir = dir_1200, - open.browser = FALSE + + task_data <- data.frame( + x = rep(1:5, 5), + y = rep(1:5, 5), + task_id = rep(c("sonar", "spam", "vowel", "waveform", "zip"), each = 5) ) - - # Read the generated plot.json files to verify height settings - json_default <- jsonlite::fromJSON(file.path(dir_default, "plot.json")) - json_800 <- jsonlite::fromJSON(file.path(dir_800, "plot.json")) - json_1200 <- jsonlite::fromJSON(file.path(dir_1200, "plot.json")) - - # Verify that custom heights are correctly stored in plot.json - expect_equal(json_800$plots$plot$options$height, 800) - expect_equal(json_1200$plots$plot$options$height, 1200) - - # The bug in issue #279 was: excessive vertical spacing below facets - # when using custom height != 400 (default) - # Expected behavior: spacing should scale proportionally, not excessively - - # We verify this indirectly by checking that the layout information - # is consistent across different height values - - # Check that panel layout is identical (same number of rows/cols) - expect_equal(json_default$plots$plot$layout$ROW, json_800$plots$plot$layout$ROW) - expect_equal(json_default$plots$plot$layout$ROW, json_1200$plots$plot$layout$ROW) - expect_equal(json_default$plots$plot$layout$COL, json_800$plots$plot$layout$COL) - expect_equal(json_default$plots$plot$layout$COL, json_1200$plots$plot$layout$COL) - - # Check that height_proportion values are identical - # (these control relative spacing, not absolute) - expect_equal(json_default$plots$plot$layout$height_proportion, - json_800$plots$plot$layout$height_proportion) - expect_equal(json_default$plots$plot$layout$height_proportion, - json_1200$plots$plot$layout$height_proportion) -}) -test_that("facet_wrap grid layout (2x2) with custom height has proportional spacing (#279)", { - skip_on_cran() - - # Test with a 2x2 grid layout (similar to ROC example in issue) - test_data_grid <- data.frame( - x = rep(1:10, 4), - y = rep(1:10, 4), - category = rep(c("A", "B", "C", "D"), each = 10) + viz_default <- list( + plot = ggplot() + + geom_point(aes(x, y), data = task_data) + + facet_grid(task_id ~ .) + + theme_bw() ) - - # Default height - viz_default <- ggplot() + - geom_point(aes(x, y), data = test_data_grid) + - facet_wrap(~category, ncol = 2) - - # Custom height (1.5x default) - viz_custom <- ggplot() + - geom_point(aes(x, y), data = test_data_grid) + - facet_wrap(~category, ncol = 2) + - theme_animint(height = 600) - - dir_default <- tempfile() - dir_custom <- tempfile() - - info_default <- animint2dir(list(plot = viz_default), out.dir = dir_default, open.browser = FALSE) - info_custom <- animint2dir(list(plot = viz_custom), out.dir = dir_custom, open.browser = FALSE) - - # Verify custom height is applied - json_custom <- jsonlite::fromJSON(file.path(dir_custom, "plot.json")) - expect_equal(json_custom$plots$plot$options$height, 600) - - # Check that layout proportions are consistent - json_default <- jsonlite::fromJSON(file.path(dir_default, "plot.json")) - expect_equal(json_default$plots$plot$layout$height_proportion, - json_custom$plots$plot$layout$height_proportion) - - # Verify we have 2 rows and 2 columns as expected - expect_equal(max(json_custom$plots$plot$layout$ROW), 2) - expect_equal(max(json_custom$plots$plot$layout$COL), 2) -}) -test_that("facet_wrap with multiple custom height values maintains proportional spacing (#279)", { - skip_on_cran() - - # Test that spacing proportions are maintained across various custom heights - test_data <- data.frame( - x = c(1, 5, 10, 1, 5, 10), - y = c(1, 5, 10, 1, 5, 10), - category = c("X", "X", "X", "Y", "Y", "Y") + viz_custom <- list( + plot = ggplot() + + geom_point(aes(x, y), data = task_data) + + facet_grid(task_id ~ .) + + theme_bw() + + theme_animint(height = 600) ) - - heights_to_test <- c(400, 600, 800, 1000, 1200) - layout_proportions <- list() - - for (i in seq_along(heights_to_test)) { - h <- heights_to_test[i] - - if (h == 400) { - # Default height - viz <- ggplot() + - geom_point(aes(x, y), data = test_data) + - facet_wrap(~category, ncol = 1) - } else { - # Custom height - viz <- ggplot() + - geom_point(aes(x, y), data = test_data) + - facet_wrap(~category, ncol = 1) + - theme_animint(height = h) - } - - test_dir <- tempfile() - info <- animint2dir(list(plot = viz), out.dir = test_dir, open.browser = FALSE) - - # Extract layout proportions - json <- jsonlite::fromJSON(file.path(test_dir, "plot.json")) - layout_proportions[[i]] <- json$plots$plot$layout$height_proportion - } - - # All layout proportions should be identical - # (height should scale, but proportions should remain constant) - for (i in 2:length(layout_proportions)) { - expect_equal(layout_proportions[[1]], layout_proportions[[i]], - info = sprintf("Height %d proportions differ from default (400)", heights_to_test[i])) - } + + info_default <- animint2HTML(viz_default) + info_custom <- animint2HTML(viz_custom) + + svg_default <- XML::getNodeSet(info_default$html, "//svg[contains(@id,'plot_plot')]") + expect_equal(length(svg_default), 1L) + h_default <- as.numeric(XML::xmlAttrs(svg_default[[1]])[["height"]]) + expect_lt(h_default, 400 * 2) + + svg_custom <- XML::getNodeSet(info_custom$html, "//svg[contains(@id,'plot_plot')]") + expect_equal(length(svg_custom), 1L) + h_custom <- as.numeric(XML::xmlAttrs(svg_custom[[1]])[["height"]]) + + expect_lt(h_custom, 600 * 2, + label = "SVG height should not be 600*num_facets — regression from issue #279") + expect_gt(h_custom, h_default) }) From 98f7efa622ea3397641964903b193310efdd84aa Mon Sep 17 00:00:00 2001 From: Gaurav Chaudhary Date: Thu, 19 Mar 2026 06:30:25 +0530 Subject: [PATCH 3/3] fix CI: use robust SVG id extraction to avoid matrix dimension error The display:block style added to SVGs causes sapply(svg.list, xmlAttrs) to return a list instead of a matrix when SVGs have inconsistent attribute counts. Use lapply/sapply with a function to extract id attributes safely regardless of attribute structure. --- tests/testthat/test-renderer3-knit-print.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-renderer3-knit-print.R b/tests/testthat/test-renderer3-knit-print.R index e732b42c8..ba69fb115 100644 --- a/tests/testthat/test-renderer3-knit-print.R +++ b/tests/testthat/test-renderer3-knit-print.R @@ -45,8 +45,9 @@ test_that("segments and breakpoints are rendered", { test_that("svg id property is unique", { svg.list <- getNodeSet(html, "//svg") - attr.mat <- sapply(svg.list, xmlAttrs) - id.counts <- table(attr.mat["id",]) + attr.list <- lapply(svg.list, xmlAttrs) + id.vec <- sapply(attr.list, function(a) a[["id"]]) + id.counts <- table(id.vec) expect_true(all(id.counts==1)) })