Skip to content

Enable more styling options for guide_colorbar(). #2541

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

Merged
merged 4 commits into from
May 2, 2018
Merged

Enable more styling options for guide_colorbar(). #2541

merged 4 commits into from
May 2, 2018

Conversation

clauswilke
Copy link
Member

Inspired by StackOverflow questions here and here: This patch enables better styling of the colorbar guide.

Examples:

set.seed(123)
df <- data.frame(x <- rnorm(10),
                 y <- rnorm(10),
                 fill <- rnorm(10))

p <- ggplot(df, aes(x, y, fill = fill)) + geom_point()

# current ggplot2
p + scale_fill_gradient(low = 'white', high = 'red')

screen shot 2018-04-28 at 10 34 44 am

# make frame around color bar
p + scale_fill_gradient(low = 'white', high = 'red',
                        guide = guide_colorbar(frame.colour = "black"))

screen shot 2018-04-28 at 10 34 54 am

# style tick marks
p + scale_fill_gradient(low = 'white', high = 'red',
                        guide = guide_colorbar(ticks.colour = "black"))

screen shot 2018-04-28 at 10 35 01 am

# style frame and tick marks at the same time, change linewidth
p + scale_fill_gradient(low = 'white', high = 'red',
                        guide = guide_colorbar(frame.colour = "black",
                                               frame.linewidth = 1,
                                               ticks.colour = "black",
                                               ticks.linewidth = 1))

screen shot 2018-04-28 at 10 35 08 am

This pull request will require some modifications once #2415 is integrated (or vice versa). I'm happy to take care of that.

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

Some minor style stuff, and a question about the argument names, but otherwise good to go.

NEWS.md Outdated
@@ -285,6 +285,11 @@ up correct aspect ratio, and draws a graticule.

* Default themes use `rel()` to set line widths (@baptiste).

### Guides

* Make `guide_colorbar()` more configurable; enable styling of tick marks and
Copy link
Member

Choose a reason for hiding this comment

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

Can you mention the argument names here please?

# frame
frame.colour = NULL,
frame.linewidth = 0.5,
frame.linetype = 1,
Copy link
Member

Choose a reason for hiding this comment

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

Have we used linewidth and linetype in other functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

linetype is a standard aesthetic, so I think that's fine. linewidth is usually called size throughout the rest of ggplot2. If you prefer size I'm happy to make the change, it's just that frame.size doesn't sound like it controls the thickness of the frame.

Copy link
Member

Choose a reason for hiding this comment

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

I don't have any particularly strong feelings here; I don't really understand how these options are supposed to fit in with the theme system.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. The question is whether every possible option to a guide function needs a theme equivalent. My personal opinion is no. I think there is a lot of room for improvement for making guide functions more flexible, and possibly providing different ones or frameworks for extending them, and it's impossible to add new theme components every time somebody makes some minor tweak to some guide function. Already, there are things the guide functions can do that the theme can't control, such as the draw.ulim option for guide_colorbar().

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. I'm happy to merge this PR; I just wanted to make sure we at least talked over the issues.


# make frame around color bar if requested (colour is not NULL)
if (!is.null(guide$frame.colour)) {
grob.bar <- grobTree(grob.bar,
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adhering to modern tidyverse style here?

grobTree(
  grob.bar,
  rectGrob(
    width = barwidth.c, height = barheight.c, default.units = "mm",
    gp = gpar(
      col = guide$frame.colour,
      lwd = guide$frame.linewidth,
      lty = guide$frame.linetype,
      fill = NA
    )
  )
)

or similar

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, will do.

@@ -352,7 +384,9 @@ guide_gengrob.colorbar <- function(guide, theme) {
y1 = rep(tic_pos.c, 2)
})
segmentsGrob(x0 = x0, y0 = y0, x1 = x1, y1 = y1,
default.units = "mm", gp = gpar(col = "white", lwd = 0.5, lineend = "butt"))
default.units = "mm", gp = gpar(col = guide$ticks.colour,
Copy link
Member

Choose a reason for hiding this comment

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

And here

@clauswilke
Copy link
Member Author

Just a note: I'll wait with finalizing this PR until after #2415 is integrated, since they touch the same files.

@clauswilke
Copy link
Member Author

I believe this latest update addresses all concerns. I added two minimal visual tests, one for the default styling (it also tests that colors can be changed via scale_color_..., so is useful anyways I think) and one for the modified styling of both the frame and the tick lines.

@hadley hadley merged commit 21b5579 into tidyverse:master May 2, 2018
@hadley
Copy link
Member

hadley commented May 2, 2018

Thank you!

@lock
Copy link

lock bot commented Oct 29, 2018

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 Oct 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants