Skip to content

Address large data table review findings#631

Merged
novykh merged 4 commits into
masterfrom
fix/large-data-table-review
Jul 13, 2026
Merged

Address large data table review findings#631
novykh merged 4 commits into
masterfrom
fix/large-data-table-review

Conversation

@novykh

@novykh novykh commented Jul 13, 2026

Copy link
Copy Markdown
Member

No description provided.

@novykh novykh requested a review from ktsaou July 13, 2026 10:31
@novykh novykh force-pushed the fix/large-data-table-review branch from 775db82 to 89e86a0 Compare July 13, 2026 11:06

@ktsaou ktsaou left a comment

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.

Two blocking regressions remain on the current head: selection-state rendering scales with the full data set, and the header-specific tooltip ownership contract is replaced by the generic body-cell contract. Both undermine the large-table work and need to be addressed before merge. The sorting-cache trade-off discussed separately is non-blocking.

Comment thread src/components/table/largeDataSelection.js
Comment thread src/components/table/body/header/cell.js Outdated
@novykh novykh requested a review from ktsaou July 13, 2026 11:26

@ktsaou ktsaou left a comment

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.

The header-tooltip ownership regression is fixed. The selection discussion is resolved: the complete render path already carried the source traversal, while this change preserves correct disabled-row semantics. Focused table tests pass; approved pending the required CI check.

@novykh novykh merged commit 7351bc7 into master Jul 13, 2026
2 checks passed
@novykh novykh deleted the fix/large-data-table-review branch July 13, 2026 11:47
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