-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add descenders to heights #1728
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
Conversation
And increase margins slightly. Fixes #1712
# Conflicts: # NEWS.md
The top of the |
margin = margin(r = half_line, l = half_line / 2), | ||
vjust = 1 | ||
margin = margin(r = half_line), | ||
vjust = 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing the added comma is problematic here. It should create an error.
Indeed, the top of capitals or ascenders and the bottom of descenders are slightly cut off (although that depends on the rendering software; when I import the PDF into Inkscape and re-export it, the clipping of letters is not as pronounced). And indeed, I think the solution is to set So basically, the changes look perfect to me now. |
There was a way at some point to change the position of the axes (put the x axis on top, the y axis on the right, etc.). See https://groups.google.com/forum/?fromgroups=#!topic/ggplot2-dev/JkJU5CLBkQw Is that not relevant anymore? If such tweaks exist then, ideally, there should be a way in the themes to specify "outer margin" instead of left/right/top/bottom margin. The easy alternative of course would be to make margins symmetrical, but I would be less pleasing visually as explained above. |
This is not currently possible, but depending on the complexity it might be included in the facet rewrite (with @hadley's blessing of course) with regards to margins, see my comment on #1727 - optimally this proposal should be implemented for all margin settings and would allow for symmetric margin setting |
I think the theming system is already sufficiently complicated without having to additionally distinguish between inner/outer. The unfortunate reality is that if you have a theme where the labels are in a different place you will have to tweak margin settings in numerous locations. I don't think there is a good solution for this apart from implementing a full constraint-based layout system. |
* Add descenders to heights And increase margins slightly. Fixes tidyverse#1712 * Add issue number * Update theme test * Reduce outer margins
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/ |
And increase margins slightly. Fixes #1712
@baptiste, @thomasp85, @jiho – can you please take a look? I always forget the consequences of tweaking theme defaults.
Before:

After:

The difference is subtle, but an improvement.