WIP: Fix panel.margin with positive values (#180)

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: animint2
 Title: Animated Interactive Grammar of Graphics
-Version: 2026.4.28
+Version: 2026.5.29
 URL: https://animint.github.io/animint2
 BugReports: https://github.com/animint/animint2/issues
 Authors@R: c(

NEWS.md

@@ -1,3 +1,19 @@
+# Changes in version 2026.5.29 (PR#286)
+
+- Added regression tests for `panel.margin` with positive values (issue #180),
+  covering vertical/horizontal `facet_grid` and `facet_wrap`. Thanks @ANAMASGARD.
+
+  **Supported units:** The renderer uses `panel_margin_lines` (via `pt.to.lines()`
+  in `parsePlot()`). Use **`"lines"`** for predictable spacing (`grid::unit(n,
+  "lines")` → `n` line-heights in the browser). **`"pt"`** / **`"points"`** are
+  converted with the ggplot2 default scale (~5.5 pt → 0.25 lines). Other units
+  (`"cm"`, `"mm"`, etc.) pass through as a numeric value interpreted as a line
+  count, not a physical conversion. Pixels are not used directly.
+
+  **Example (horizontal spacing between facet columns):** see
+  `inst/examples/panel-margin-issue-180.R` (`facet_grid(. ~ Species)`, compare
+  `panel.margin = unit(0, "lines")` vs `unit(2, "lines")`).
+
 # Changes in version 2026.4.28 (PR#292)
 
 - `geom(showSelected=character())` means to opt-out of interactive legends. Thanks @ANAMASGARD.

inst/examples/panel-margin-issue-180.R

@@ -0,0 +1,19 @@
+# Issue #180 / PR #286 — horizontal facet spacing (panel.margin)
+# Run from repo root: source("inst/examples/panel-margin-issue-180.R")
+library(animint2)
+
+make_plot <- function(n) {
+  list(
+    plot1 = ggplot(iris, aes(Sepal.Width, Sepal.Length, colour = Species)) +
+      geom_point() +
+      facet_grid(. ~ Species) +
+      theme_bw() +
+      theme(panel.margin = grid::unit(n, "lines"))
+  )
+}
+
+# Screenshot 1: panel.margin = 0 lines (tight columns)
+animint2dir(make_plot(0), "issue180_horizontal_margin0", open.browser = TRUE)
+
+# Screenshot 2: panel.margin = 2 lines (wider gaps between columns)
+animint2dir(make_plot(2), "issue180_horizontal_margin2", open.browser = TRUE)

tests/testthat/test-panel-margin-positive.R

@@ -0,0 +1,84 @@
+acontext("panel.margin with positive values - Issue #180")
+test_that("pt.to.lines handles positive lines unit correctly", {
+  lines_value <- grid::unit(2, "lines")
+  converted <- pt.to.lines(lines_value)
+  expect_true(is.numeric(converted))
+  expect_equal(converted, 2)
+  expect_gt(converted, 0)
+})
+test_that("pt.to.lines handles positive cm unit correctly", {
+  cm_value <- grid::unit(0.5, "cm")
+  converted <- pt.to.lines(cm_value)
+  expect_true(is.numeric(converted))
+  expect_gt(converted, 0)
+})
+test_that("pt.to.lines handles positive pt unit correctly", {
+  pt_value <- grid::unit(12, "pt")
+  converted <- pt.to.lines(pt_value)
+  expect_true(is.numeric(converted))
+  expect_gt(converted, 0)
+  expect_false(identical(converted, as.numeric(pt_value)))
+})
+viz.default <- list(
+  p1 = ggplot() +
+    geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris))
+test_that("plot_theme extracts panel.margin correctly for default theme", {
+  theme.pars <- plot_theme(viz.default$p1)
+  panel_margin <- theme.pars$panel.margin
+  expect_false(is.null(panel_margin))
+  expect_true(grid::is.unit(panel_margin))
+})
+test_that("positive lines preserved through plot_theme and pt.to.lines", {
+  viz <- list(
+    p1 = ggplot() +
+      geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris) +
+      theme(panel.margin = grid::unit(2, "lines")))
+  theme.pars <- plot_theme(viz$p1)
+  panel_margin <- theme.pars$panel.margin
+  expect_false(is.null(panel_margin))
+  converted <- pt.to.lines(panel_margin)
+  expect_true(is.numeric(converted))
+  expect_equal(converted, 2)
+})
+test_that("positive cm preserved through plot_theme and pt.to.lines", {
+  viz <- list(
+    p1 = ggplot() +
+      geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris) +
+      theme(panel.margin = grid::unit(1, "cm")))
+  theme.pars <- plot_theme(viz$p1)
+  panel_margin <- theme.pars$panel.margin
+  expect_false(is.null(panel_margin))
+  converted <- pt.to.lines(panel_margin)
+  expect_true(is.numeric(converted))
+  # pt.to.lines() returns non-pt units as-is (documented limitation):
+  # unit(1, "cm") returns numeric 1, not a proper cm-to-lines conversion.
+  # The JavaScript renderer uses this value as a line count.
+  expect_equal(converted, 1)
+})
+test_that("zero panel.margin should result in zero spacing", {
+  viz <- list(
+    p1 = ggplot() +
+      geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris) +
+      theme(panel.margin = grid::unit(0, "lines")))
+  theme.pars <- plot_theme(viz$p1)
+  panel_margin <- theme.pars$panel.margin
+  converted <- pt.to.lines(panel_margin)
+  expect_equal(converted, 0)
+})
+test_that("positive panel.margin in lines greater than zero", {
+  viz.positive <- list(
+    p1 = ggplot() +
+      geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris) +
+      theme(panel.margin = grid::unit(2, "lines")))
+  viz.zero <- list(
+    p1 = ggplot() +
+      geom_point(aes(Petal.Length, Sepal.Length, color = Species), data = iris) +
+      theme(panel.margin = grid::unit(0, "lines")))
+  theme.positive <- plot_theme(viz.positive$p1)
+  theme.zero <- plot_theme(viz.zero$p1)
+  converted.positive <- pt.to.lines(theme.positive$panel.margin)
+  converted.zero <- pt.to.lines(theme.zero$panel.margin)
+  expect_equal(converted.zero, 0)
+  expect_gt(converted.positive, converted.zero)
+  expect_equal(converted.positive, 2)
+})

