Skip to content

fix(datagrid): keep keyboard focus across the grid filter flow (#1490)#1492

Merged
datlechin merged 2 commits into
mainfrom
fix/focus-grid-filter-flow
May 29, 2026
Merged

fix(datagrid): keep keyboard focus across the grid filter flow (#1490)#1492
datlechin merged 2 commits into
mainfrom
fix/focus-grid-filter-flow

Conversation

@datlechin
Copy link
Copy Markdown
Member

Part of #1490 (keyboard, focus, and accessibility).

Makes the reported grid -> filter -> grid flow work without the mouse, the native way. From the audit, the breaks were all at the SwiftUI filter panel / AppKit grid boundary.

What changed

  • Focus returns to the grid. New TableViewCoordinator.focusGrid() and MainContentCoordinator.focusActiveGrid() route through the existing dataGridAttach(tableViewCoordinator:) registration (no new references). Called after Apply, Unset, removing the last filter row, and Escape.
  • Return applies. The Apply button is now .keyboardShortcut(.defaultAction), and the panel is a .focusSection() so the default action and Tab grouping scope to the panel.
  • Escape works. In FilterValueTextField, cancelOperation now consumes the event only when the suggestion popover is open; otherwise it calls onCancel, which closes the panel and returns focus to the grid. .onExitCommand covers the case where focus is on a SwiftUI control in the panel.
  • Removed the focus hack. Focus into the filter value field no longer uses DispatchQueue.main.async { makeFirstResponder } plus a UUID nil-poke. It is now a proper AppKit first-responder bridge: synchronous makeFirstResponder in updateNSView/viewDidMoveToWindow, with becomeFirstResponder/resignFirstResponder writing the binding back so it always reflects the real first responder.
  • Accessibility + tooltip. accessibilityValue on the column and operator pickers, label + button/selected traits on suggestion-dropdown rows, and the status-bar Filters tooltip now reads the live shortcut instead of a hardcoded "⇧⌘F".

Design notes

  • I evaluated SwiftUI @FocusState for the value field (the audit suggested it) and chose a plain Binding<UUID?> bridge instead. @FocusState bound to a value with no SwiftUI .focused() target (an NSViewRepresentable text field is not one) gets reset to nil by SwiftUI's focus engine, which makes focus-into-field flaky. The manual first-responder bridge is the correct, reliable pattern for an AppKit NSTextField, and it still removes the async/nil-poke hacks.
  • No custom focus engine, no suppressed focus rings, documented APIs only (.defaultAction, .cancelAction/onExitCommand, .focusSection, NSResponder first responder).

Deferred (follow-up, on purpose)

  • Initial grid focus when a table opens (step 1 of the flow). This is coupled to SQLEditorCoordinator re-focusing the editor ~50ms after a tab appears; a guarded grid auto-focus would either be a no-op or lose a focus race, which would be a timing hack. It should land with the SQL-editor focus guard (tracked in the audit's focus-infrastructure notes).
  • Full AppKit key-view-loop wiring so Tab cycles value -> picker -> Apply without Full Keyboard Access. .focusSection() keeps focus from leaking out; deeper Tab ordering is a separate pass.

Out of scope here (separate surfaces in #1490): toolbar .focusable(false), overlay-editor focus ring, menu-picker Tab reachability without Full Keyboard Access.

Verify

Build the branch, then keyboard-only with a table open and focus in the grid:

  1. Open filters, type a value, Return -> filter applies and focus is back on a grid cell.
  2. Open filters, Escape -> panel closes, focus back on the grid.
  3. Apply, then arrow keys move cells immediately (no click).
  4. With Full Keyboard Access on, the Apply button and pickers are reachable and the focus ring shows.

Focus/responder behavior is not unit-testable here (consistent with the codebase); verification is manual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant