-
Notifications
You must be signed in to change notification settings - Fork 664
fix: toolbox-search cannot focus in Blockly v12 #2578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: toolbox-search cannot focus in Blockly v12 #2578
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
registerShortcut on line 90 also needs fixing, it seems to select the category but not focus the input box. |
|
Hi @kuangshen04 (and @xXBuilderBXx), and thanks for sending us this PR. The changes you propose all look fairly reasonable, but found myself confused about why they should be necessary. It eems the plugin works fine with Blockly v12, so, per the reasoning I outlined in my comment on #2577 I believe the underlying issue is actually an unintentional breaking change in Blockly, and should be fixed there. I'm going to leave this PR open for the time being, for two reasons:
My colleagues will do triage on Friday, so I'll aim to revisit this next week. |
|
@cpcallen I think we should merge this. The discussion about the Blockly change in #9082 shouldn't block fixing the broken plugin. Can you review (if necessary) and merge when ready? |
|
Thanks for sending this fix, and apologies that it has taken so long to get it reviewed and merged! |
The basics
The details
Resolves
Fixes #2577
The input in toolbox-search cannot focus because of the new FocusManager mechanism in Blockly v12.
Proposed Changes
Rewrite the focus logic using the interface provided by FocusManager.
Reason for Changes
Test Coverage
Documentation
Additional Information