Skip to content

Conversation

@teunbrand
Copy link
Collaborator

This PR to the RC aims to make a simplification to guide_legend().

Briefly, this PR adds a legend_add_title() helper that is used in guide_legend() and guide_custom(). This has two advantages:

  • We don't have to maintain two separate instances of title logic.
  • We can simplify the layout code in guide_legend(), as we no longer have to do bookkeeping and adjustments to the key/label positions based on the title.

I figured if there was ever a time to simplify the legend layout logic, without having to deal with backward compatibility issues, now is that time.

@teunbrand teunbrand changed the base branch from main to rc/3.5.0 January 16, 2024 09:17
@teunbrand teunbrand marked this pull request as draft January 16, 2024 10:20
Merge branch 'rc/3.5.0' into isolate_legend_title_logic

# Conflicts:
#	R/guide-custom.R
@teunbrand teunbrand changed the base branch from rc/3.5.0 to main January 19, 2024 08:06
@teunbrand teunbrand changed the base branch from main to rc/3.5.0 January 19, 2024 08:07
@teunbrand teunbrand marked this pull request as ready for review January 19, 2024 08:07
@teunbrand teunbrand added this to the ggplot2 3.5.0 milestone Jan 19, 2024
@teunbrand teunbrand requested a review from thomasp85 January 22, 2024 12:52
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

@teunbrand teunbrand merged commit 5c550e7 into tidyverse:rc/3.5.0 Jan 25, 2024
@teunbrand teunbrand deleted the isolate_legend_title_logic branch January 25, 2024 08:07
thomasp85 pushed a commit that referenced this pull request Feb 23, 2024
* extract title logic in `legend_add_title()`

* `guide_custom()` uses title logic

* simplify legend layout

* borrow legend's setup_elements method

* make `add_title` a ggproto method

* fix messy merge conflict

* document `add_title` method

* `unique()` --> `unique0()`
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.

2 participants