feat: Deephaven UI table databar support

[mattrunyon]

Changes needed for dh.ui databars

• Support for extracting data in viewport columns not listed in table.columns. This allows us to hide the aggregation columns for databars from the user but still get their data. If a column is not in table.columns then it will be added to viewport data via its name instead of model index
• Improved gradient rendering to be much more efficient
• Fixed opacity being ignored if a gradient was used
• Fixed some of the databar spacing (was 1px off center)
• Modified colorValueStyle to have overrides so if we pass a string we don't get back string | undefined when we know we'll get a string
• Modified resolveCssVariablesInRecord to always resolve CSS colors regardless of if they contain a variable

[codecov]

Codecov Report

Attention: Patch coverage is 18.91892% with 60 lines in your changes missing coverage. Please review.

Project coverage is 46.64%. Comparing base (5b4ce50) to head (850d476).

Files	Patch %	Lines
packages/grid/src/DataBarCellRenderer.ts	3.44%	56 Missing ⚠️
...ckages/iris-grid/src/IrisGridTableModelTemplate.ts	57.14%	3 Missing ⚠️
packages/iris-grid/src/IrisGridTreeTableModel.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2190   +/-   ##
=======================================
  Coverage   46.64%   46.64%           
=======================================
  Files         692      692           
  Lines       38578    38571    -7     
  Branches     9758     9855   +97     
=======================================
- Hits        17995    17992    -3     
+ Misses      20530    20526    -4     
  Partials       53       53           

Flag	Coverage Δ
unit	46.64% <18.91%> (+<0.01%)	⬆️

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.

[mofojed]

Test case for when this is true?

[mattrunyon]

Should be pretty much all the other test cases. We mocked it to true by default

[mofojed]

With the levels of context.saves(), it'd be nice if they were at the start/end of a block, extracting this info out or just enclosing it in blocks. Maybe even have our own function for it?

function withContextState(context, callback: () => undefined) {
  context.save();
  callback();
  context.restore();
}

Just musing though. Not necessary to change, but I did find it a little confusing seeing two context.restore()s close to each other below, and then tracing it back to the matching context.save()

[mattrunyon]

Ya that would be neat and I agree it is a bit confusing. It's because you have to clip first and then draw, so

I added the 2nd context save/restore because I needed it for re-using the gradient across multiple parts of the grid (or even multiple grids since I made the cache static on the renderer)

I think I might be able to simplify some of it and at least put both restores near each other by using the flip/translate for everything based on LTR or RTL. Basically we don't need to calculate special positioning based on if it's LTR or RTL, but instead calculate just based on LTR. Then if it's RTL, the context scale(-1, 1) w/ translation makes it draw RTL even though the coordinates are LTR. I already did this w/ the gradient, but could also add it for the markers and axis

[mofojed]

This shouldn't be necessary? I think it should map to the dataBarOptionsForCell already no?

[mattrunyon]

No. This is part of model.viewportData.

The problem is when we subscribe to columns and the UITableModel in plugins adds the auto-agg columns to the subscription, but hides them from model.columns. In the extractViewportRow method, we expect the column to exist in model.columns because we index based on our model index.

For the auto-agg columns which are hidden from model.columns, but added to the subscription, these show up in extractViewportRow, but don't have a model index associated with the column.

So this change is so subscribing to columns that aren't part of model.columns works without throwing.

The value is then read by column name from the model viewportData in the plugin

[mofojed]

Ah yea - extractViewportRow should be using this.table.columns instead of this.columns, then I think it works out.
The viewport data should only be accessed internally from that model, so the access should always match up with the table columns I believe.

[mattrunyon]

I'll try moving the proxy off of the table and onto the model in the plugin for columns. model.columns just returns this.table.columns by default.

My concern with that was somewhere not using model.columns and instead using table.columns and causing some breakage

Just glancing through IrisGridTableModelTemplate, get columnMap accesses this.table.columns instead of this.columns, but looks like that's the only place in the template model. It's just something that's hard to guard against

[mattrunyon]

And using this.table.columns in extractViewportRow might not fix it. It already iterates the columns in the viewport data and then we lookup the index with this.getColumnIndexByName. So if the model hides the column from this.columns, then it still won't have an index for aggregations when extracting the viewport row.

[mofojed]

Everything outside of the model itself should be using model.columns instead of accessing table.columns directly. Shouldn't even be trying to access the table at all.

this.getColumnIndexByName is getting the index within the model, it's a public API defined in the abstract class IrisGridModel. Since the viewportData extracted in extractViewportData is entirely internal to the implementation of the model, using this.getColumnIndexByName isn't correct - it should really be looking at the table columns and getting the index from that, then everything accessing the viewportData should be mapping the public "model" index to the private table index. In general those have matched up so it hasn't been a problem before. Kind of tricky to sort out which set of columns to use... If I had a redesign, I'd probably go for a immutable fluent design (similar to Table on the server) such that applying an operation (such as databars hiding columns) would handle it all in it's layer, making it clearer how it's mapping columns and it only needs to handle one thing. Then just apply layers on top of that for each other operation.

Side note - why do we even need the get columnMap? It doesn't seem to actually be used anywhere except to get the size of the map in getMemoizedInitalMovedColumns. Should we just get rid of that?

[mattrunyon]

So do you want me to change anything here? I can get rid of get columnMap

I think the proper way (as a separate ticket) would be to move away from the notion of a model index and just to names or visible index. We already do this for some things (like sorts seem to be serialized by name), so I'm not sure where model index should be used over the column name.