wilkelab / gridtext

Improved text rendering support for grid graphics in R
https://wilkelab.org/gridtext
Other
96 stars 17 forks source link

Solve #24 – use `font` gpar instead of `fontface` to work with R 4.2.0 #25

Closed netique closed 2 years ago

netique commented 2 years ago

This PR solves https://github.com/wilkelab/gridtext/issues/24 for me. I've checked carefully every fontface appearance in the package and fixed the critical places. Note that if you use fontface in gpar(), it is automatically "translated" to font (and fontface is discarded) with appropriate integer code, so in those occasions, it could be left intact (I believe this font-centered {grid} behavior will last many years to come).

One special place is in set_context_fontface() – this is called only by process_tags(), which utilizes character-based fontface argument, but it is used as positional argument in every occasion, so it works. The function retrieved face from drawing_context$gp$fontface, which was in this PR changed to drawing_context$gp$font and subsequent usage of this retrieved face was edited to work with integer face par.

I tested the compatibility with {ggtext}, which also uses fontface at its last stage before handed over to {gridtext}. This is again accomplished by passing fontface inside gpar(), so it is "translated "correctly even in R 4.2.0.

clauswilke commented 2 years ago

Aside from the comments I made, looks good to me. Maybe also add a bullet point in NEWS.md.

netique commented 2 years ago

Thanks for the review. I accepted all proposed changes and made a bullet in NEWS.md. Generally speaking, all user-facing functions and documentation should have fontface arguments.

clauswilke commented 2 years ago

Super, thanks for your help. I have been dragging my feet on making a new release, but this can't wait. I'll try to get this submitted to CRAN ASAP.