Skip to content

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Jul 24, 2023

This PR aims to fix #5347.

Briefly, it soft-deprecates the legend.title.align and legend.text.align arguments in favour of the legend.text/title = element_text(hjust) setting.

The align arguments are automatically translated to the hjust setting. This mostly works fine, with the notably exception if an upstream legend.title/text is not a plain element_text. See example below:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

theme(legend.title = ggtext::element_markdown(colour = "red")) +
  theme(legend.title.align = 0.5)
#> Warning: The `legend.title.align` argument of `theme()` is deprecated as of ggplot2
#> 3.5.0.
#> ℹ Please use theme(legend.title = element_text(hjust)) instead.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> Error:
#> ! Problem merging the `legend.title` theme element
#> Caused by error in `merge_element()` at ggplot2/R/theme.R:524:6:
#> ! Only elements of the same class can be merged

Created on 2023-07-24 with reprex v2.0.2

It also supplants legend.title/text = element_blank() with a text element, but these are contradicting instructions anyway. Using it the other way around works just as intended:

devtools::load_all("~/packages/ggplot2/")
#> ℹ Loading ggplot2

theme(legend.title.align = 0.5) +
  theme(legend.title = ggtext::element_markdown(colour = "red"))
#> Warning: The `legend.title.align` argument of `theme()` is deprecated as of ggplot2
#> 3.5.0.
#> ℹ Please use theme(legend.title = element_text(hjust)) instead.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> List of 1
#>  $ legend.title:List of 22
#>   ..$ family        : NULL
#>   ..$ face          : NULL
#>   ..$ size          : NULL
#>   ..$ colour        : chr "red"
#>   ..$ fill          : NULL
#>   ..$ box.colour    : NULL
#>   ..$ linetype      : NULL
#>   ..$ linewidth     : NULL
#>   ..$ hjust         : num 0.5
#>   ..$ vjust         : NULL
#>   ..$ halign        : NULL
#>   ..$ valign        : NULL
#>   ..$ angle         : NULL
#>   ..$ lineheight    : NULL
#>   ..$ margin        : NULL
#>   ..$ padding       : NULL
#>   ..$ r             : NULL
#>   ..$ align_widths  : NULL
#>   ..$ align_heights : NULL
#>   ..$ rotate_margins: NULL
#>   ..$ debug         : logi FALSE
#>   ..$ inherit.blank : logi FALSE
#>   ..- attr(*, "class")= chr [1:3] "element_markdown" "element_text" "element"
#>  - attr(*, "class")= chr [1:2] "theme" "gg"
#>  - attr(*, "complete")= logi FALSE
#>  - attr(*, "validate")= logi TRUE

Created on 2023-07-24 with reprex v2.0.2

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - @hadley do you remember the reason for this redundancy?

@hadley
Copy link
Member

hadley commented Aug 1, 2023

Nope.

@teunbrand
Copy link
Collaborator Author

Thanks for the review!

@teunbrand teunbrand merged commit a747da3 into tidyverse:main Aug 1, 2023
@teunbrand teunbrand deleted the deprecate_align branch August 1, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate legend.title.align and legend.text.align

3 participants