data_table: Rework column/table width API#51060
data_table: Rework column/table width API#51060Anthony-Eid merged 14 commits intozed-industries:mainfrom
Conversation
Extract data table modules into separate files This PR extracts the `tests` and `table_row` modules from `data_table.rs` into separate files to improve code organization. This is preparatory work for the upcoming column width API rework (#2 in the series), where separating mechanical changes from logical changes will make the review easier. The extraction was performed using rust-analyzer's "Extract module to file" command. **Context:** This is part 1 of a 3-PR series improving data table column width handling: 1. **This PR**: Extract modules into separate files (mechanical change) 2. [#51060](#51060) - Introduce width config enum for redistributable column widths (API rework) 3. Implement independently resizable column widths (new feature) The series builds on previously merged infrastructure: - [#46341](#46341) - Data table dynamic column support - [#46190](#46190) - Variable row height mode for data tables Primary beneficiary: CSV preview feature ([#48207](#48207)) Release Notes: - N/A
Anthony-Eid
left a comment
There was a problem hiding this comment.
If we end up splitting up some of data_table.rs into width_managements.rs that should be it's own PR, just to make this one as easy to review as possible.
There was a problem hiding this comment.
I would rather prevent separating data_table.rs into two files, it's inconsistent with the rest of the Zed codebase for the most part, and it would make finding the resizable columns stuff harder for devs that are used to our current organization patterns
There was a problem hiding this comment.
Okay i'll merge it back in
There was a problem hiding this comment.
What's the motivation behind this change? Cargo.toml changes like this should be in their own PR
There was a problem hiding this comment.
it seems i accidentally commited redundant commit
Combine column widths, resize behavior, and table width into a single ColumnWidthConfig enum that makes invalid states unrepresentable. This replaces the old `.column_widths()` + `.resizable_columns()` + `.width()` builder pattern with `.width_config(config)`.
07ffa1f to
a461da9
Compare
|
Does anything besides conflicts prevent this pr from being merged? |
There were some bugs with this PR that I just fixed. The resize behavior didn't work and all tables became resizable instead of being explicitly marked with the intractable function like it was before. I also fixed the pixel mis alignment issues with the dividers for columns. |
data_table: Replace column width builder API with
ColumnWidthConfigenumThis PR consolidates the data table width configuration API from three separate builder methods (
.column_widths(),.resizable_columns(),.width()) into a single.width_config(ColumnWidthConfig)call. This makes invalid state combinations unrepresentable and clarifies the two distinct width management modes.What changed:
ColumnWidthConfigenum with two variants:Static: Fixed column widths, no resize handlesRedistributable: Drag-to-resize columns that redistribute space within a fixed table widthTableResizeBehaviorenum (None,Resizable,MinSize(f32)) for per-column resize policyTableColumnWidths→RedistributableColumnsStateto better reflect its purposewidth_management.rsmodulecsv_preview,git_graph,keymap_editor,edit_prediction_context_viewWhy:
The old API allowed callers to combine methods incorrectly. The new enum-based design enforces correct usage at compile time and provides a clearer path for adding independently resizable columns in PR #3.
Context:
This is part 2 of a 3-PR series improving data table column width handling:
The series builds on previously merged infrastructure:
Primary beneficiary: CSV preview feature (#48207)
Anthony's note
This PR also fixes the table dividers being a couple pixels off, and the csv preview from having double line rendering for a single column in some cases.
Before you mark this PR as ready for review, make sure that you have:
Release Notes: