Skip to content

Clarify Tab files/filter focus copy#121

Merged
benvinegar merged 1 commit intomodem-dev:mainfrom
ferologics:fix/tab-focus-toggle-copy
Mar 29, 2026
Merged

Clarify Tab files/filter focus copy#121
benvinegar merged 1 commit intomodem-dev:mainfrom
ferologics:fix/tab-focus-toggle-copy

Conversation

@ferologics
Copy link
Copy Markdown
Contributor

@ferologics ferologics commented Mar 25, 2026

https://pi.dev/session/#8a410a9ef0ac846d8452317507dd6a14

Summary

  • rename the File menu Tab action to Toggle files/filter focus
  • route that menu entry through the same toggle helper used by the Tab keybinding
  • align the keyboard help copy and UI tests with the actual behavior

Testing

  • bun run typecheck
  • bun run lint
  • bun run format:check
  • bun test test/ui-lib.test.ts test/ui-components.test.tsx test/app-interactions.test.tsx
  • bun test (all suites passed locally, then Bun 1.3.9 segfaulted after completion)

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This PR is a focused copy-and-behaviour-consistency cleanup. It introduces a single toggleFocusArea useCallback in AppShell that replaces two previously divergent Tab-key implementations (one set focus unconditionally to "files", the other toggled), routes the File-menu entry through that same helper, and aligns the HelpDialog copy and all test assertions with the new wording.

Key changes:

  • src/ui/App.tsx: toggleFocusArea extracted as a useCallback; both Tab-key handlers and the buildAppMenus call now use it. The useMemo dependency array is correctly updated.
  • src/ui/lib/appMenus.ts: focusFiles option removed; toggleFocusArea added; menu label updated to "Toggle files/filter focus".
  • src/ui/components/chrome/HelpDialog.tsx: Tab key description updated to "toggle files/filter focus".
  • Tests (app-interactions, ui-components, ui-lib): String assertions updated and a new toMatchObject assertion added to verify the Tab hint on the menu entry.

The bug fix here is subtle but meaningful: previously, pressing Tab while the filter was focused correctly toggled, but pressing Tab while the files pane was focused (or invoking the menu action) would always switch to "files" regardless of the current state — now both paths behave identically via the shared toggle.

Confidence Score: 5/5

  • Safe to merge — the changes are a clean refactor that fixes an existing behavioural inconsistency and updates all affected tests.
  • All six changed files are in tight scope: one bug fix (Tab from files-pane now toggles instead of no-oping), one label rename, and the corresponding test updates. No new logic is introduced beyond extracting the shared helper. The useMemo dependency array is correctly updated, no type errors are possible given the interface change is fully propagated, and the tests explicitly assert the new hint and label values.
  • No files require special attention.

Important Files Changed

Filename Overview
src/ui/App.tsx Extracts toggleFocusArea into a useCallback and replaces two separate inline implementations (one that set focus unconditionally to "files", one that toggled) with the shared helper. The useMemo dependency array is updated correctly.
src/ui/lib/appMenus.ts Removes the now-unused focusFiles option from BuildAppMenusOptions and replaces it with toggleFocusArea. The File menu entry label is updated from "Focus files" to "Toggle files/filter focus".
src/ui/components/chrome/HelpDialog.tsx One-line copy update: Tab key description changed from "swap files / filter focus" to "toggle files/filter focus" to match the new menu label and actual behaviour.
test/app-interactions.test.tsx All string assertions updated from "Focus files" to "Toggle files/filter focus" to match the renamed menu item.
test/ui-components.test.tsx Adds an assertion that the HelpDialog renders "files/filter focus" in the help dialog frame, improving test coverage for the updated copy.
test/ui-lib.test.ts Removes focusFiles from the test fixture, adds toggleFocusArea, updates the label assertion, and adds a new toMatchObject assertion to verify the Tab hint is present on the first file menu entry.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Key event: Tab] --> B{focusArea}
    B -->|"files"| C[setFocusArea → filter]
    B -->|"filter"| D[setFocusArea → files]
    C --> E[toggleFocusArea helper]
    D --> E

    F[Menu: Toggle files/filter focus] --> E

    G[Key event: /] --> H[setFocusArea → filter directly]
    I[Menu: Focus filter] --> H

    style E fill:#4a90d9,color:#fff
    style H fill:#888,color:#fff
Loading

Reviews (1): Last reviewed commit: "clarify Tab files/filter focus copy" | Re-trigger Greptile

@benvinegar
Copy link
Copy Markdown
Member

Thanks @ferologics!

@ferologics ferologics force-pushed the fix/tab-focus-toggle-copy branch from 9bc1e14 to 64a4817 Compare March 26, 2026 14:31
@benvinegar benvinegar merged commit f354d83 into modem-dev:main Mar 29, 2026
3 checks passed
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.

2 participants