added fill to gpar in geom_curve()

[hrbrmstr]

Based on https://stackoverflow.com/questions/47878787/fill-arrow-on-geom-curve-ggplot2/47879776#47879776

this adds a missing fill gpar (i.e. back on par with geom_segment) and takes care of ^^ issue.

I also added the fill aesthetic to geom_segment() and changed the fill there to use it.

this means the arrows can be filled in for both.

NOTE: @clauswilke had the idea to add the fill aesthetic to geom_curve and I figured geom_segment shldn't be left out.

[hadley]

I think you can omit this since it will be inherited from geom_segment()

[hadley]

Can you please add a bullet to NEWS? It should briefly describe the change (starting with name of the function), and crediting yourself with (@yourname, #issuenumber).

I think this is a slight change to behaviour? i.e. if you had arrows and mapped the colour to a variable, the arrows will now be black instead of coloured?

[clauswilke]

It is a change in behavior, and on further reflection making the arrow fill an aesthetic may cause problems for some.

First, under the old behavior geom_curve() would always fill the arrow head in white, regardless of any colour or fill settings. That's clearly wrong.

Second, the question is whether the arrow head should take the color of colour or the color of fill. If people currently draw filled arrows specified by colour and use fill for some other geom then they may get unexpected results. It might be better to use a non-standard aesthetic, such as arrow_fill, which by default mirrors colour. This then links back to the needs explained in issue #2345, but the change wouldn't cause any noticeable regressions.

Unless @hrbrmstr beats me to it, I can address this towards the end of the week. Not today, unfortunately.

[hadley]

For now, I'd prefer to make this a parameter of the geom, and then we can figure out how to make it an aesthetic post-2345.

[clauswilke]

So, it'd be a parameter that is set to NA or NULL by default, in which case the geom uses the colour aesthetic to fill the arrow, but if it's set to a color then that's used to override the fill color with that specific value? This sounds reasonable to me.

[hadley]

Exactly - you can just copy what geom_boxplot() does with outlier.colour etc

[hrbrmstr]

Aye. I'll make all those changes later today and also mod the docs + NEWS

[hrbrmstr]

set.seed(123)
data <- data_frame(x = rnorm(10), y = rnorm(10))

gg <- ggplot(data, aes(x, y)) + geom_point()

gg + geom_curve(aes(x = 0, y = 0, xend = 1, yend = 1),
                color = "black",
                arrow = arrow(type = "closed"))

gg + geom_curve(aes(x = 0, y = 0, xend = 1, yend = 1),
                color = "black", arrow.fill = "red",
                arrow = arrow(type = "closed"))

gg + geom_segment(aes(x = 0, y = 0, xend = 1, yend = 1),
                  color = "black",
                  arrow = arrow(type = "closed"))

gg + geom_segment(aes(x = 0, y = 0, xend = 1, yend = 1),
                  color = "black", arrow.fill = "red",
                  arrow = arrow(type = "closed"))

and, for good measure

gg + geom_curve(aes(x = 0, y = 0, xend = 1, yend = 1),
                color = "#00000000", arrow.fill = "black",
                arrow = arrow(type = "closed"))

gg + geom_segment(aes(x = 0, y = 0, xend = 1, yend = 1),
                  color = "#00000000", arrow.fill = "red",
                  arrow = arrow(type = "closed"))

[hrbrmstr]

Roxygen docs were also updated as well as the NEWS.md

[hadley]

Can you please rewrite so the function names come first? (And then use that to alphabetise within this list of changes?)

(Normally I do that just before release but this release has been dragging on because I haven't found the time to finish it off)

[hrbrmstr]

Done.

[hadley]

Thanks!

[lock]

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/