Skip to content

fix: DH-19614: Add tree table formatting logic to tree model#2461

Merged
gzh2003 merged 7 commits intodeephaven:mainfrom
gzh2003:DH-19614-rollups-should-render-aggregated-constituents-based-on-the-original-column-type
Jun 17, 2025
Merged

fix: DH-19614: Add tree table formatting logic to tree model#2461
gzh2003 merged 7 commits intodeephaven:mainfrom
gzh2003:DH-19614-rollups-should-render-aggregated-constituents-based-on-the-original-column-type

Conversation

@gzh2003
Copy link
Copy Markdown
Contributor

@gzh2003 gzh2003 commented Jun 11, 2025

Part of DH-19614. Fixes an issue with rollups where tree table aggregated constituents were not being rendered based on the original column type. This change ensures that leaf nodes in tree tables use the appropriate column type for formatting and alignment.

Changes:

  • Added tree table specific overrides to IrisGridTreeTableModel:
    • Updated colorForCell to use column.constituentType instead of column,type for leaf node values
    • Updated textAlignForCell to use column.constituentType for leaf node values
  • Added type guard isUITreeRow function in IrisGridTreeTableModel.ts

@gzh2003 gzh2003 requested a review from mofojed June 11, 2025 18:24
@gzh2003 gzh2003 self-assigned this Jun 11, 2025
@gzh2003 gzh2003 marked this pull request as draft June 11, 2025 18:41
@gzh2003 gzh2003 force-pushed the DH-19614-rollups-should-render-aggregated-constituents-based-on-the-original-column-type branch from 53fdfa7 to 42b1a14 Compare June 11, 2025 18:54
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 11, 2025

Codecov Report

Attention: Patch coverage is 92.42424% with 5 lines in your changes missing coverage. Please review.

Project coverage is 47.35%. Comparing base (9de971f) to head (dfd6828).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
...ckages/iris-grid/src/IrisGridTableModelTemplate.ts 0.00% 4 Missing ⚠️
packages/iris-grid/src/IrisGridTreeTableModel.ts 97.61% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2461      +/-   ##
==========================================
+ Coverage   47.31%   47.35%   +0.03%     
==========================================
  Files         723      727       +4     
  Lines       39772    39833      +61     
  Branches    10144     9975     -169     
==========================================
+ Hits        18819    18861      +42     
- Misses      20899    20961      +62     
+ Partials       54       11      -43     
Flag Coverage Δ
unit 47.35% <92.42%> (+0.03%) ⬆️

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.

@gzh2003 gzh2003 changed the title fix: DH-19614: Use constituentType for tree table leaf node formatting fix: DH-19614: Add tree table formatting logic to tree model Jun 11, 2025
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.

Looks like a good start

Comment thread packages/iris-grid/src/IrisGridTreeTableModel.ts
@mofojed
Copy link
Copy Markdown
Member

mofojed commented Jun 13, 2025

We'll want to add unit tests/e2e tests for that

@gzh2003
Copy link
Copy Markdown
Contributor Author

gzh2003 commented Jun 16, 2025

We'll want to add unit tests/e2e tests for that

While working on the e2e tests, I discovered a separate formatting issue and created a new ticket for it: DH-19693.

Since this issue isn’t directly related to the color and alignment changes in the original ticket, I’m planning to wrap it up without the e2e tests for now. Then, once DH-19693 is resolved, I'll complete the e2e tests and submit them in a separate PR.

@gzh2003 gzh2003 requested a review from mofojed June 16, 2025 20:46
@gzh2003 gzh2003 marked this pull request as ready for review June 16, 2025 21: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.

Excellent work, good job filing DH-19693 as well.

@gzh2003 gzh2003 merged commit 1c9a8ed into deephaven:main Jun 17, 2025
11 checks passed
@gzh2003 gzh2003 deleted the DH-19614-rollups-should-render-aggregated-constituents-based-on-the-original-column-type branch June 17, 2025 20:08
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 17, 2025
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.

2 participants