Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for user to specify outline color #67

Merged
merged 10 commits into from
Aug 25, 2023
Merged

Allow for user to specify outline color #67

merged 10 commits into from
Aug 25, 2023

Conversation

willgearty
Copy link
Collaborator

This adds a fill argument/aesthetic that allows users to specify different outline and body colors for silhouettes (in geom_phylopic, add_phylopic, and add_phylopic_base):
image

Note that I was able to make it such that if only the color aesthetic/argument was specified (the old user behavior) in geom_phylopic or add_phylopic that that color is used for both the color and fill now.

Fixes #58.

@willgearty willgearty added this to the 1.2.0 milestone Jul 19, 2023
Copy link
Collaborator

@LewisAJones LewisAJones left a comment

Choose a reason for hiding this comment

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

Hey @willgearty, this is looking great! Nothing really to comment on the code... behaves as it should. I've suggested a few changes to documentation. My main comment is that I think we should try to ensure the same behaviour between add_phylopic, add_phylopic_base, and geom_phylopic for the color and fill arguments.

R/add_phylopic.r Outdated Show resolved Hide resolved
R/add_phylopic.r Outdated Show resolved Hide resolved
R/add_phylopic.r Outdated Show resolved Hide resolved
R/add_phylopic.r Outdated Show resolved Hide resolved
R/add_phylopic.r Show resolved Hide resolved
R/add_phylopic_base.r Show resolved Hide resolved
R/add_phylopic_base.r Outdated Show resolved Hide resolved
R/geom_phylopic.R Outdated Show resolved Hide resolved
R/geom_phylopic.R Show resolved Hide resolved
R/phylopic_utils.R Outdated Show resolved Hide resolved
@willgearty
Copy link
Collaborator Author

willgearty commented Aug 24, 2023

Thanks for the review @LewisAJones! I believe that should address all of your comments. I've changed the default behavior, and this behavior is now the same across all three functions.

Copy link
Collaborator

@LewisAJones LewisAJones left a comment

Choose a reason for hiding this comment

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

Thanks @willgearty! Changes look good to me now, I have nothing worthwhile adding. Good job!

@willgearty willgearty merged commit c19c760 into main Aug 25, 2023
7 checks passed
@willgearty willgearty deleted the outline-color branch August 25, 2023 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

color outlines with recolor_phylopic
2 participants