Skip to content

fix: DH-21616: use model text color instead of databar color for cell text #2629

Draft
gzh2003 wants to merge 3 commits intodeephaven:mainfrom
gzh2003:DH-21616-databar-overrides-text-color-when-using-format-prop
Draft

fix: DH-21616: use model text color instead of databar color for cell text #2629
gzh2003 wants to merge 3 commits intodeephaven:mainfrom
gzh2003:DH-21616-databar-overrides-text-color-when-using-format-prop

Conversation

@gzh2003
Copy link
Contributor

@gzh2003 gzh2003 commented Feb 27, 2026

For DH-21616. Previously, DataBarCellRenderer used the databar fill color for both the bar and the text, ignoring any explicit text color set via TableFormat.color.

Now calls model.colorForCell() to check for an explicit text color. If one is set, it takes precedence over the databar color for text rendering. When no explicit text color is set, the text still inherits the databar color.

@gzh2003 gzh2003 requested a review from mofojed February 27, 2026 13:46
@gzh2003 gzh2003 self-assigned this Feb 27, 2026
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.77%. Comparing base (0cab21d) to head (ee911da).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
packages/grid/src/DataBarCellRenderer.ts 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2629      +/-   ##
==========================================
+ Coverage   49.48%   49.77%   +0.28%     
==========================================
  Files         772      774       +2     
  Lines       43738    43777      +39     
  Branches    11066    11262     +196     
==========================================
+ Hits        21643    21788     +145     
+ Misses      22077    21971     -106     
  Partials       18       18              
Flag Coverage Δ
unit 49.77% <0.00%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dsmmcken
Copy link
Contributor

This was a stylistic choice. However, I agree that explicitly setting a color should override it.

Does this code still maintain that choice, if I make a purple, or gradient bar with no explicit text color set, is the color still the same as it was?

@gzh2003 gzh2003 marked this pull request as draft February 27, 2026 16:38
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.

2 participants