-
Notifications
You must be signed in to change notification settings - Fork 843
Charts: Support HSL, RGB, and RGBA theme colors (CHARTS-138) #46349
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: trunk
Are you sure you want to change the base?
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
- Updated global-charts-provider to use normalizeColorToHex for all color processing - Added support for RGB, RGBA, and HSL color formats in theme colors - Updated normalizeColorToHex to handle rgba() colors (previously rejected) - Added comprehensive tests for RGB, HSL, and RGBA color support - Updated existing tests to reflect new color normalization behavior Fixes: https://linear.app/a8c/issue/CHARTS-138
|
@copilot try to commit again. I have updated your firewall allowlist |
The commit has been successfully pushed! The branch is now up to date with all changes for CHARTS-138, including:
Commit: 7cd8804 |
|
@copilot the description needs to follow |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Updated the PR description to follow the template format with proper sections including Fixes, Proposed changes, Testing instructions, and other required fields. |
Code Coverage SummaryCoverage changed in 2 files.
|
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.
Pull request overview
This PR adds support for HSL, RGB, and RGBA color formats in theme colors, where previously only hex colors were supported. This fix resolves an issue where CSS variables resolving to non-hex color formats were silently skipped during color cache building.
Key Changes:
- Extended
normalizeColorToHexto handlergba()colors in addition to existingrgb()andhsl()support - Refactored color cache building in
GlobalChartsProviderto usenormalizeColorToHexfor all color processing, ensuring consistent handling across all color formats - Added comprehensive test coverage for new RGBA format support and CSS variables resolving to RGB/HSL/RGBA values
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
projects/js-packages/charts/src/utils/color-utils.ts |
Added rgba() to the list of supported color formats in normalizeColorToHex and updated documentation in parseRgbString to clarify it only handles RGB (not RGBA) |
projects/js-packages/charts/src/utils/test/color-utils.test.ts |
Added comprehensive tests for RGBA color conversion including edge cases with alpha channel and fully transparent colors |
projects/js-packages/charts/src/providers/chart-context/global-charts-provider.tsx |
Refactored color cache building to use normalizeColorToHex for all color processing, replacing manual CSS variable resolution with centralized normalization |
projects/js-packages/charts/src/providers/chart-context/test/chart-context.test.tsx |
Added integration tests for CSS variables resolving to RGB, HSL, and RGBA formats, and updated existing test expectations to reflect 3-digit hex expansion behavior |
projects/js-packages/charts/changelog/charts-138-support-hsl-rgb-rgba-colors |
Added changelog entry documenting the new color format support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes https://linear.app/a8c/issue/CHARTS-138/support-hsl-rgb-and-rgba-theme-colors
Proposed changes:
normalizeColorToHexto handle rgba() colors in addition to rgb() and hsl()global-charts-provider.tsxto usenormalizeColorToHexfor all color processingOther information:
Jetpack product discussion
https://linear.app/a8c/review/charts-add-global-theme-support-for-generating-colors-from-a-single-842566402164#comment-ce4ba572
Does this pull request change what data or activity we track or use?
No
Testing instructions:
Setup
cd projects/js-packages/chartspnpm installVerify the fix
pnpm testchart-context.test.tsxcovering RGB, HSL, and RGBA formatspnpm run buildpnpm run typecheckTechnical details
Before this change: CSS variables resolving to
rgb(),rgba(), orhsl()were silently skipped during color cache building because the code only checked for colors starting with#.After this change: All color formats are normalized using
normalizeColorToHex, which:resolveCssVariable#bad→#bbaadd)Note: Alpha channel information in RGBA colors is lost during hex conversion (this is expected d3-color behavior).
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.