-
Notifications
You must be signed in to change notification settings - Fork 3
Addition of new "Alucard" light mode palette #38
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new "Alucard" theme and associated color palette for use with ggplot2 in R. This includes new exported palette data, discrete and continuous scale functions, a custom theme function, comprehensive documentation, and corresponding unit tests to ensure correct palette behavior and integration with ggplot2 plots. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ggplot2
participant dRacula (package)
participant theme_alucard
participant scale_*_alucard
participant alucard_tibble
User->>ggplot2: Create plot with theme_alucard() and scale_*_alucard()
ggplot2->>theme_alucard: Apply theme customization
ggplot2->>scale_*_alucard: Apply color/fill scale
scale_*_alucard->>alucard_tibble: Retrieve palette colors
ggplot2-->>User: Rendered plot with Alucard theme and colors
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/testthat/test-theme_alucard.R (1)
1-3
: LGTM! Consider adding more comprehensive theme validation tests.The basic smoke test is appropriate and follows testthat conventions correctly. However, consider adding tests that validate the theme actually returns a proper ggplot2 theme object and that key theme elements are set correctly.
Example additional test:
test_that("theme_alucard returns a proper theme object", { theme_obj <- theme_alucard() expect_s3_class(theme_obj, "theme") expect_s3_class(theme_obj, "gg") })R/alucard_palette.R (1)
1-10
: Fix the documentation URL reference.The documentation references
https://spec.draculatheme.com
but this is for the Alucard palette, not Dracula. This appears to be a copy-paste error from the original Dracula implementation.-#' @description A Tibble of Alucard data that includes the palette specification. -#' See https://spec.draculatheme.com for details. +#' @description A Tibble of Alucard data that includes the palette specification. +#' Alucard is a light mode variant of the Dracula theme.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/testthat/_snaps/scales/diamonds-point-plot-alucard.svg
is excluded by!**/*.svg
tests/testthat/_snaps/scales/mtcars-lm-plot-alucard.svg
is excluded by!**/*.svg
tests/testthat/_snaps/scales/rnorm-hex-plot-alucard.svg
is excluded by!**/*.svg
📒 Files selected for processing (10)
NAMESPACE
(1 hunks)R/alucard_palette.R
(1 hunks)R/scales.R
(1 hunks)R/theme_alucard.R
(1 hunks)man/alucard_tibble.Rd
(1 hunks)man/scale_alucard.Rd
(1 hunks)man/theme_alucard.Rd
(1 hunks)tests/testthat/test-alucard_palette.R
(1 hunks)tests/testthat/test-scales.R
(1 hunks)tests/testthat/test-theme_alucard.R
(1 hunks)
🔇 Additional comments (9)
tests/testthat/test-alucard_palette.R (1)
1-8
: Excellent test coverage for the palette function.The tests comprehensively cover the key behaviors:
- Default palette size validation
- Custom size validation
- Error handling for invalid input
The test structure follows testthat best practices and provides good coverage for a discrete palette function.
man/theme_alucard.Rd (1)
12-31
: Excellent documentation examples.The examples demonstrate both common usage patterns effectively:
- Global theme setting with
theme_set()
- Pipeline usage with data manipulation and plotting
The examples are practical and help users understand how to integrate the theme with the Alucard color scales.
NAMESPACE (1)
3-12
: NAMESPACE exports are correctly structured.The new Alucard-related exports are properly added:
alucard_tibble
dataset export- Scale functions with both American (
scale_color_alucard
) and British (scale_colour_alucard
) spelling variantstheme_alucard
function exportThe exports maintain consistency with the existing Dracula theme exports and follow R package conventions.
man/alucard_tibble.Rd (1)
1-21
: Well-structured dataset documentation.The documentation properly follows R conventions for dataset documentation:
- Correct docType and format specifications
- Appropriate description linking to the theme specification
- Proper keyword classification as "datasets"
The reference to the Dracula theme specification website provides helpful context for users wanting to understand the color palette origins.
man/scale_alucard.Rd (1)
1-26
: LGTM! Well-structured documentation.The documentation follows proper R package conventions with correct roxygen2 formatting, appropriate aliases, and comprehensive parameter descriptions.
R/theme_alucard.R (1)
1-49
: Excellent theme implementation with comprehensive documentation.The theme follows best practices by extending
theme_minimal()
and provides a well-designed light color scheme. The examples are particularly helpful, showing both global theme setting and pipeline usage.R/scales.R (1)
38-72
: Excellent implementation mirroring existing patterns.The Alucard scale functions perfectly mirror the existing Dracula implementation, ensuring consistency across the codebase. The conditional logic for discrete vs continuous scales is correct, and the British spelling alias is properly included.
tests/testthat/test-scales.R (1)
50-90
: Comprehensive test coverage with proper visual regression testing.The new test cases effectively mirror the existing Dracula tests and provide thorough coverage of the Alucard scales. The use of
expect_doppelganger
for visual regression testing is the appropriate approach for ggplot2 themes and scales.R/alucard_palette.R (1)
11-65
: Well-implemented palette with proper validation.The color palette definition and helper functions are excellently implemented. The tibble structure with hex, RGB, and HSL values is comprehensive, and the discrete palette function includes proper validation for maximum color count.
theme_alucard() | ||
} | ||
\description{ | ||
Provides a minimal \code{ggplot2} theme with a Alucard, light backdrop. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammatical error in the description.
There's an extra article "a" before "Alucard" in the description.
-Provides a minimal \code{ggplot2} theme with a Alucard, light backdrop.
+Provides a minimal \code{ggplot2} theme with an Alucard light backdrop.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Provides a minimal \code{ggplot2} theme with a Alucard, light backdrop. | |
Provides a minimal \code{ggplot2} theme with an Alucard light backdrop. |
🤖 Prompt for AI Agents
In man/theme_alucard.Rd at line 10, remove the extra article "a" before
"Alucard" in the description to fix the grammatical error. The phrase should
read "with Alucard, light backdrop" instead of "with a Alucard, light backdrop."
Added versions of functions (except the dracula_brand function) with the new alucard light mode colour specs from this open pr. Also included relevant tests following the same format as the original dRacula functions.
Everything follows the dracula theme versions, just with the relevant colours swapped for those in the alucard palette. Only exception is that in
theme_alucard()
thelegend.box.background
variable hasfill
set to"transparent"
rather than"#CFCFDE"
, as I felt that looked better.