Repository navigation
[ASNT-29][ASNT-40] feat(table): per-column filtering and global find - #46
Merged
Merged
Conversation
Introduce a view→source row mapping (`visible_rows`) so the grid can narrow without reordering the editable source data. Selection and the in-flight edit now store SOURCE indices, converted from the library's view indices at the single `TableEvent` boundary, so a cell edited in a filtered view writes back to the correct row (previously latent write-back corruption once any view narrowing existed). - Per-column filter: a Google Sheets-style header dropdown (search box + value checklist) via `render_th` + a `Popover`; unchecking values hides their rows. - Global find: a `Search` action (Ctrl+F, scoped to the `TablePanel` context) and a title-bar find bar with next/prev and an "N of M" readout, reusing the native cell-selection highlight to locate matches. - The two share one `contains` predicate; no matcher/filter abstraction. The `#` column now shows the stable source row number. Filtering composes with virtualization for free (`rows_count` re-queried each render).
devnull03
changed the title
devnull03
changed the title
devnull03
changed the title
The settings body was wrapped in `overflow_y_scrollbar`, which re-parents its child under an `h_auto` content div. `Settings` sizes its sidebar via `h_resizable`, whose panel widths are latched from a measured `on_prepaint` bounds and only recomputed when that bounds changes — so the group latched a degenerate width on the first frame and only corrected while the window was being resized. The nested scroll was also redundant: `SettingPage` already scrolls its own content and expects a definite-height parent.
Reworks the column filter end to end and adds the settings machinery that
switches it on.
Filter UI:
- Rebuild the dropdown to match the app's idiom: bordered/rounded value list,
row hover, muted empty state, taller search box, outlined All/None buttons.
- Rename "Select all"/"Clear" to All/None — "Clear" excluded every value, the
opposite of what it read as.
- Render values as a Checkbox *child* rather than .label(): the label path
forces line_height(1.0) under an overflow_hidden parent, which clipped
descenders and made underscores in ids vanish entirely.
- Virtualize the value list (gpui_component::v_virtual_list), replacing the
200-item cap.
- Key checkboxes by value, not list position, so searching doesn't leave
hover/focus on the wrong row.
- Show blank cells as a muted "Empty" instead of an unclickable-looking gap.
- Swallow horizontal wheel in the list so it stops scrolling the table under it.
- Swap the chevron for the settings-2 slider glyph; the bundled set has no
funnel.
Settings:
- New settings::columns — per-column preferences, project-scoped, stored as one
JSON blob keyed by the table's stable c{ix} column key. A map rather than
ColumnLayout's positional vec, so a changed header set costs only the columns
that went away instead of everyone's preferences.
- New Columns settings page: add a column to unlock its settings. Widens
build_pages to fn(&App) so a page can see the open project.
- Filtering is now off for every column until switched on there.
Also:
- New settings::dirty, a global unsaved-work tracker (dirty domains + a
monotonic revision) for the coming undo/redo and autosave work. Marks are
wired at the mutation sites now so those features inherit them.
- Guard TablePanel's project observer on the project path. CurrentProject is
mutated by every project-scoped setting write, so it fired far more often
than the project changed — a settings toggle re-ran set_data and wiped the
user's filters and selection.
Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets
-- -D warnings -A dead_code, and cargo test --workspace (41 passing) all green.
Filter dropdown driven manually in the running app: open/close, All/None round
trip, empty state, and the descender fix confirmed on screen.
The page had three stacked heading levels and put its controls in the wrong kind of widget. - Replace the add-column Dropdown with an Add button opening a popover list. The dropdown couldn't scroll, and picking from it read as choosing a value rather than performing an action. - Collapse to two levels: an "Add column" heading and a single subtitle row carrying the Add button on its right, at the same height. - Reword "unlock its settings" to "change its settings". - Render each column's name at text_base/semibold instead of the item-title text_sm, so it stands out as the section it heads. - Swap the Remove switch for an icon button beside the column name. A switch read as a toggleable state when it is a one-shot action. The per-column header is a SettingItem::render custom row rather than a group title, since a group title is text-only and the name needs its button beside it. Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings -A dead_code, cargo test --workspace (41 passing) all green.
- Give each per-column group a title so it lists as a sub-entry under "Columns" in the settings sidebar, alongside "Add column". The sidebar only picks up groups that have a title, which is why the columns were missing. A group title is text-only, so Remove moves to its own right-aligned row directly beneath the name. - Bind Ctrl+, to OpenSettings. The action and its global handler already existed for the menu item, so this is only the keybinding. Also tracks CLAUDE.md and the .gitignore change that untracked it. Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings -A dead_code, cargo test --workspace (41 passing) all green.
`v_virtual_list` mints a fresh `VirtualListScrollHandle` on every call, and the popover rebuilds its content each frame, so the offset was reset to zero as fast as it could be scrolled — the list looked frozen. Hold the handle on the delegate instead, next to `filter_search`, which is there for the same reason, and hand it to the list via `track_scroll`. Also jump back to the top when the value-search box changes: narrowing the list could otherwise leave the offset past the end of the shorter list, showing an empty box. Verification: cargo fmt --all --check, cargo clippy --workspace --all-targets -- -D warnings -A dead_code, cargo test --workspace (41 passing) all green.
Rework the find bar to match Zed's search: match-case, whole-word and regex toggles, previous/next buttons, and an "Invalid regex" readout. All three modes compile to a single regex; regex escaping and the (?i) and word-boundary wrappers keep plain substring search behaving as before.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-user search and filtering capabilities to the qrate table by separating view rows (visible/filtered) from source rows (editable backing data), and wiring UI affordances for a per-column filter popover plus a Ctrl+F-scoped find bar. Also introduces project-scoped column preferences in Settings and persists a Details-panel image pane height.
Changes:
- Implemented view→source row mapping in
QrateTableDelegateto support filtering without reordering editable source data; updated selection/edit bridging accordingly. - Added Google-Sheets-style per-column filter UI (popover with value search + checklist) and a scoped find bar with next/prev navigation and Zed-like toggles (case/word/regex).
- Added project-scoped per-column settings UI (“Columns” settings page) and some new persistence/dirty-tracking plumbing.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/workspace/src/panels/details.rs | Persist/restore Details image pane height per project. |
| crates/table/src/panel.rs | TablePanel integration: Ctrl+F action, find bar UI, view→source event bridging, project reload guard, layout persistence changes. |
| crates/table/src/lib.rs | Export Search action and add filter module. |
| crates/table/src/filter.rs | New per-column filter popover UI (value search + checklist). |
| crates/table/src/editing.rs | Treat edit row as source index; mark project data dirty on commit. |
| crates/table/src/delegate.rs | Core: replace grid with source rows + visible_rows mapping; filtering and search match engine; header render hook. |
| crates/table/src/cell.rs | Clarify row index semantics (source vs view). |
| crates/table/Cargo.toml | Add regex dependency for find functionality. |
| crates/settings/src/lib.rs | Add columns/dirty modules; make SettingsWindow pages builder take &App. |
| crates/settings/src/dirty.rs | New global dirty tracker (domains + revision). |
| crates/settings/src/columns.rs | New project-scoped per-column settings persistence API. |
| crates/app/src/main.rs | Initialize dirty tracker; clear dirty state after synchronous flush on quit. |
| crates/app/src/app_settings/mod.rs | Add “Columns” settings page UI (add/remove columns, toggle filter enable). |
| crates/app/src/actions.rs | Add Ctrl+, binding for Settings and scoped Ctrl+F binding for table find. |
| CLAUDE.md | Add repository collaboration/style guidance document. |
| Cargo.lock | Lockfile update for regex. |
| .gitignore | Stop ignoring CLAUDE.md and remove other previously-ignored patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+99
to
+102
| let shown: Vec<SharedString> = values | ||
| .into_iter() | ||
| .filter(|v| needle.is_empty() || v.to_lowercase().contains(&needle)) | ||
| .collect(); |
Comment on lines
+3
to
+10
| //! qrate persists continuously through debounced writers, so today nothing in the app has to ask | ||
| //! whether state is dirty. This exists for the work that will: hand-rolled undo/redo needs a change | ||
| //! cursor, and an explicit autosave needs to know what still owes a write. Marking is therefore | ||
| //! wired up now, at every site that mutates persisted state, so those features inherit correct | ||
| //! marks instead of having to retrofit them. | ||
| //! | ||
| //! Two things are tracked: a set of dirty *domains* (what changed) and a monotonic *revision* | ||
| //! (how many changes have happened). Undo/redo wants the revision; autosave wants the domains. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related
Summary
Implements find + per-column filtering for the qrate table (ASNT-29 / ASNT-40). Sorting stays deferred per the owner's decision.
The load-bearing change is a view→source row split. Today view index == source index, so every site conflated them; filtering breaks that. A
visible_rows: Vec<usize>(view→source) narrows the grid without ever reordering the editable source data (grid.rowsand the parallelimage_pathsstay put). Selection and the in-flight edit now store SOURCE indices, converted from the library's view indices at the singleTableEventboundary inpanel.rs— sodetails.rs,cell_location.rs, androw_imageneed zero changes, and a cell edited in a filtered view writes back to the correct row (this was latent write-back corruption the moment any narrowing existed).Per-column filter — Google Sheets style
set_column_filter_enabled+ the Columns settings page); most headers render just the name.TableDelegate::render_th+ aPopover(not the action-onlyPopupMenu, which closes on every click and can't host a live checklist/search box).Global find — Zed-style search bar
Searchaction declared incrate::table(app → table is one-way), bound to Ctrl+F scoped to theTablePanelcontext incrates/app/src/actions.rs(global would steal Ctrl+F from the cell editor'sInput). The toolbar Search button toggles it directly via a weak handle.render(nottitle_suffix, which the parentTabPanelnever observes, so the bar would sit invisible until an unrelated redraw). Layout mirrors Zed's search: query input, three toggles, match count, prev/next, close.regex::Regex: non-regex queries areregex::escaped, whole-word wraps in\b(?:…)\b, case-insensitive prefixes(?i). An unparseable pattern flips the readout to "Invalid regex" instead of a misleading "No results".set_selected_cellscrolls the match to center) instead of span-level highlighting.Filtering composes with virtualization for free (
rows_countis re-queried each render, so it needs onlycx.notify()). The#column shows the stable source row number.Cut from this PR (as scoped): search-and-replace, span-level match highlighting, filter persistence in
.qrate, and persisting the find-bar toggle state across sessions.Groundwork for later (intentionally unused)
Some scaffolding lands here ahead of its consumers rather than being retrofitted later:
settings::dirty— a global unsaved-work tracker (dirty domains + a monotonic revision), marked at every mutation site now so a future undo/redo or explicit autosave inherits correct marks. No readers yet.crates/ai— provider trait + mock/Cohere scaffolding for the planned agent panel.components::labeled_select— a reusable labeled-dropdown element for settings/wizard forms.Verification
CI runs Windows/macOS (gpui won't build on Linux CI). Ran locally before pushing:
cargo fmt --all --check— clean.cargo clippy --workspace --all-targets -- -D warnings -A dead_code— clean.cargo test -p table— 13 passed, including filter narrowing, view/source index divergence, case-insensitive find scoped to the visible set, and the newsearch_toggles_case_word_and_regex(match-case, whole-word, regex alternation, and invalid-pattern → no matches).gpui can't run headless here, so the interactive paths (popover focus/live-narrowing, Ctrl+F focus, Escape-to-dismiss, the toggle buttons) are verified by compile + the logic unit tests, not by driving the app. Worth a quick manual pass in the running app before merge.
🤖 Generated with Claude Code