Repository navigation
Restyle the Keys panel, make it focusable, and bind it to F7 - #1169
Conversation
The Keys panel now matches Harlequin's other panels (rounded border with its name, app background and scrollbars, keys in the footer's colors), sits in the main row above the footer, and can be focused by click or Tab and scrolled from the keyboard. While it has focus it keeps listing the keys of the widget focused before it. A new `toggle_keys_panel` action, bound to F7 in the default keymap, shows and hides it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnaMXxkgCL76UXYyA1SfQE
tconbeer
left a comment
There was a problem hiding this comment.
Review: #1169, Keys panel restyle, focus and F7
What I looked at: correctness and edge cases of the new keys_panel.py overrides and of action_show_help_panel / action_toggle_keys_panel; reuse and whether this is the simplest way to build it; comments and changelog against AGENTS.md. I skipped a security pass because the diff touches no I/O, config or adapter code.
Findings: 2 inline comments, both non-blocking.
- Low: the command-palette path is untested. It is the reason
action_show_help_panelis overridden, and it only works because Textual dismisses the palette before it runs the command (app.pyL1336–1344). - Nit: the CHANGELOG entry is three sentences; the rule asks for one or two.
Checked with no problems found
-
Both overrides are needed, and the test catches the loss of either one. I disabled each in a scratch script, focused the list and pressed PgDn, then checked whether
Format Querywas still listed:- baseline:
True rows 61 scroll_y 33 KeysList.recompose→KeyPanel.recompose:False rows 26 scroll_y 0.0KeysTable.render→BindingsTable.render:False rows 26 scroll_y 33
So
test_toggle_keys_panel's"Format Query" in listed_descriptions()assertion fails if either override is removed. That also limits the risk of importing the privatetextual.widgets._key_panel: if a Textual bump changes howKeyPanelrefreshes, the test will fail rather than the panel silently going stale. - baseline:
-
Dropping
-textual-systemdoes what the PR says.css/model.pystops the universal*rule from matching nodes with that class, andScreen.get_focusable_widget_atrefuses to focus them on click. Tab order with the panel open:ResultsViewer → KeysList → DatabaseTree → editor. -
Pressing F7 while the list is focused removes the panel, and focus lands on
ResultsViewer. F10 (full screen) with the list focused goes up the ancestors and returns without error. -
Layout: at 120×36 the panel is 39 columns wide, and at 80×24 it is 30 wide (its min-width), which leaves the editor column 38 wide.
-
f7is not bound anywhere else insrc/,textual_textareaortextual_fastdatatable. -
The
test_keys_appsnapshot churn is only the new "Toggle Keys Panel f7" row, plus the rows below it shifting down. I compared the text extracted from the SVGs onorigin/mainand on this branch. -
uv run mypy,uv run ruff check .anduv run lint-importsall pass.pytest -m "not online" tests/functional_tests: 199 passed, 3 skipped. -
CI: the one red job (MacOs-latest 3.11) is
test_hsql_serve.py::test_a_client_that_gives_up_while_it_waits_runs_nothingtiming out, and this PR does not touch hsql.
Design questions, not findings
- While the list is focused it keeps showing the previous widget's keys, such as
f4 Format Query. Those keys do nothing until focus goes back to that widget. That is the intended behavior per the PR body; I'm raising it only so the choice is deliberate. - In full-screen mode (F10) the panel stays open next to the full-screen widget and fills the row the hidden footer used. That seems reasonable, since the user opened it.
Not checked
- I did not run the py12 snapshot pass, and I did not run the suite on Windows or macOS. I relied on CI for those.
- I did not look at the snapshots with an SVG renderer. I only compared their text, so colors (
$secondarykeys,$primarydescriptions, the scrollbar) were not checked visually. - I did not check ANSI or light themes, or what happens when the theme changes while the list is focused (the cached
Tablekeeps the old theme's styles until focus leaves). - I did not check the palette "Keys" command opened from a modal screen (the
else: self.screen.mount(...)branch). F7 does nothing onHistoryScreenandHelpScreen. - I did not review the harlequin-web docs, since no PR has been opened there yet.
Generated by Claude Code
Keep a reference to the main row from compose() so the Keys panel mounts into it without narrowing data_catalog.parent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnaMXxkgCL76UXYyA1SfQE
The key comes from the active keymap; with none bound, the tip points to the command palette instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnaMXxkgCL76UXYyA1SfQE
Closes #753
What are the key elements of this solution?
harlequin.components.keys_panelwith Harlequin subclasses of Textual'sHelpPanelandKeyPanel:KeysPanel(HelpPanel)hasBORDER_TITLE = "Keys"and takes the same rounded border/focus colors as the Data Catalog, Query Editor and Results Viewer (app.tcss). Keys are$secondary(green) and descriptions$primary, matching the footer.KeysList(KeyPanel, can_focus=True)can be focused by clicking it or with Tab, and scrolled with the arrow keys or PgUp/PgDn.-textual-systemclass. With that class, the app's* { scrollbar-* }rules don't match (that's where the pink scrollbar came from), and the screen won't focus the widget on click.KeysList.recompose()skips while it is focused, andKeysTable.render()reuses its last table while its list is focused, so the panel keeps showing the keys of the widget you came from. Both checkscreen.focusedrather thanhas_focus, which is set a render later.Harlequin.action_show_help_panelis overridden, so the command palette's "Keys" command opens the new panel too. On the main screen it mounts into the main row (self.main_row) as a column above the footer, not as a screen split over it.toggle_keys_panelaction, bound toF7in the default (vscode) keymap, to make the panel easier to find.Why did you design your solution this way? Did you assess any alternatives? Are there tradeoffs?
Subclassing
HelpPanel, rather than writing a new widget, keeps the palette's show/hide command (screen.query("HelpPanel")) working. Freezing the table while focused is a small override of Textual internals (textual.widgets._key_panel.BindingsTable). An upstreamcan_focusoption onKeyPanelwould be cleaner, but Textual isn't one of ours.F7was the free function key between the focus keys (F2/F5/F6) and History (F8); F11 is usually taken by the terminal emulator. It isn't in the footer, because adding it there would change every app snapshot; the Help screen names it instead.Does this PR require a change to Harlequin's docs?
F7: toggle_keys_panel(show/hide the Keys panel), and the keymap docs' action list gainstoggle_keys_panel.Did you add or update tests for this change?
test_layout.py::test_toggle_keys_panelcovers F7 show/hide, the command palette's Keys command, mounting in the main row, focusing by click, and the keys staying those of the Query Editor after focus and scrolling, with two new snapshots.test_help_screen.pychecks the Help screen's key line, including the version shown when nothing is bound. Thetest_keys_appand Help screen snapshots are regenerated.Please complete the following checklist:
CHANGELOG.md, under the[Unreleased]section heading. That entry references the issue closed by this PR.🤖 Generated with Claude Code
https://claude.ai/code/session_01VnaMXxkgCL76UXYyA1SfQE