Export combine_elements() - #6885
Open
dmuenz wants to merge 2 commits into
Open
Conversation
* Renamed e1 and e2 parameters to child and parent -- much clearer * Expanded documentation, with description and examples including comparison with merge_element() * Ran devtools::document() to create the .Rd file and update NAMESPACE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR aims to fix #6882.
ggplot2 does not have an exported function that resolves a theme element object against a parent element object, applying the same resolution logic as
calc_element(). In particular, such a function should correctly resolve deferred values likerel()andmargin_part(). However, an unexported function namedcombine_elements()does exactly this -- it is the function thatcalc_element()uses under the hood.So this PR simply (1) exports
combine_elements, (2) improves its documentation since it will now be user-facing, and (3) for the same reason changes the function's parameter names frome1ande2tochildandparent. As a bonus, (4) I added some new unit tests to check that deferred value inheritance works.Note that an exported function named
merge_element()exists and appears very similar tocombine_elements(). Butmerge_element()does not resolve deferred values between a child and parent element object. I address this in the revised documentation forcombine_elements().BTW, I'm open to other name ideas for what I'm calling "deferred values", i.e., the return values of the
rel()andmargin_part()helpers that don't fully specify a value. I use this term in the documentation.