Correct fonts in labels #67
Conversation
Use finalize_plot() to update font faces, families, sizes, and colors for text added to the plot through geom_text() or annotate(). Requires hard-coded alternative to default value because font sizes are not playing nice.
|
I'm OK with this change although (I know I sound like a broken record but...) I don't love that it kicks in at finalize and not at theme_cmap. I can't think of a clean or effective way to put it in theme, though. We should specify in documentation, I think, that font can be set by specifying aesthetics in geom_text() if the user wants to. Which brings back up the question of user access to variables in |
Instead of main, for on-chart labels.
Makes the hard-coding more obvious. Also, I added a light gray color.
|
@matthewstern I agree that doing this in finalize is not ideal. This may be an issue of having an incomplete theme, although I'm not sure about that. That's my best guess, though. To your question - after I use |
|
|
|
Oof. Ok, good to know.
|
|
Following up on the discussion here, it looks like the authors of ggplot recommend explicitly building off a built-in ggplot theme if creating a custom theme. This should enable our theme to be 'complete'. Perhaps this in combination with an understanding of the I can try to look into this in the coming days if no one else gets to it first. |
|
Changing to draft because we will need to give this some more thought after #72 is merged. |
Still needs manual adjustment for font size
|
I updated this to accommodate the new |
|
If the labels are supposed to use Whitney Semibold (which you had in your previous iteration), then they should now use the |
|
Oops, good catch, yes @nmpeterson |
from regular to strong
|
I presume you've seen this but it looks like you're not the only one who has decided 14/5 is the right ratio. https://stackoverflow.com/questions/25061822/ggplot-geom-text-font-size-control/25062509 "it's the difference in units, geom_text default of 5 might be 5mm and the theme() size unit is point. 1 point is 1/72 inch=0.35mm, so 1 in geom_text() is 1mm, 1/0.35 =~ 14/5" The unit of size strikes again! |
|
@dlcomeaux @matthewstern I just discovered that |
This suggests that we may want to revisit our size conversion function. It may become irrelevant if we can directly convert sizes to mm and then multiply explicitly by ggplot's built-in constant. The help file isn't clear... I presume .stroke is for lwd and .pt is for font size? Interesting that they are different from one another. |
|
That was the impression I got, too, but yes -- unclear. |
|
OK @matthewstern @nmpeterson I'll give it a try and see if I can make a new version of the function work. |
|
I believe this is ready for review. In addition to incorporating the changes from #81, this PR also:
|
Looks good. Should double check with Comms.
and update documentation accordingly
Replaces two calls in the build_chart function with calls that allow for finalizing non-cmap themed charts. @matthewstern I used `theme_get` instead of `ggplot_globals` to remove the call to an unexported element, since check() was throwing some exceptions to that.
package will error on load if the list produced by `init_cmap_default_aes` does not line up with `cmapplot_globals$geoms_that_change`
…into label_fonts
…into label_fonts
Also explicitly call dplyr when using `first`
to 0.9.2 from 0.9.1
Use finalize_plot() to update font faces, families, sizes, and colors for text added to the plot through geom_text() or annotate(). Requires hard-coded alternative to default value because font sizes are not playing nice.