fix: resolve reported data grid, editor, and filter UX issues#1773
fix: resolve reported data grid, editor, and filter UX issues#1773datlechin wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ed9dc8f87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onDismiss: { tab.display.activeResultSet?.errorMessage = nil } | ||
| onDismiss: { | ||
| tab.display.activeResultSet?.errorMessage = nil | ||
| tab.execution.errorMessage = nil |
There was a problem hiding this comment.
When the banner is showing tab.execution.errorMessage, this dismiss handler tries to assign through the QueryTab value captured by resultsSection(tab:). Unlike activeResultSet (a reference type), execution is a stored struct property on an immutable function parameter, so this breaks the build instead of clearing the error; route the dismissal through tabManager.mutate(tabId:) or another mutable tab-state path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73bd3840c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func fixErrorWithAI(error: String) { | ||
| guard let query = lastQueryErrorSQL else { return } |
There was a problem hiding this comment.
This reads a coordinator-global lastQueryErrorSQL, which is overwritten by the next failing query in any tab. Because the banner is stored per tab via tab.execution.errorMessage, a user can fail a query in tab A, fail a different query in tab B, then return to tab A and click Fix with AI; the AI receives tab B's SQL paired with tab A's error. Store or pass the failed SQL with the tab/result that owns the banner instead of using a single coordinator field.
Useful? React with 👍 / 👎.
| private func executionErrorBanner(tab: QueryTab) -> some View { | ||
| if let error = tab.execution.errorMessage, | ||
| tab.display.activeResultSet?.errorMessage == nil, | ||
| tab.display.resultSets.count <= 1 { |
There was a problem hiding this comment.
When a tab has a pinned result (or otherwise has more than one result set), this condition suppresses the new execution-level banner. The single-statement error path only sets tab.execution.errorMessage and leaves the existing display.resultSets in place, so after a failed run with pinned results the UI keeps showing the old results and no error banner, while the modal sheet was removed in this commit. The result-set count should not hide execution errors that are not represented by an active result set.
Useful? React with 👍 / 👎.
| let liveWidths = currentColumnWidths() | ||
| guard !liveWidths.isEmpty else { return saved } | ||
| return (saved ?? ColumnLayoutState()).mergingWidths(liveWidths) |
There was a problem hiding this comment.
This merges current NSTableView widths even on updates where the result schema changed. In applyStructuralUpdate, rebuildColumnMetadataCache(from:) has already swapped identitySchema to the new columns before this runs, while the table still contains slot-based columns from the previous result (dataColumn-0, etc.), so old widths are attributed to unrelated new column names by position. After resizing a column and running a different SELECT, the new result can inherit stale widths instead of its stored/default layout; only merge live widths when the schema is unchanged, or capture them before replacing the schema.
Useful? React with 👍 / 👎.
1 participant
Fixes a batch of UX issues reported from real first-use feedback. Investigated with parallel codebase tracing across the data grid, SQL editor, filter bar, and the bottom toolbar.
Root cause: one bug behind three "stopped working mid-session" symptoms
The dead Columns / + Add buttons, the dead mouse in the SQL editor, and the general "unusable after a while" feeling were all the same bug. The SQL autocomplete popup (
SuggestionControllerin the vendored CodeEditSourceEditor) is a borderless non-activating panel at pop-up-menu level, added as a child window ordered above the main window. Its positioning only flipped the panel above the cursor when it would run off the screen bottom, never the editor's bottom. With the cursor on a lower line and the schema loaded, the panel drew straight down over the status bar and lower editor lines. Being non-activating, clicking it never changed first responder, so it never auto-closed and just ate every click until Esc.Fixes
Editor / autocomplete
FindPanelHostingViewthat leaked an Escape key monitor every time a tab was closed with the Find bar open.cancelHover()on the jump-to-definition failure path.Data grid
mouseDownnow bails to pure resize when the click starts in the resize zone).Filters
Not changed (by design)
Backspace deleting two characters is CodeEditSourceEditor's smart-edit behavior (delete a full indent unit in leading whitespace, or both halves of an auto-inserted bracket pair), the same as Xcode. Left as-is.
Tests
ColumnLayoutStateTests- geometry updates preserve the hidden-column set.FileColumnLayoutPersisterTests-cacheLayoutis in-memory only and is not written to disk.FilterValueTextFieldTests- whitespace suppresses completion; Escape grace keeps the filter bar open.The gesture/positioning fixes (header resize zone, panel geometry) are AppKit-window-bound and verified by hand rather than unit tested.
Notes
LocalPackages/CodeEditSourceEditor. They lint clean under that package's own config.https://claude.ai/code/session_0198faM6VCrViRU4XwRoS1DC