Focus selected Blockly block on Ctrl+E before activating keyboard navigation#576
Focus selected Blockly block on Ctrl+E before activating keyboard navigation#576tracygardner wants to merge 4 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds a new utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 flockxr with
|
| Latest commit: |
853e37a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e0fdf0db.flockxr.pages.dev |
| Branch Preview URL: | https://codex-fix-focus-outline-for.flockxr.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/blocklyutil.js`:
- Around line 75-76: selectedBlock returned from Blockly.common.getSelected()
can be ICopyable or null, so accessing selectedBlock.workspace is unsafe; update
the check in the code that uses selectedBlock (the variable and the call to
Blockly.common.getSelected()) to first narrow the type to a BlockSvg (e.g.,
verify the object has block-specific shape or instanceof BlockSvg or check for
the presence of .workspace and other block-only properties) before comparing its
.workspace to workspace, and return false if the selected object is not a block.
- Around line 72-89: The function focusHighlightedBlock currently returns true
when a selected block exists even if no DOM element was focused; update it so it
only returns true when a real DOM focus was applied: locate
focusHighlightedBlock and compute focusableElement via
selectedBlock.getFocusableElement?.() || selectedBlock.getSvgRoot?.() before
returning, call focusableElement?.focus?.({ preventScroll: true }) and only
return true if focusableElement was non-null (and focus was invoked); keep
existing calls to Blockly.keyboardNavigationController?.setIsActive,
Blockly.getFocusManager()?.focusNode, selectedBlock.select, and
workspace.getCursor()?.setCurNode, but change the final return to false when no
focusableElement was found so callers (e.g., focusTree(workspace)) can run
fallback logic.
🪄 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: e77b540b-345b-4d5e-ac97-314c1f7ca5c6
📒 Files selected for processing (2)
main/main.jsui/blocklyutil.js
1 participant
Motivation
Ctrl+E, avoiding unnecessary activation of the global keyboard navigation and preventing loss of cursor position.Description
focusHighlightedBlock(workspace = Blockly.getMainWorkspace())toui/blocklyutil.jswhich attempts to focus and select the currently selected/highlighted block, moves the workspace cursor to it, and returns a boolean indicating success.main/main.jsto importfocusHighlightedBlockand call it in theCtrl+Ekeyboard handler, falling back to the previousBlockly.keyboardNavigationControllerandfocusTreebehavior only if no highlighted block could be focused.preventScrollto avoid unwanted scrolling when moving focus.Testing
npm test(frontend unit tests) which completed successfully.npm run buildto verify the bundle and compilation, which succeeded.Codex Task
Summary by CodeRabbit