Use .blue() instead of .b() in textColorForBackgroundColor - #1031
Conversation
WalkthroughUpdated Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…call Color.blue() refs Ghost (core) Issue #27797 - deleted test that was asserting the buggy behavior
refs TryGhost/Ghost#27797 Kept the distinguishing #cccccc case and updated its expected result so reverting from Color.blue() to the Lab b-channel fails the package tests.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1031 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 30 30
Lines 3060 3060
Branches 459 459
=========================================
Hits 3060 3060 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9larsons
left a comment
There was a problem hiding this comment.
Reviewed the corrected RGB blue-channel implementation and generated artifacts. Added regression coverage that fails if Color.b() is reintroduced. The Color Utils build/tests, SDK-wide lint/tests, and upstream Node 20/22 checks all pass.
refs #1031 refs TryGhost/Ghost#27797 Covered the reported lavender, pink, and blue backgrounds plus grays surrounding the YIQ threshold without changing existing assertions.
Summary
Fixes
textColorForBackgroundColorusingColor.b(), which returns the Lab blue-yellow channel rather than the RGB blue channel. The YIQ calculation now usesColor.blue().Supersedes #905 and fixes TryGhost/Ghost#27797.
Changes
.b()with.blue()in the Color Utils source.#ccccccregression case and updates its expected contrast text from white to black.Validation
yarn workspace @tryghost/color-utils buildyarn workspace @tryghost/color-utils test— 11 tests passed with 100% coverageyarn test— lint and tests passed across all 17 SDK packagesThere's a clear use-case for this code change
Commit messages have short titles and reference the relevant issue
The build, lint, and tests pass