fix: DH-21616: Databar overrides text color when using format prop#2640
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2640 +/- ##
==========================================
+ Coverage 49.48% 49.55% +0.07%
==========================================
Files 772 774 +2
Lines 43738 43871 +133
Branches 11066 11295 +229
==========================================
+ Hits 21643 21740 +97
- Misses 22077 22085 +8
- Partials 18 46 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mofojed
left a comment
There was a problem hiding this comment.
The Grid component by design is generally dumb/explicit about what it's rendering; the GridModel is basically explicitly what it's going to render (colorForCell, textForCell, etc). The brains/logic are designed to be in the specific implementations of the GridModel (e.g. IrisGridModel, or UITableModel).
By putting the textColor explicitly in the DataBarOptions, we're keeping that logic of "user explicitly set a format value" in the UITableModel.
| /** | ||
| * The text color for the cell, resolved from format rules/bar color. | ||
| * If the user has explicitly set a text color via formatting, that color is used. | ||
| * Otherwise it defaults to the databar's bar color. | ||
| */ |
There was a problem hiding this comment.
Should just be "the text color for the cell". How it is derived is not determined from within this context.
If we took an optional textColor here and fell back to the color, we could say that. But this behaviour documented here is not accurate from this context.
And then the color attribute should have a doc string for "Color of the databar", maybe explain how you can pass a gradient.
For DH-21616. Databar cells were using the databar bar color for both the bar and the text, ignoring any explicit text color set via formatting.
Companion PR: deephaven/deephaven-plugins#1323