feat(tables): row-gutter drag-select, Cmd+F find, and select-all polish#4901
feat(tables): row-gutter drag-select, Cmd+F find, and select-all polish#4901TheodoreSpeaks wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Integrations & blocks: Skills and commands now require exporting Connectors & validation: API & client patterns: Global rules now point BYOK registration at Architecture & UI: Docs site: Removes Form deployment and A2A / Circleback tool pages; adds knowledge and table to blocks nav; updates Mothership tables copy to the Table block; quick-reference drops workflow color; icon mapping and several tool page Reviewed by Cursor Bugbot for commit 8205fae. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 19cff3c. Configure here.
| enabled: Boolean(workspaceId && tableId) && q.trim().length > 0, | ||
| staleTime: 30 * 1000, | ||
| placeholderData: keepPreviousData, | ||
| }) |
There was a problem hiding this comment.
Empty find keeps stale matches
Medium Severity
Submitting an empty trimmed find term disables useFindTableRows, but keepPreviousData leaves the last successful findData in place. The find bar can still show the previous match count and allow next/prev navigation even though no active query matches the cleared input.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 19cff3c. Configure here.
| useEffect(() => { | ||
| setCurrentMatchIndex(0) | ||
| if (findMatches.length > 0) goToMatch(0) | ||
| }, [findMatches, goToMatch]) |
There was a problem hiding this comment.
Find refetch jumps to first
Medium Severity
A useEffect keyed on findMatches always calls goToMatch(0) when that memo’s result changes. Any refetch or findData update recomputes a new sorted array (new reference), resetting the active match to the first hit even if the user was on a later match and the hits are unchanged.
Reviewed by Cursor Bugbot for commit 19cff3c. Configure here.
Greptile SummaryThis PR adds three table-grid improvements: row-gutter drag-to-select (with auto-scroll), a Cmd/Ctrl+F in-table find backed by a new
Confidence Score: 4/5The backend find endpoint and drag-select logic are solid; the main risks are in the find navigation UX within table-grid.tsx. The SQL service layer, API route, contracts, and React Query hook are all well-structured and tested. The three concerns are in the client-side orchestration: the Cmd+F shortcut intercepts globally without checking that the table container has focus; rapid next/prev calls can cause the loading spinner to disappear while a jump is still in flight; and toggling column visibility while find is open silently resets the user's current match position to 0. apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx — find shortcut scoping, isJumping race, and displayColumns-triggered match reset. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant TG as TableGrid
participant TF as TableFind UI
participant RQ as useFindTableRows
participant API as GET /rows/find
participant DB as Postgres
U->>TG: Cmd+F
TG->>TF: setFindOpen(true), focus input
U->>TF: type query, press Enter
TF->>TG: onSubmit() → setSubmittedQuery(q)
TG->>RQ: "enabled=true, q=submittedQuery"
RQ->>API: "GET ?q=...&filter=...&sort=..."
API->>DB: CTE row_number() + CROSS JOIN LATERAL jsonb_each_text ILIKE
DB-->>API: "[{ordinal, id, column_name}] (≤1001 rows)"
API-->>RQ: "{matches, truncated}"
RQ-->>TG: findData updated
TG->>TG: findMatches memoized (filter by displayColumns)
TG->>TG: useEffect → goToMatch(0)
TG->>TG: await ensureRowsLoadedUpTo(ordinal+1)
TG->>TG: "pendingMatchRef = match, setPendingMatchTick++"
TG->>TG: "reveal effect: setSelectionAnchor({rowIndex, colIndex})"
TG-->>U: cell highlighted + scrolled into view
U->>TF: press Enter (next)
TF->>TG: onNext() → goToMatch(currentIndex+1)
TG->>TG: await ensureRowsLoadedUpTo(match.ordinal+1)
TG-->>U: next match highlighted
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| useEffect(() => { | ||
| if (embedded) return | ||
| const handleFindShortcut = (e: KeyboardEvent) => { | ||
| if (!(e.metaKey || e.ctrlKey) || e.key !== 'f') return | ||
| if (!containerRef.current) return | ||
| e.preventDefault() | ||
| setFindOpen(true) | ||
| requestAnimationFrame(() => { | ||
| findInputRef.current?.focus() | ||
| findInputRef.current?.select() | ||
| }) | ||
| } | ||
| document.addEventListener('keydown', handleFindShortcut) | ||
| return () => document.removeEventListener('keydown', handleFindShortcut) | ||
| }, [embedded]) |
There was a problem hiding this comment.
The handler attaches to document and only checks containerRef.current (component is mounted), not whether focus is within the table container. Any keystroke from an unrelated focused element — a sidebar input, a modal text field, or a browser extension overlay — will call e.preventDefault() and open the table find panel. The existing Ctrl+A handler uses the same pattern, but Cmd+F is a more universally expected browser shortcut and overriding it unconditionally is more likely to surprise users. Adding a containerRef.current.contains(document.activeElement) guard would confine the override to when the table already has focus.
Find: - New GET /api/table/[tableId]/rows/find endpoint + contract + useFindTableRows hook - findRowMatches: case-insensitive substring search across every cell via a row_number() CTE + jsonb_each_text + ILIKE, returning each matching cell with its ordinal in the filtered/sorted view. Shared buildRowOrderBySql keeps the find ordinals aligned with the paginated list (with an id tiebreak). - Cmd/Ctrl+F floating find box (search-on-Enter), cell-by-cell next/prev that loads the target row then reveals the cell via the existing anchor scroll. Gutter UI: - Row number/checkbox centered in the region left of the run button; the select-all header checkbox centers in the same region so they line up. - emcn Checkbox gains an indeterminate (minus) state; select-all shows the minus on any partial selection and clears everything on click. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire the existing "Rename" item in the per-table context menu to inline rename (useInlineRename + InlineRenameInput via the Resource name cell's `content` override), matching the Files list. Enter/blur saves, Esc cancels; replaces the prior modal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reuse the workflow search visual language (surface-1 panel, emcn Input, muted "k of N" counter, size-8 ghost chevron buttons) instead of the flat custom input. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TheodoreSpeaks
force-pushed
the
improvement/table-select-ui
branch
from
19cff3c to
8205fae
Compare
1 participant
Summary
Three table-grid improvements on this branch:
Find — how it works
GET /api/table/[tableId]/rows/find(contract +useFindTableRowshook). Search is Enter-triggered (Clay-style), respecting the view's active filter/sort.findRowMatchesruns arow_number()CTE +CROSS JOIN LATERAL jsonb_each_text+ILIKE, returning each matching cell as{ ordinal, rowId, column }, capped at 1000 with atruncatedflag.buildRowOrderBySql(now used by bothqueryRowsandfindRowMatches, with anidtiebreak) keeps a match's ordinal aligned with its index in the paginated list, so the client can page up to and reveal the exact cell.Performance / scope (deliberate v1 choices)
ILIKEoverjsonb_each_textcan't use the JSONB GIN index; the scan is bounded by thetable_idbtree. Sub-500ms for tables ≪100k rows. Apg_trgmGIN index on a text projection is the documented future accelerator.ensureRowsLoadedUpTo(loading state shown); fine for target sizes, with parallel/offset-windowed loading as a follow-up if needed.Notable change to shared code
emcnCheckboxgains an indeterminate (minus) state — additive; no change to checked/unchecked usages.Tests / checks
findRowMatchesunit test (mapping, truncation, filter/sort threading) and a/rows/findroute test (auth, validation, workspace mismatch, error mapping). 11 passing.bun run lint:check,type-check, andcheck:api-validationall pass (route-count baseline bumped 791→792 for the new route).{ordinal, column}matches in ~60ms.🤖 Generated with Claude Code