Skip to content

feat: Table rendering support for databars#1212

Merged
mofojed merged 18 commits intodeephaven:mainfrom
emilyhuxng:feature-1151-table-rendering-support-for-databars
May 4, 2023
Merged

feat: Table rendering support for databars#1212
mofojed merged 18 commits intodeephaven:mainfrom
emilyhuxng:feature-1151-table-rendering-support-for-databars

Conversation

@emilyhuxng
Copy link
Copy Markdown
Contributor

@emilyhuxng emilyhuxng commented Apr 6, 2023

Closes #1151

The example can be seen at the style guide.

IrisGridRenderer and GridRenderer now use different cell renderers depending on the type.

@emilyhuxng emilyhuxng added this to the April 2023 milestone Apr 6, 2023
@emilyhuxng emilyhuxng self-assigned this Apr 6, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 6, 2023

Codecov Report

Merging #1212 (669384b) into main (1b29af1) will decrease coverage by 0.31%.
The diff coverage is 27.55%.

❗ Current head 669384b differs from pull request most recent head f482077. Consider uploading reports for the commit f482077 to get more accurate results

@@            Coverage Diff             @@
##             main    #1212      +/-   ##
==========================================
- Coverage   45.57%   45.27%   -0.31%     
==========================================
  Files         486      485       -1     
  Lines       34101    34167      +66     
  Branches     8538     8576      +38     
==========================================
- Hits        15543    15470      -73     
- Misses      18507    18646     +139     
  Partials       51       51              
Flag Coverage Δ
unit 45.27% <27.55%> (-0.31%) ⬇️

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

Impacted Files Coverage Δ
packages/code-studio/src/styleguide/Grids.tsx 100.00% <ø> (ø)
packages/grid/src/DataBarGridModel.ts 0.00% <0.00%> (ø)
packages/grid/src/GridMetricCalculator.ts 75.59% <0.00%> (-0.65%) ⬇️
packages/grid/src/GridTheme.ts 100.00% <ø> (ø)
packages/grid/src/MockDataBarGridModel.ts 0.00% <0.00%> (ø)
...ackages/iris-grid/src/IrisGridCellRendererUtils.ts 0.00% <0.00%> (ø)
...kages/iris-grid/src/IrisGridDataBarCellRenderer.ts 0.00% <0.00%> (ø)
packages/iris-grid/src/IrisGridTheme.ts 100.00% <ø> (ø)
...rid/src/mousehandlers/IrisGridTokenMouseHandler.ts 4.22% <0.00%> (-0.54%) ⬇️
packages/grid/src/DataBarCellRenderer.ts 0.38% <0.38%> (ø)
... and 14 more

... and 41 files with indirect coverage changes

Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
@emilyhuxng emilyhuxng force-pushed the feature-1151-table-rendering-support-for-databars branch from 83234d6 to c467052 Compare April 12, 2023 14:22
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/CellRenderer.ts Outdated
state: GridRenderState,
column: VisibleIndex,
row: VisibleIndex,
textOverride?: string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not part of your change, but just something that bugs me - textOverride is a property that's really only relevant to the TextCellRenderer, not the general CellRenderer case. I feel like the text override (which is only really used in IrisGridRenderer to display null or empty in a specific case) should be pushed down into model.textForCell, and even have a model.fontForCell... there may have been other implications for why we didn't do that. @mattrunyon added it as part of PR #408 , but it doesn't look like I had any comments there about pushing textOverride down to the model and why we went with the direction we did.

Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/iris-grid/src/IrisGridRenderer.ts Outdated
Comment thread packages/iris-grid/src/IrisGridRenderer.ts Outdated
Comment thread packages/iris-grid/src/IrisGridRenderer.ts Outdated
Comment thread packages/iris-grid/src/mousehandlers/IrisGridTokenMouseHandler.ts Outdated
Comment thread packages/grid/src/CellRenderer.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/MockDataBarGridModel.ts Outdated
@emilyhuxng emilyhuxng requested a review from mofojed April 18, 2023 20:28
Comment thread packages/grid/src/DataBarGridModel.ts Outdated
Comment thread packages/grid/src/DataBarGridModel.ts Outdated
Comment thread packages/grid/src/GridMetricCalculator.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/GridModel.ts Outdated
Comment thread packages/grid/src/CellRenderer.ts Outdated
Comment thread packages/iris-grid/src/IrisGridTextCellRenderer.ts
Comment thread packages/iris-grid/src/IrisGridTableModelTemplate.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/DataBarCellRenderer.ts Outdated
Comment thread packages/grid/src/GridMetricCalculator.ts
Comment thread packages/iris-grid/src/IrisGridDataBarCellRenderer.ts Outdated
@emilyhuxng emilyhuxng requested review from mofojed April 21, 2023 16:00
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! As discussed, will not be merging until start of May.

Comment thread packages/iris-grid/src/IrisGridTextCellRenderer.ts
@mofojed mofojed marked this pull request as ready for review May 4, 2023 15:45
@mofojed mofojed enabled auto-merge (squash) May 4, 2023 15:46
@mofojed mofojed merged commit a17cc0e into deephaven:main May 4, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Table rendering support - registering/using custom cell renderers

3 participants