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

Fill closed arrows in element_line() #2924

Merged

Conversation

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Oct 1, 2018

Fixes #2922

Currently, element_grob.element_line() doesn't specify fill in gpar(). This PR specifies it to fill arrows.

library(ggplot2)

arrow = arrow(angle=10, type = "closed")
ggplot(iris, aes(x=Sepal.Length, y=Sepal.Width, color=Species)) +
  geom_point() +
  theme(axis.line = element_line(arrow=arrow)) +
  annotate("segment", x = 3, xend = min(iris$Sepal.Length), y = 4, yend = 3, arrow=arrow)

Created on 2018-10-02 by the reprex package (v0.2.1)

Note that, for consistency with geom_segment() and geom_curve(), element_line() might needs arrow.fill parameter, but it seems a bit harder to introduce a new parameter to theme. So, users need to rely on this workaround if they want different colours.

@@ -226,9 +226,11 @@ element_grob.element_line <- function(element, x = 0:1, y = 0:1,
default.units = "npc", id.lengths = NULL, ...) {

# The gp settings can override element_gp
gp <- gpar(lwd = len0_null(size * .pt), col = colour, lty = linetype, lineend = lineend)
gp <- gpar(lwd = len0_null(size * .pt), col = colour, lty = linetype,
lineend = lineend, fill = colour)
Copy link
Member

@clauswilke clauswilke Oct 1, 2018

Could you use tidyverse-style formatting? The same applies for the other change.

gp <- gpar(
  lwd = len0_null(size * .pt), col = colour, lty = linetype,
  lineend = lineend, fill = colour
)

Copy link
Member

@clauswilke clauswilke Oct 1, 2018

And maybe reorder the parameters so col and fill go together and then lwd, lty, and lineend.

Copy link
Member Author

@yutannihilation yutannihilation Oct 1, 2018

Sure.

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Oct 1, 2018

Thanks! This will have to wait until after the 3.1.0 release.

@yutannihilation
Copy link
Member Author

@yutannihilation yutannihilation commented Oct 1, 2018

Thanks for reviewing! I'll wait (and will add a NEWS bullet after the release).

@yutannihilation
Copy link
Member Author

@yutannihilation yutannihilation commented Nov 13, 2018

@clauswilke Could you review this again when you have time?

@yutannihilation yutannihilation merged commit a330da3 into tidyverse:master Nov 14, 2018
4 checks passed
@yutannihilation yutannihilation deleted the fill-element-line-arrow branch Nov 14, 2018
@yutannihilation
Copy link
Member Author

@yutannihilation yutannihilation commented Nov 14, 2018

Thanks!

@lock
Copy link

@lock lock bot commented May 13, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators May 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants