Add descenders to heights (tidyverse#1728)

* Add descenders to heights

And increase margins slightly. Fixes tidyverse#1712

* Add issue number

* Update theme test

* Reduce outer margins

Author: hadley

NEWS.md

@@ -1,6 +1,12 @@
-
 # ggplot2 2.1.0.9000 
 
+* When computing the height of titles ggplot2, now inclues the height of the
+  descenders (i.e. the bits `g` and `y` that hang underneath). This makes 
+  improves the margins around titles, particularly the y axis label (#1712).
+  
+    I have also very slightly increased the inner margins of axis titles,
+    and removed the outer margins. 
+
 * Themes are more homogeneous visually, and match `theme_grey` better.
   (@jiho, #1679)
 

R/margins.R

@@ -62,20 +62,26 @@ titleGrob <- function(label, x, y, hjust, vjust, angle = 0, gp = gpar(),
   text_grob <- textGrob(label, x, y, hjust = hjust, vjust = vjust,
     rot = angle, gp = gp)
 
+  # The grob dimensions don't include the text descenders, so add on using
+  # a little trigonometry. This is only exactly correct when vjust = 1.
+  descent <- descentDetails(text_grob)
+  text_height <- unit(1, "grobheight", text_grob) + cos(angle / 180 * pi) * descent
+  text_width <- unit(1, "grobwidth", text_grob) + sin(angle / 180 * pi) * descent
+
   if (expand_x && expand_y) {
-    widths <- unit.c(margin[4], unit(1, "grobwidth", text_grob), margin[2])
-    heights <- unit.c(margin[1], unit(1, "grobheight", text_grob), margin[3])
+    widths <- unit.c(margin[4], text_width, margin[2])
+    heights <- unit.c(margin[1], text_height, margin[3])
 
     vp <- viewport(layout = grid.layout(3, 3, heights = heights, widths = widths), gp = gp)
     child_vp <- viewport(layout.pos.row = 2, layout.pos.col = 2)
   } else if (expand_x) {
-    widths <- unit.c(margin[4], unit(1, "grobwidth", text_grob), margin[2])
+    widths <- unit.c(margin[4], text_width, margin[2])
     vp <- viewport(layout = grid.layout(1, 3, widths = widths), gp = gp)
     child_vp <- viewport(layout.pos.col = 2)
 
     heights <- unit(1, "null")
   } else if (expand_y) {
-    heights <- unit.c(margin[1], unit(1, "grobheight", text_grob), margin[3])
+    heights <- unit.c(margin[1], text_height, margin[3])
 
     vp <- viewport(layout = grid.layout(3, 1, heights = heights), gp = gp)
     child_vp <- viewport(layout.pos.row = 2)

R/theme-defaults.r

@@ -85,11 +85,13 @@ theme_grey <- function(base_size = 11, base_family = "") {
     axis.ticks =         element_line(colour = "grey20"),
     axis.ticks.length =  unit(half_line / 2, "pt"),
     axis.title.x =       element_text(
-                           margin = margin(t = 0.8 * half_line, b = 0.8 * half_line / 2)
+                           margin = margin(t = half_line),
+                           vjust = 1
                          ),
     axis.title.y =       element_text(
                            angle = 90,
-                           margin = margin(r = 0.8 * half_line, l = 0.8 * half_line / 2)
+                           margin = margin(r = half_line),
+                           vjust = 1,
                          ),
 
     legend.background =  element_rect(colour = NA),
@@ -126,17 +128,17 @@ theme_grey <- function(base_size = 11, base_family = "") {
     plot.background =    element_rect(colour = "white"),
     plot.title =         element_text(
                            size = rel(1.2),
-                           hjust = 0,
+                           hjust = 0, vjust = 1,
                            margin = margin(b = half_line * 1.2)
                          ),
     plot.subtitle =      element_text(
                            size = rel(0.9),
-                           hjust = 0,
+                           hjust = 0, vjust = 1,
                            margin = margin(b = half_line * 0.9)
                          ),
     plot.caption =       element_text(
                            size = rel(0.9),
-                           hjust = 1,
+                           hjust = 1, vjust = 1,
                            margin = margin(t = half_line * 0.9)
                          ),
     plot.margin =        margin(half_line, half_line, half_line, half_line),
@@ -179,12 +181,12 @@ theme_linedraw <- function(base_size = 11, base_family = "") {
       axis.ticks       = element_line(colour = "black", size = 0.25),
       # NB: match the *visual* thickness of axis ticks to the panel border
       #     0.5 clipped looks like 0.25
-      
+
       # pure black panel border and grid lines, but thinner
       panel.border     = element_rect(fill = NA, colour = "black", size = 0.5),
       panel.grid.major = element_line(colour = "black", size = 0.05),
       panel.grid.minor = element_line(colour = "black", size = 0.025),
-      
+
       # strips with black background and white text
       strip.background = element_rect(fill = "black"),
       strip.text       = element_text(colour = "white", size = rel(0.8))
@@ -210,7 +212,7 @@ theme_light <- function(base_size = 11, base_family = "") {
 
       # match legend key to panel.background
       legend.key       = element_rect(fill = "white", colour = NA),
-      
+
       # dark strips with light text (inverse contrast compared to theme_grey)
       strip.background = element_rect(fill = "grey70", colour = NA),
       strip.text       = element_text(colour = "white", size = rel(0.8))
@@ -233,7 +235,7 @@ theme_dark <- function(base_size = 11, base_family = "") {
 
       # match axes ticks thickness to gridlines
       axis.ticks       = element_line(colour = "grey20", size = 0.25),
-      
+
       # match legend key to panel.background
       legend.key       = element_rect(fill = "grey50", colour = NA),
 

tests/testthat/test-theme.r

@@ -4,7 +4,7 @@ test_that("Modifying theme element properties with + operator", {
 
   # Changing a "leaf node" works
   t <- theme_grey() + theme(axis.title.x = element_text(colour = 'red', margin = margin()))
-  expect_identical(t$axis.title.x, element_text(colour = 'red', margin = margin()))
+  expect_identical(t$axis.title.x, element_text(colour = 'red', margin = margin(), vjust = 1))
   # Make sure the theme class didn't change or get dropped
   expect_true(is.theme(t))
   # Make sure the element class didn't change or get dropped