-
Notifications
You must be signed in to change notification settings - Fork 273
feat(legacy-plugin-chart-calendar): increase the contrast of calendar heatmap color and label #1452
feat(legacy-plugin-chart-calendar): increase the contrast of calendar heatmap color and label #1452
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/superset/superset-ui/2WNYCb3pL7k4j7qwUBSP3WeVxFBx |
@@ -1652,6 +1652,20 @@ CalHeatMap.prototype = { | |||
} | |||
} | |||
|
|||
function formatTextFill(value) { |
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.
This is super nifty! I wonder if we should label it as something more descriptive, like "getContrastingColor" so that we can eventually (after monorepo migration, perhaps) export it from a shared module, like Superset's colorUtils.ts
file.
ef6b183
to
96e150c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1452 +/- ##
==========================================
+ Coverage 30.48% 30.62% +0.14%
==========================================
Files 498 499 +1
Lines 10019 10048 +29
Branches 1691 1699 +8
==========================================
+ Hits 3054 3077 +23
- Misses 6719 6723 +4
- Partials 246 248 +2
Continue to review full report at Codecov.
|
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.
LGTM, Thanks for adding the complete testing.
🏆 Enhancements
The algorithm is to decide label color in white or black depending on background color (refer to https://stackoverflow.com/questions/3942878/how-to-decide-font-color-in-white-or-black-depending-on-background-color)