diff --git a/NEWS.md b/NEWS.md index 9519c2120..93b418465 100644 --- a/NEWS.md +++ b/NEWS.md @@ -32,6 +32,10 @@ - Improve common chunk detection, output `na_group` and `row_in_group` when there are missing values. +# Changes in version 2025.10.4 (Issue #234) + +- Fixed `coord_equal()` and `coord_fixed()` to properly fill available plotting space. Previously, plots with fixed aspect ratios were unnecessarily shrunk due to incorrect normalization in the `fixed_spaces()` function. The fix changes from using `min(z, 1)` to normalizing by the maximum value across both dimensions, ensuring at least one dimension fills the available space while maintaining the correct aspect ratio. + # Changes in version 2025.10.3 (PR#240) - `guide_legend(override.aes)` works in a plot with both color and fill legends. diff --git a/R/z_facets.R b/R/z_facets.R index 26c72a404..6eb2f75ac 100644 --- a/R/z_facets.R +++ b/R/z_facets.R @@ -145,5 +145,9 @@ fixed_spaces <- function(ranges, ratio = 1) { function(z) diff(z$y.range) / diff(z$x.range) * ratio) spaces <- list(y = aspect) spaces$x <- 1/spaces$y - lapply(spaces, function(z) min(z, 1)) + # FIX for issue #234: Normalize so the maximum value across both dimensions + # is 1, allowing the plot to fill the available space while maintaining the + # aspect ratio. Previously used min(z, 1) which caused excessive shrinking. + max_val <- max(unlist(spaces)) + lapply(spaces, function(z) z / max_val) } diff --git a/inst/htmljs/animint.js b/inst/htmljs/animint.js index f48129110..809c80c86 100644 --- a/inst/htmljs/animint.js +++ b/inst/htmljs/animint.js @@ -432,12 +432,21 @@ var animint = function (to_select, json_file) { } else { var aspect = 1; } + // FIX for issue #234: Remove Math.min() that was shrinking proportions. + // The R side already normalized properly, so we just apply the aspect ratio. var wp = p_info.layout.width_proportion.map(function(x){ - return x * Math.min(1, aspect); + return x * aspect; }) var hp = p_info.layout.height_proportion.map(function(x){ - return x * Math.min(1, 1/aspect); + return x * (1/aspect); }) + // Normalize so at least one dimension fills the space + var all_props = wp.concat(hp); + var max_prop = Math.max.apply(null, all_props); + if (max_prop > 0) { + wp = wp.map(function(x) { return x / max_prop; }); + hp = hp.map(function(x) { return x / max_prop; }); + } // track the proportion of the graph that should be 'blank' // this is mainly used to implement coord_fixed() diff --git a/tests/testthat/test-compiler-coord-equal-fix.R b/tests/testthat/test-compiler-coord-equal-fix.R new file mode 100644 index 000000000..5e191eb3b --- /dev/null +++ b/tests/testthat/test-compiler-coord-equal-fix.R @@ -0,0 +1,17 @@ +acontext("coord_equal fix for issue #234") + +test_that("fixed_spaces normalizes to fill available space", { + # Test with iris petal data ranges (issue #234 example) + ranges <- list(list(x.range = c(1, 7), y.range = c(0, 2.5))) + spaces <- animint2:::fixed_spaces(ranges, ratio = 1) + + # At least one dimension should be 1 (fills available space) + max_prop <- max(spaces$x, spaces$y) + expect_equal(max_prop, 1) + + # Both proportions should be positive and <= 1 + expect_gt(spaces$x, 0) + expect_lte(spaces$x, 1) + expect_gt(spaces$y, 0) + expect_lte(spaces$y, 1) +}) diff --git a/tests/testthat/test-renderer-coord_equal.R b/tests/testthat/test-renderer-coord_equal.R new file mode 100644 index 000000000..4c76d6c1e --- /dev/null +++ b/tests/testthat/test-renderer-coord_equal.R @@ -0,0 +1,61 @@ +acontext("coord_equal renderer tests") + +test_that("coord_equal fills available space (issue #234)", { + # Create the iris petal plot from issue #234 + # This test ensures coord_equal() doesn't shrink the plot unnecessarily + library(data.table) + data.mat <- as.matrix(iris[,c("Petal.Width","Petal.Length")]) + + # Create the visualization with a specific size + viz <- animint( + plot1=ggplot(data=data.table(data.mat), aes(Petal.Length, Petal.Width))+ + geom_point()+ + coord_equal()+ + theme_animint(width=500, height=500) + ) + + # Render the plot using animint2HTML (not animint2dir) + info <- animint2HTML(viz) + + # Extract SVG plotting area dimensions + # Get the SVG element to check overall plot dimensions + svg.nodes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]') + expect_equal(length(svg.nodes), 1) + svg.attrs <- xmlAttrs(svg.nodes[[1]]) + + # Extract width and height attributes + svg.width <- as.numeric(svg.attrs[["width"]]) + svg.height <- as.numeric(svg.attrs[["height"]]) + + # The key test: with coord_equal(), at least one dimension should + # fill the available space (close to 500px as specified in theme_animint) + # On the old buggy code (before the fix in commit e4b9634b), BOTH dimensions + # would be unnecessarily shrunk below the available space. + # With the fix, at least one dimension uses nearly all available space. + + # The iris petal data has aspect ratio of ~0.417 (y_range=2.5, x_range=6) + # So with coord_equal (ratio=1), the x-axis fills space (width≈500) + # and y-axis is constrained by aspect ratio (height≈200) + + # Test that at least ONE dimension is close to requested size (500) + max_dimension <- max(svg.width, svg.height) + expect_gt(max_dimension, 450) + + # Both dimensions should be positive + expect_gt(svg.width, 0) + expect_gt(svg.height, 0) + + # Verify aspect ratio is maintained (ratio = 1 for coord_equal) + x.axes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]//g[contains(@class, "xaxis")]') + y.axes <- getNodeSet(info$html, '//svg[@id="plot_plot1"]//g[contains(@class, "yaxis")]') + + expect_equal(length(x.axes), 1) + expect_equal(length(y.axes), 1) + + xdiff <- getTickDiff(x.axes[[1]]) + ydiff <- getTickDiff(y.axes[[1]], axis = "y") + + # With ratio = 1 (coord_equal), normalized diffs should be equal + diffs <- normDiffs(xdiff, ydiff, ratio = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) +}) diff --git a/tests/testthat/test-renderer1-coord.R b/tests/testthat/test-renderer1-coord.R index a4e8521f4..07f33a399 100644 --- a/tests/testthat/test-renderer1-coord.R +++ b/tests/testthat/test-renderer1-coord.R @@ -30,7 +30,17 @@ test_that("coord_fixed with shrinking y-axis", { xdiff <- getTickDiff(x.axes[[1]]) ydiff <- getTickDiff(y.axes[[1]], axis = "y") diffs <- normDiffs(xdiff, ydiff, ratio5) - expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1e-3) +}) + +test_that("xaxis width increases with coord_equal", { + p_equal <- p + coord_equal() + animint2HTML(animint(p_equal)) + bbox_default <- get_element_bbox("g.xaxis") + animint2HTML(animint(p_equal+theme_animint(width=1000))) + bbox_big <- get_element_bbox("g.xaxis") + ratio <- bbox_big$width/bbox_default$width + expect_gt(ratio, 2) }) test_that("coord_fixed with shrinking x-axis", { @@ -42,5 +52,5 @@ test_that("coord_fixed with shrinking x-axis", { xdiff <- getTickDiff(x.axes[[1]]) ydiff <- getTickDiff(y.axes[[1]], axis = "y") diffs <- normDiffs(xdiff, ydiff, ratio10) - expect_equal(diffs[1], diffs[2], tolerance = 1, scale = 1) + expect_equal(diffs[1], diffs[2], tolerance = 1e-3) })