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

Subtheme functions #5430

Merged
merged 12 commits into from
Jan 27, 2025
Merged

Subtheme functions #5430

merged 12 commits into from
Jan 27, 2025

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Sep 20, 2023

This PR aims to fix #5301.

Briefly, it adds functions that allow specifying parts of a theme with shorter arguments.

A few considerations:

  • I tried making a function factory for these subtheme functions, to automatically populate the formals as the argument names are very regex friendly. However, roxygen2 wouldn't recognise the formals in the @usage section.
  • Should these function be named subtheme_*() as to avoid confusion with e.g. theme_light() which is conceptually doing a different thing?

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

I do think we need a different prefix than theme_, but conceptually I think it should be shorter since the whole point of these functions are to be shortcuts... maybe just th_? Or maybe that is too unpronouncable...

@teunbrand
Copy link
Collaborator Author

I like this as an idea, but I'm not sure we can gracefully compact the word 'theme' any futher. Using th_ as a prefix, reminds me too much of the html <th> tag for table headers.

I don't think of these functions as strictly shortcuts (as in abbreviations): they mostly avoid needless repetition in argument names.
Writing theme_axis_left(line = element_line(...)) is not a lot shorter than theme(axis.line.y.left = element_line(...)), but does become shorter when you have multiple axis.{x}.y.left declarations. For this reason, I still think using a subtheme_ prefix is not unreasonable.

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

I think I would probably prefer theme_sub_*() then (I know that is longer even). I like that all theme functions have the same (partial) prefix

p + 
  theme_minimal() + 
  theme_sub_axis(...)

looks more organised than

p + 
  theme_minimal() + 
  subtheme_axis(...)

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

LGTM

@teunbrand teunbrand merged commit 1803552 into tidyverse:main Jan 27, 2025
13 checks passed
@teunbrand teunbrand deleted the subtheme branch January 27, 2025 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Syntactic sugar for theme
2 participants