Toolbox keyboard: persistent category prefix typing and improved focus handling#540
Conversation
Deploying flockxr with
|
| Latest commit: |
864ebe4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://acf3e945.flockxr.pages.dev |
| Branch Preview URL: | https://codex-enhance-keyboard-navig.flockxr.pages.dev |
📝 WalkthroughWalkthroughAdds keyboard-driven category prefix navigation to the Blockly toolbox: accumulates normalized typed characters, matches the first category whose label starts with the prefix, expands/selects it, and resets the prefix on search focus, focus changes, navigation keys, clicks, Escape, Enter/Space toggles, and related interactions. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Toolbox as Toolbox UI
participant Workspace as Blockly Workspace / FocusManager
participant Flyout as Flyout
User->>Toolbox: press printable key / Backspace / Enter / Arrow / Escape
Toolbox->>Toolbox: normalize input, update categoryTypePrefix
Toolbox->>Workspace: request selectable category items
Workspace-->>Toolbox: list of toolbox categories
Toolbox->>Toolbox: find first category matching prefix
Toolbox->>Toolbox: expand/select & focus category node
Toolbox->>Workspace: update focusTree / maintain toolbox focus
alt Flyout visible + ArrowRight
Toolbox->>Flyout: preserve flyout navigation (reset prefix first)
end
note right of Toolbox: prefix reset on search focus, focus changes, clicks, Escape, arrows, Enter/Space
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying flockdev with
|
| Latest commit: |
864ebe4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8082893f.flockdev.pages.dev |
| Branch Preview URL: | https://codex-enhance-keyboard-navig.flockdev.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
main/blocklyinit.js (1)
1-1:⚠️ Potential issue | 🟡 MinorPrettier CI is currently failing for this file.
Please run Prettier on
main/blocklyinit.jsto clear the pipeline warning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/blocklyinit.js` at line 1, Prettier is flagging formatting in main/blocklyinit.js; run Prettier (or your repo's format script) against that file and commit the resulting changes so the import line and any other formatting match the project's Prettier rules; specifically format main/blocklyinit.js (the file that contains the Blockly import) and stage the updated file before pushing to clear the CI warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@main/blocklyinit.js`:
- Around line 717-719: isToolboxContext currently treats the flyout as part of
the toolbox (it matches ".blocklyFlyout"), which prevents the prefix reset path
from running when focus moves from toolbox categories into the flyout; change
isToolboxContext to stop matching ".blocklyFlyout" (only match
".blocklyToolboxDiv, .blocklyToolbox") and, if you need to detect flyout entries
specifically, add a separate check for ".blocklyFlyoutEntry". Then update the
prefix-reset logic that uses isToolboxContext (the code that clears the prefix
state) so that entering the flyout triggers the reset (i.e., treat blocklyFlyout
as a non-toolbox context or explicitly reset the prefix when focus lands on
".blocklyFlyoutEntry"). Ensure you modify the isToolboxContext declaration and
the prefix-reset condition (the code that clears the prefix variable)
accordingly.
- Around line 930-940: The Backspace handler currently exits early without
consuming the event when categoryTypePrefix is cleared to empty; update the
handler so that once prefix editing has started (i.e., when categoryTypePrefix
was non-empty before the slice), the event is always consumed: call
e.preventDefault(), e.stopPropagation(), and e.stopImmediatePropagation() before
any return that occurs after slicing the prefix (even when categoryTypePrefix
becomes empty), and keep the existing applyPrefixMatch(categoryTypePrefix) logic
to decide additional handling; reference categoryTypePrefix and applyPrefixMatch
in the anonymous Backspace event branch.
In `@tests/playwright/blocks.spec.js`:
- Around line 235-236: Replace the fixed sleeps by polling/wait helpers: instead
of awaiting new Promise/setTimeout before calling keydown("d") (and at lines
246-247), wait for the DOM state you expect — e.g., use Playwright's
page.waitForFunction or locator.waitFor()/expect(...).toHaveClass to assert the
category element has the "selected" state and use locator.waitFor() or
expect(locator).toBeFocused / page.waitForFunction(() => document.activeElement
=== searchInput) to assert the search input is focused — then call keydown("d");
update tests/playwright/blocks.spec.js to remove the setTimeouts and replace
them with these condition-based waits around the selected category and focused
search input checks.
---
Outside diff comments:
In `@main/blocklyinit.js`:
- Line 1: Prettier is flagging formatting in main/blocklyinit.js; run Prettier
(or your repo's format script) against that file and commit the resulting
changes so the import line and any other formatting match the project's Prettier
rules; specifically format main/blocklyinit.js (the file that contains the
Blockly import) and stage the updated file before pushing to clear the CI
warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7ac33df-e1ab-43ef-84bc-785b57a31e41
📒 Files selected for processing (2)
main/blocklyinit.jstests/playwright/blocks.spec.js
Motivation
Description
categoryTypePrefixstate with helpersresetCategoryTypePrefix,normalizeLabel,getSelectableCategories, andapplyPrefixMatchto support persistent prefix typing and diacritic-insensitive matching.Backspaceto delete last character of the prefix,Escape/arrow navigation to reset the prefix, and fallback to single-key matches when multi-key prefix fails.focusin,focusout, and toolboxclick) to reset prefix state appropriately, and ensure prefix is reset when focusing toolbox search via theCtrl+Fhandler or when switching into flyout/interacting with selected items.Testing
npx playwright testand observed successful completion of the suite.supports persistent toolbox category prefix typing without timeoutintests/playwright/blocks.spec.js, which passed and validated prefix typing, backspace behavior, and toolbox search focus behavior.Issue
Addresses #359
AI Use
Developed in an interactive session with Codex GPT5.4 with a tight specification for UX and interactive rework of proposed approach.
Codex Task
Summary by CodeRabbit
New Features
Bug Fixes
Tests