test: add regression tests for issue #279 (facet_wrap spacing with custom height) by ANAMASGARD · Pull Request #288 · animint/animint2

ANAMASGARD · 2026-01-04T16:20:59Z

Summary

Adds comprehensive regression tests for issue #279 to prevent reoccurrence of excessive spacing below facet_wrap plots when using custom height parameters.

Background

Issue #279 reported that when using facet_wrap() with a custom height parameter (different from the default 400px), excessive vertical spacing appeared below the faceted plots. The spacing did not scale proportionally with the custom height.

After thorough investigation and testing with version 2025.12.4, this issue appears to be resolved. These tests ensure it doesn't regress in future updates.

Changes

Added test-issue-279-facet-wrap-custom-height-spacing.R
3 comprehensive test cases with 16 assertions
Tests cover:
- Vertical facet layout (ncol=1) with 2x and 3x custom heights (800, 1200)
- Grid layout (2×2, ncol=2) with 1.5x custom height (600)
- Multiple height values (400, 600, 800, 1000, 1200) to verify proportionality

What the Tests Verify

The key verification is that height_proportion values remain identical across all custom heights:

Custom height parameters are correctly stored in plot.json
Panel layout structure (ROW/COL) remains consistent
height_proportion values are identical regardless of custom height
- These values control relative spacing between facets
- If excessive spacing existed, these would differ with custom heights
- Consistent proportions = correct proportional scaling ✓

Testing

All tests pass on current main branch :-

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

codecov · 2026-01-04T16:40:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.03%. Comparing base (426295c) to head (0d1a83d).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #288   +/-   ##
=======================================
  Coverage   73.03%   73.03%           
=======================================
  Files         164      164           
  Lines        8840     8841    +1     
=======================================
+ Hits         6456     6457    +1     
  Misses       2384     2384

Flag	Coverage Δ
javascript	`81.26% <100.00%> (+<0.01%)`	⬆️
r	`69.14% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tdhock · 2026-02-27T17:09:17Z

can you please post a screenshot of the proposed test?

…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.

ANAMASGARD · 2026-03-10T15:44:03Z

localhost_8788_issue279-facet-height-600_index html(Surface Pro 7)

tdhock · 2026-03-10T18:03:57Z

please fix CI

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.

tdhock · 2026-05-20T18:00:04Z

+acontext("Issue #279: facet_grid spacing with custom height")
+
+test_that("facet_grid SVG height is proportional to theme_animint height, no excess space (#279)", {
+  skip_on_cran()


please remove, not necessary

please remove

please remove skip

- use facet_wrap instead of facet_grid (matches original bug report) - remove acontext() call as requested - remove empty lines inside test_that block - introduce n_facets variable and use 400*n_facets / 600*n_facets bounds to make the regression guard self-explanatory - refactor with viz_list + lapply to eliminate code duplication

ANAMASGARD · 2026-05-26T11:08:19Z

Sir @tdhock, I've addressed all four review comments:

Removed the acontext() call
Removed empty lines inside the test_that block
Switched from facet_grid to facet_wrap to match the original bug report in issue too much space below facet wrap #279
Introduced n_facets and updated bounds to 400 * n_facets / 600 * n_facets to make the regression guard self-explanatory
Refactored using viz_list + lapply to eliminate duplicated code

All 5 assertions pass locally (PASS 5). But when i pushed the commit I first had to rebase it so I rebased this branch then when I pushed the github CI , should run automatically but it is not working ??

tdhock · 2026-05-28T14:43:27Z

+acontext("Issue #279: facet_grid spacing with custom height")
+
+test_that("facet_grid SVG height is proportional to theme_animint height, no excess space (#279)", {
+  skip_on_cran()


please remove

ANAMASGARD · 2026-05-29T09:23:03Z

Sir @tdhock can you please review and give your feedback I have removed the

acontext("Issue #279: facet_grid spacing with custom height")

as you said .

Restore test-renderer3-knit-print.R to master, remove skip_on_cran(), and assert plot SVG uses display:block via getComputedStyle.

sapply(xmlAttrs) no longer simplifies to a matrix when plot SVGs have style attributes from the issue #279 fix. Use getPropertyValue().

tdhock · 2026-07-29T19:46:25Z

+      display = runtime_evaluate(sprintf(
+        "window.getComputedStyle(document.getElementById('%s')).display", svg_id))
+    )
+  })
+  expect_equal(results$default$display, "block",
+    label = "plot SVG should use display:block to prevent whitespace below it")
+  expect_equal(results$custom$display, "block",
+    label = "plot SVG should use display:block to prevent whitespace below it")
+  h_list <- lapply(results, `[[`, "height")
+  expect_lt(h_list$default, 400 * n_facets)
+  expect_lt(h_list$custom, 600 * n_facets,
+    label = "SVG height should not be 600*num_facets — regression from issue #279")
+  expect_gt(h_list$custom, h_list$default)
+  bottom_gap <- runtime_evaluate("(() => { const svg = document.querySelector(\"svg[id*='plot_plot']\"); const parent = svg.parentElement; return parent.getBoundingClientRect().bottom - svg.getBoundingClientRect().bottom; })()")


can you please use existing R helper functions instead of runtime evaluate with custom js code?

Sir @tdhock I have removed all custom runtime_evaluate JS from test file
The test now uses getStyleValue() for inline display:block and get_element_bbox() for the parent–SVG gap check (both default and custom height).
Passes locally (9/9). Please re-review when you have time thanks .

Replace custom runtime_evaluate JS with existing R helpers per review. Assert display:block and bottom_gap for both default and custom heights.

ANAMASGARD and others added 2 commits March 10, 2026 15:50

Merge branch 'master' into fix-issue-279-facet-height-spacing

31acddd

ANAMASGARD and others added 2 commits March 11, 2026 05:48

Merge branch 'master' into fix-issue-279-facet-height-spacing

260c26f

ANAMASGARD requested a review from tdhock March 19, 2026 04:03

ANAMASGARD and others added 3 commits April 7, 2026 11:37

Merge branch 'master' into fix-issue-279-facet-height-spacing

f4b796e

Merge branch 'master' into fix-issue-279-facet-height-spacing

3d0a90f

Merge branch 'master' into fix-issue-279-facet-height-spacing

74c745a

ANAMASGARD mentioned this pull request May 18, 2026

Gaurav's GSoC 2026 Updates #327

Open

tdhock reviewed May 20, 2026

View reviewed changes

tdhock requested changes May 20, 2026

View reviewed changes

ANAMASGARD and others added 2 commits May 26, 2026 14:00

Merge branch 'master' into fix-issue-279-facet-height-spacing

74b3f42

ANAMASGARD requested a review from tdhock May 26, 2026 10:54

ci: trigger GitHub Actions

c48f242

tdhock requested changes May 28, 2026

View reviewed changes

Merge branch 'master' into fix-issue-279-facet-height-spacing

acce7a2

ANAMASGARD requested a review from tdhock May 29, 2026 09:21

Merge branch 'master' into fix-issue-279-facet-height-spacing

6a99013

tdhock requested changes Jun 29, 2026

View reviewed changes

Comment thread tests/testthat/test-renderer3-knit-print.R Outdated

ANAMASGARD and others added 3 commits June 29, 2026 20:34

Merge branch 'master' into fix-issue-279-facet-height-spacing

6f2a5fe

test: address review feedback and guard display:block fix for #279

4112796

Restore test-renderer3-knit-print.R to master, remove skip_on_cran(), and assert plot SVG uses display:block via getComputedStyle.

fix: robust SVG id extraction after display:block attrs change

bdc834f

sapply(xmlAttrs) no longer simplifies to a matrix when plot SVGs have style attributes from the issue #279 fix. Use getPropertyValue().

ANAMASGARD requested a review from tdhock July 29, 2026 04:49

tdhock requested changes Jul 29, 2026

View reviewed changes

ANAMASGARD and others added 2 commits July 30, 2026 07:12

Merge branch 'master' into fix-issue-279-facet-height-spacing

246f9a3

test: use getStyleValue and get_element_bbox for issue #279

0d1a83d

Replace custom runtime_evaluate JS with existing R helpers per review. Assert display:block and bottom_gap for both default and custom heights.

Uh oh!

Conversation

ANAMASGARD commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Changes

What the Tests Verify

Testing

Uh oh!

codecov Bot commented Jan 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tdhock commented Feb 27, 2026

Uh oh!

ANAMASGARD commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdhock commented Mar 10, 2026

Uh oh!

tdhock May 20, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock May 28, 2026

Choose a reason for hiding this comment

Uh oh!

tdhock Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ANAMASGARD commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdhock May 28, 2026

Choose a reason for hiding this comment

Uh oh!

ANAMASGARD commented May 29, 2026

Uh oh!

Uh oh!

tdhock Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

ANAMASGARD Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ANAMASGARD commented Jan 4, 2026 •

edited

Loading

codecov Bot commented Jan 4, 2026 •

edited

Loading

ANAMASGARD commented Mar 10, 2026 •

edited

Loading

ANAMASGARD commented May 26, 2026 •

edited

Loading