diff --git a/NEWS.md b/NEWS.md index 35c848a08..9cb2b5572 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,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 02c8d760a..ea6445c88 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..4aac94a65 --- /dev/null +++ b/tests/testthat/test-renderer-coord_equal.R @@ -0,0 +1,62 @@ +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, + info="At least one dimension should fill available space") + + # 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) +})