Toolbox search fixes#658
Conversation
📝 WalkthroughWalkthroughBlock labels are cached in a new workspace map and built from block field text and input shapes during search indexing. Mobile search results display these cached labels, and the mobile overlay refactors open/collapse behavior with state-driven animations and improved input handling for Enter-key acceptance and focus-based close logic. ChangesMobile search UI and block label caching
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main/blocklyinit.js`:
- Around line 847-856: The collapseOverlay path currently only removes the
overlay, leaving mobile results mounted and searchCategory.matchBlocks stuck as
the no-op set by openOverlay(); update collapseOverlay to perform the same
mobile-results cleanup as the open/cancel path: if resultsPanel is mounted,
remove/unmount it and restore searchCategory.matchBlocks to the original
matching function (use whatever variable you saved earlier when openOverlay
replaced it), and clear any related state (e.g., blurTimeout) before removing
the overlay so the next search session works correctly; reference
functions/idents: collapseOverlay, openOverlay, resultsPanel,
searchCategory.matchBlocks, originalParent, searchInput, and blurTimeout.
In `@main/translation.js`:
- Around line 155-158: The search cache rebuild runs immediately by calling
workspace.flockSearchCategory.blockSearcher.indexBlocks() even when the earlier
import("../toolbox.js") branch is refreshing the toolbox; change the flow so you
await completion of the toolbox/language refresh before resetting
workspace.flockSearchIndexedBlocks and workspace.flockBlockLabelMap and before
calling blockSearcher.indexBlocks(), ensuring the rebuild uses the updated
workspace.options.languageTree; locate the import("../toolbox.js") branch and
the call to workspace.flockSearchCategory.blockSearcher.indexBlocks() and
chain/await the toolbox refresh promise (or make indexBlocks async and call it
after the await) so labels/results are built from the new language.
In `@style/blockly.css`:
- Around line 853-855: The current rule removes the only keyboard focus cue by
setting outline: none on the search input; add a focused-visible replacement for
.mobile-search-overlay input[type="search"] (e.g., .mobile-search-overlay
input[type="search"]:focus-visible) that restores a clear, accessible focus
indicator such as a visible outline or box-shadow (choose a high-contrast color,
set outline-width and outline-offset or a box-shadow) so keyboard users still
see focus while preserving the existing non-focused styling; keep outline: none
only if you rely on the new :focus-visible rule to provide the visible cue.
🪄 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: 06496495-249a-4c42-8fde-a95b5cb6df0d
📒 Files selected for processing (3)
main/blocklyinit.jsmain/translation.jsstyle/blockly.css
Summary
Fix a number of issues with the toolbox search on mobile:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements