Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e2211ad
test: add regression test for issue #279 facet_wrap spacing
ANAMASGARD Jan 4, 2026
31acddd
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Mar 10, 2026
a659929
fix #279: set SVG display:block to eliminate inline whitespace below …
ANAMASGARD Mar 10, 2026
260c26f
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Mar 11, 2026
98f7efa
fix CI: use robust SVG id extraction to avoid matrix dimension error
ANAMASGARD Mar 19, 2026
f4b796e
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Apr 7, 2026
3d0a90f
Merge branch 'master' into fix-issue-279-facet-height-spacing
tdhock May 7, 2026
74c745a
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD May 18, 2026
74b3f42
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD May 26, 2026
e19b3b5
test: addresses Sir tdhock review comments for issue #279
ANAMASGARD May 26, 2026
c48f242
ci: trigger GitHub Actions
ANAMASGARD May 26, 2026
acce7a2
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD May 29, 2026
6a99013
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Jun 12, 2026
6f2a5fe
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Jun 29, 2026
4112796
test: address review feedback and guard display:block fix for #279
ANAMASGARD Jul 21, 2026
bdc834f
fix: robust SVG id extraction after display:block attrs change
ANAMASGARD Jul 25, 2026
246f9a3
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Jul 30, 2026
0d1a83d
test: use getStyleValue and get_element_bbox for issue #279
ANAMASGARD Jul 30, 2026
da904bc
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Aug 14, 2026
d7d25c1
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Aug 20, 2026
ab5483f
test: address tdhock review for issue #279
ANAMASGARD Aug 21, 2026
417dc31
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Aug 25, 2026
905fc4e
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Sep 4, 2026
15eb57a
fix #279: document SVG display:block and bump version
ANAMASGARD Sep 4, 2026
4bac89a
Merge branch 'master' into fix-issue-279-facet-height-spacing
ANAMASGARD Sep 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# Changes in version 2026.9.4 (PR#288)

- Plot SVGs use `display: block` so browsers do not leave baseline whitespace below the plot (issue #279). Thanks @ANAMASGARD.

# Changes in development (PR#339)

# Changes in version 2026.9.5

- geom_text() renders with .text(), instead of setMultilineText(), fixing interactive updates.
Expand Down
4 changes: 3 additions & 1 deletion inst/htmljs/animint.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,9 @@ var setMultilineText = function(textElement, text) {
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)
// block avoids inline SVG baseline/descender gap below the plot (issue #279)
.style("display", "block");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to add display block?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plot SVGs are inline by default, so the browser leaves a little space under them for text descenders. That shows up as whitespace below the plot (#279). Setting display: block removes that gap without changing facet layout. Locally the gap was about 5px without it and 1px with it. Also added NEWS and a DESCRIPTION version bump.


issue279-facet-heights-600-screenshot

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, if I understand correctly, display: block reduces the space between svgs? is that when there are multiple ggplots or multiple facet panels? can this PR work without the display: block? or is adding that necessary to get it to work? if it is necessary, please explain why in NEWS? If not, please remove from this PR.


// divvy up width/height based on the panel layout
var nrows = Math.max.apply(null, p_info.layout.ROW);
Expand Down
43 changes: 43 additions & 0 deletions tests/testthat/test-issue-279-facet-wrap-custom-height-spacing.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
test_that("facet_wrap SVG height is proportional to theme_animint height, no excess space (#279)", {
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)
)
n_facets <- length(unique(task_data$task_id))
default_theme_height <- 400
custom_theme_height <- 600
svg_xpath <- "//svg[@id='plot_plot']"
svg_sel <- "svg#plot_plot"
parent_sel <- "td:has(> svg#plot_plot)"
base_plot <- ggplot() +
geom_point(aes(x, y), data = task_data) +
facet_wrap(~ task_id, ncol = 1) +
theme_bw()
viz_list <- list(
default = animint(plot = base_plot),
custom = animint(plot = base_plot + theme_animint(height = custom_theme_height))
)
Comment on lines +17 to +20

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use animint() instead of list() ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced list(plot = ...) with animint(plot = ...)

heights <- list()
for (viz_name in names(viz_list)) {
info <- animint2HTML(viz_list[[viz_name]])
svg_node <- getNodeSet(info$html, svg_xpath)
expect_equal(length(svg_node), 1L)
svg_attrs <- xmlAttrs(svg_node[[1]])
display <- getStyleValue(info$html, svg_xpath, "display")
expect_equal(display, "block",
label = "plot SVG should have inline display:block to prevent whitespace below it")
svg_box <- get_element_bbox(svg_sel)
parent_box <- get_element_bbox(parent_sel)
bottom_gap <- (parent_box$top + parent_box$height) - (svg_box$top + svg_box$height)
expect_lte(bottom_gap, 1,
label = "no excess rendered gap below plot SVG (issue #279)")
heights[[viz_name]] <- as.numeric(svg_attrs[["height"]])
}
expect_lt(heights$default, default_theme_height * n_facets)
expect_lt(heights$custom, custom_theme_height * n_facets,
label = sprintf(
"SVG height should not be %d*num_facets — regression from issue #279",
custom_theme_height))
expect_gt(heights$custom, heights$default)
})
7 changes: 3 additions & 4 deletions tests/testthat/test-renderer3-knit-print.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,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",])
expect_true(all(id.counts==1))
id.vec <- getPropertyValue(html, "//svg", "id")
id.counts <- table(id.vec)
expect_equal(max(id.counts), 1L)
})

all.list <- getNodeSet(html, "//*")
Expand Down
Loading