Skip to content

Unexpected behavior in element_text() horizontal justification #2653

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
zlskidmore opened this issue May 23, 2018 · 7 comments
Closed

Unexpected behavior in element_text() horizontal justification #2653

zlskidmore opened this issue May 23, 2018 · 7 comments
Labels
bug an unexpected problem or unintended behavior
Milestone

Comments

@zlskidmore
Copy link

In the upcoming release of ggplot2 (2.3.0) the behavior of element_text() has changed. Specifically it looks like the justification anchors have changed in relation to the text. For Example:

In the current release:
ggplot_2 2 1

In the development version:
ggplot2_2 3 0

mtcars$name <- rownames(mtcars)

ggplot(mtcars, aes(x=name, y=carb)) + geom_bar(stat="identity") + theme(axis.text.x=element_text(angle=60, hjust=1))
@karawoo karawoo added the bug an unexpected problem or unintended behavior label May 23, 2018
@karawoo
Copy link
Member

karawoo commented May 23, 2018

Thanks for reporting this. I believe this bug is due to the change in how xp and yp are calculated in #2577.

@karawoo karawoo added this to the v2.3.0 milestone May 23, 2018
@karawoo
Copy link
Member

karawoo commented May 23, 2018

@hadley @clauswilke I think we should revert this change, it's going to break a lot of users' plots.

@clauswilke
Copy link
Member

@karawoo I'll look into it. However, I'm not yet convinced about the cause of this problem, because I checked for this particular issue when I made the patch, if I remember correctly. I think this has actually to do with changed grob heights due to fixed descenders.

A workaround exists, but I agree it's not intuitive (why vjust = 0.65?)

library(ggplot2)
mtcars$name <- rownames(mtcars)

ggplot(mtcars, aes(x=name, y=carb)) + 
  geom_bar(stat="identity") + theme(axis.text.x=element_text(angle=60, hjust=1, vjust = 0.65))

Created on 2018-05-23 by the reprex package (v0.2.0).

@clauswilke
Copy link
Member

When looking at this in debug mode, we see that for axis tick labels along the x axis the reference point travels vertically for different vjust values rather than orthogonally to the direction of the text. As far as I recall, this behavior is identical to the currently released ggplot2 version, but it's different from the behavior for hjust (which is along the text direction) and it's also different from hjust and vjust for axis titles, legend text and titles, and so on.

library(ggplot2)
mtcars$name <- rownames(mtcars)

ggplot(mtcars, aes(x=name, y=carb)) + 
  geom_bar(stat="identity") + 
  theme(axis.text.x=element_text(angle=60, hjust=1, vjust = 0, debug = TRUE))

ggplot(mtcars, aes(x=name, y=carb)) + 
  geom_bar(stat="identity") + 
  theme(axis.text.x=element_text(angle=60, hjust=1, vjust = 0.5, debug = TRUE))

ggplot(mtcars, aes(x=name, y=carb)) + 
  geom_bar(stat="identity") + 
  theme(axis.text.x=element_text(angle=60, hjust=1, vjust = 1, debug = TRUE))

There are two possible alternative ways to fix this:

  • Make the reference point travel inside the range indicated by the colored square, so that vjust = 0 indicates the bottom of the square and vjust = 1 indicates the top.

  • Make the reference point travel orthogonally to the text direction at all times.

@clauswilke
Copy link
Member

I see what the problem is now. It arises in title_spec() when one position coordinate is set and one is not. I can solve things for this particular case by replacing these lines:

ggplot2/R/margins.R

Lines 47 to 48 in eecc450

x <- x %||% unit(rep(just$hjust, n), "npc")
y <- y %||% unit(rep(just$vjust, n), "npc")

with

  if (is.null(x)) {
    if (is.null(y)) {
      x <- unit(rep(just$hjust, n), "npc")
      y <- unit(rep(just$vjust, n), "npc")
    } else {
      x <- unit(rep(hjust, n), "npc")
    }
  } else if (is.null(y)) {
    y <- unit(rep(vjust, n), "npc")
  }

but that's not a general solution (it fails at other angles). Let me ponder this some more. I'd rather fix this in a forward-looking manner than going back to behavior that is broken in other ways.

@clauswilke
Copy link
Member

clauswilke commented May 23, 2018

I've been pondering this issue the whole afternoon and I'm not sure how to fix this. (We can revert, of course.) The bigger problem I'm seeing is that we are using two different justification systems throughout ggplot, and things break one way or another when we mix them (as is the case for axis tick labels): One system uses a fixed reference point and moves the text relative to that point. E.g., hjust = 0 means the entire string is to the left of that point, and hjust = 1 means the entire string is to the right. The other system uses a fixed extent and moves the string inside that extent. E.g., hjust = 0 means the string is at the left boundary of that extent, and hjust = 1 means it's on the right boundary. For x axis tick labels, we want the first justification system along the x axis and the second orthogonal to the x axis. But we also want to use hjust and vjust in the internal coordinate system of the text label, so that hjust = 1 right-justifies all the text labels regardless of angle. I haven't yet found a way bring all these conflicting constraints into one consistent framework that makes sense at all angles.

Since I don't even know what the correct behavior is in the general case, I just created a PR that reverts to the previous model specifically when the justification model is mixed: #2654

With this PR:

library(ggplot2)
mtcars$name <- rownames(mtcars)

ggplot(mtcars, aes(x=name, y=carb)) + 
  geom_bar(stat="identity") + theme(axis.text.x=element_text(angle=60, hjust=1))

Created on 2018-05-23 by the reprex package (v0.2.0).

clauswilke added a commit to wilkelab/ggplot2_archive that referenced this issue May 24, 2018
@lock
Copy link

lock bot commented Nov 21, 2018

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants