[JEWEL-61] LazyTable Component #3312
Conversation
faogustavo
requested review from
daaria-s,
jakub-senohrabek,
nebojsa-vuksic,
rock3r and
wellingtoncosta
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| } | ||
|
|
||
| fun getAndMeasureOrNull(column: Int, row: Int): LazyTableMeasuredItem? { | ||
| if (column >= columns - 1 || row >= rows - 1) { |
There was a problem hiding this comment.
Bug: Off-by-one error in last cell measurement check
The condition column >= columns - 1 || row >= rows - 1 incorrectly rejects the last valid column and row indices. Valid indices range from 0 to columns - 1 and 0 to rows - 1 inclusive, so the check should be column >= columns || row >= rows. Currently, cells at the last column or last row cannot be measured, causing layout failures when those positions contain the first visible cell.
| * @param position The 2D position (column, row) of the cell. | ||
| * @return The content type of the cell, or null if not specified. | ||
| */ | ||
| override fun getContentType(position: IntOffset): Any? = withCellAt(position) { index -> key?.invoke(index) } |
There was a problem hiding this comment.
Bug: getContentType returns key instead of type in position variant
The getContentType(position: IntOffset) method returns key?.invoke(index) but should return type.invoke(index). This is inconsistent with the getContentType(index: Int) method at line 162 which correctly returns type.invoke(cellIndex). Content types should be determined by the type property, not the key property. This will cause content type mismatches for cell composition and reuse optimization.
|
That's fine, this PR isn't going to be merged soon. Gustavo will rebase. |
- Add LazyTable component for large datasets - Create OverflowBox utility for smart cell content overflow on hover - Provide themed TableViewCell and TableViewHeader components with IntelliJ theme integration - Added theme classes based on IJ table Co-authored-by: James Rose <[email protected]>
faogustavo
force-pushed
the
gv/JEWEL-61
branch
from
76a99e3 to
71a1a1a
Compare
| override fun expectedDistanceTo(index: Int, targetScrollOffset: Int): Float { | ||
| if (state.layoutInfo.pinnedColumns > index) { | ||
| return 0f | ||
| } |
There was a problem hiding this comment.
Bug: Vertical scroll animation checks wrong pinned field
In LazyTableAnimateVerticalScrollScope.expectedDistanceTo(), the code checks state.layoutInfo.pinnedColumns > index when it should check state.layoutInfo.pinnedRows > index. The vertical scroll scope is checking the wrong pinned dimension, which causes incorrect distance calculations for animated vertical scrolling. The horizontal scroll scope correctly checks pinnedColumns at line 266, but the vertical scope has this backwards.
|
I tried it and it looks very impressive, but one thing is very bad and it is performance. On pretty medium harware on linux it feels very very laggy. 🙁 Anyway thank you for this awesome work 🙂 |
4 participants
Note
This PR includes the
platform/jewel/docs/lazy-table.mdfile. It's a base documentation of features and how to implement them to help during reviewMy plan was to use it mainly to support review, but I can put more effort into it if we want to store it somewhere
Note
Most of the work was imported from another branch/fork that had an implementation of it almost complete.
This PR only addresses a few remaining items and adapts the code to Jewel code-style/standards and update to match proposal from the ticket comments
Original Patch File
Evidences
Release notes
New features
Note
Introduce experimental LazyTable with 2D lazy layout, pinned rows/cols, selection, drag-and-drop, resizing, themed table cells/headers, docs, samples, and styling/theme plumbing (incl. OverflowBox and scrollbar adapters).
org.jetbrains.jewel.foundation.lazy.table.LazyTableand full stack: state, measure/layout, animate/scroll, scrollbar adapters, pinned rows/columns, beyond-bounds, and APIs (LazyTableScope,LazyTableState, etc.).lazy.table.selectable.*,lazy.table.draggable.*) plus generallazy.selectable.*andlazy.draggable.*utilities.OverflowBoxfor hover-based overflow rendering.TableViewCell,TableViewHeaderCell,SimpleTextTableViewCell,SimpleTextTableViewHeaderCell.TableStyle,TableColors,TableMetrics, wire throughJewelTheme/DefaultComponentStylingand exposeLocalTableStyle.ScrollbarAdapterproviders for table integration.TableStyle(light/dark) and bridge table colors.Tablesshowcase (simple, selection, draggable, interactable, all-features), new icons and navigation entry.platform/jewel/docs/lazy-table.mdwith usage, features, theming, and requirements.Written by Cursor Bugbot for commit 71a1a1a. This will update automatically on new commits. Configure here.