tests/testthat/test-renderer1-panel-margin.R

@@ -0,0 +1,180 @@
+acontext("panel.margin positive values - Issue #180 - renderer")
+library(XML)
+panel_x <- function(node_list) {
+  attrs <- sapply(node_list, xmlAttrs)
+  as.numeric(attrs["x", ])
+}
+panel_w <- function(node_list) {
+  attrs <- sapply(node_list, xmlAttrs)
+  as.numeric(attrs["width", ])
+}
+panel_y <- function(node_list) {
+  attrs <- sapply(node_list, xmlAttrs)
+  as.numeric(attrs["y", ])
+}
+panel_h <- function(node_list) {
+  attrs <- sapply(node_list, xmlAttrs)
+  as.numeric(attrs["height", ])
+}
+gap_between_panels_y <- function(node_list) {
+  y <- sort(panel_y(node_list))
+  h <- panel_h(node_list)[order(panel_y(node_list))]
+  y[2] - (y[1] + h[1])
+}
+gap_between_panels_x <- function(node_list) {
+  ord <- order(panel_x(node_list))
+  x <- panel_x(node_list)[ord]
+  w <- panel_w(node_list)[ord]
+  x[2] - (x[1] + w[1])
+}
+vertical_gap_between_wrap_rows <- function(node_list) {
+  ys <- panel_y(node_list)
+  hs <- panel_h(node_list)
+  uy <- sort(unique(ys))
+  if (length(uy) < 2) {
+    return(NA_real_)
+  }
+  tol <- 1e-4
+  i1 <- abs(ys - uy[1]) < tol
+  bottom1 <- max(ys[i1] + hs[i1])
+  i2 <- abs(ys - uy[2]) < tol
+  top2 <- min(ys[i2])
+  top2 - bottom1
+}
+horizontal_gap_between_wrap_cols <- function(node_list) {
+  xs <- panel_x(node_list)
+  ws <- panel_w(node_list)
+  ux <- sort(unique(xs))
+  if (length(ux) < 2) {
+    return(NA_real_)
+  }
+  tol <- 1e-4
+  i1 <- abs(xs - ux[1]) < tol
+  right1 <- max(xs[i1] + ws[i1])
+  i2 <- abs(xs - ux[2]) < tol
+  left2 <- min(xs[i2])
+  left2 - right1
+}
+
+viz.zero <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(0, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_grid(Species ~ .)
+)
+viz.positive <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(2, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_grid(Species ~ .)
+)
+info.zero <- animint2HTML(viz.zero)
+info.positive <- animint2HTML(viz.positive)
+bg.zero <- getNodeSet(
+  info.zero$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+bg.positive <- getNodeSet(
+  info.positive$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+test_that("three panels rendered with zero panel.margin", {
+  expect_equal(length(bg.zero), 3)
+})
+test_that("three panels rendered with positive panel.margin", {
+  expect_equal(length(bg.positive), 3)
+})
+test_that("panel y-positions are strictly increasing (vertically stacked)", {
+  y.vals <- sort(panel_y(bg.positive))
+  expect_true(all(diff(y.vals) > 0))
+})
+test_that("positive panel.margin produces larger inter-panel gap than zero margin", {
+  gap.zero <- gap_between_panels_y(bg.zero)
+  gap.positive <- gap_between_panels_y(bg.positive)
+  expect_gt(gap.positive, 0)
+  expect_gt(gap.positive, gap.zero)
+})
+
+viz.zero.h <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(0, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_grid(. ~ Species)
+)
+viz.positive.h <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(2, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_grid(. ~ Species)
+)
+info.zero.h <- animint2HTML(viz.zero.h)
+info.positive.h <- animint2HTML(viz.positive.h)
+bg.zero.h <- getNodeSet(
+  info.zero.h$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+bg.positive.h <- getNodeSet(
+  info.positive.h$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+test_that("facet_grid columns: three panels with zero panel.margin", {
+  expect_equal(length(bg.zero.h), 3)
+})
+test_that("facet_grid columns: three panels with positive panel.margin", {
+  expect_equal(length(bg.positive.h), 3)
+})
+test_that("facet_grid columns: x-positions strictly increasing", {
+  x.vals <- sort(panel_x(bg.positive.h))
+  expect_true(all(diff(x.vals) > 0))
+})
+test_that("facet_grid columns: positive margin widens horizontal gap", {
+  gap.zero <- gap_between_panels_x(bg.zero.h)
+  gap.positive <- gap_between_panels_x(bg.positive.h)
+  expect_gt(gap.positive, 0)
+  expect_gt(gap.positive, gap.zero)
+})
+
+viz.zero.w <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(0, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_wrap(~Species, ncol = 2)
+)
+viz.positive.w <- list(
+  plot1 = ggplot() +
+    theme_bw() +
+    theme(panel.margin = grid::unit(2, "lines")) +
+    geom_point(aes(Sepal.Width, Sepal.Length, colour = Species), data = iris) +
+    facet_wrap(~Species, ncol = 2)
+)
+info.zero.w <- animint2HTML(viz.zero.w)
+info.positive.w <- animint2HTML(viz.positive.w)
+bg.zero.w <- getNodeSet(
+  info.zero.w$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+bg.positive.w <- getNodeSet(
+  info.positive.w$html,
+  '//svg[@id="plot_plot1"]//rect[@class="background_rect"]')
+test_that("facet_wrap: three panels with zero panel.margin", {
+  expect_equal(length(bg.zero.w), 3)
+})
+test_that("facet_wrap: three panels with positive panel.margin", {
+  expect_equal(length(bg.positive.w), 3)
+})
+test_that("facet_wrap: positive margin widens horizontal gap between columns", {
+  g0 <- horizontal_gap_between_wrap_cols(bg.zero.w)
+  g1 <- horizontal_gap_between_wrap_cols(bg.positive.w)
+  expect_false(is.na(g0))
+  expect_false(is.na(g1))
+  expect_gt(g1, 0)
+  expect_gt(g1, g0)
+})
+test_that("facet_wrap: positive margin widens vertical gap between rows", {
+  g0 <- vertical_gap_between_wrap_rows(bg.zero.w)
+  g1 <- vertical_gap_between_wrap_rows(bg.positive.w)
+  expect_false(is.na(g0))
+  expect_false(is.na(g1))
+  expect_gt(g1, 0)
+  expect_gt(g1, g0)
+})