-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix: Auto-close widget divs on lost focus #9216
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: Auto-close widget divs on lost focus #9216
Conversation
Also, fix a comment.
BenHenning
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-checked and fixed a few things, and added a couple more tests.
|
FYI: I'm going to update RaspberryPiFoundation/blockly-keyboard-experimentation#649 to include tests for the field editor to integrate test the widget changes, and then I'm going to try and verify all samples that might be affected by both widget and drop-down div changes. |
|
Did a quick test of this in MakeCode and it looks promising. Will test further tomorrow (don't block merge on my feedback). Fields that use both widget + dropdown only close widget but I think that's probably on us to sort as it's the widget that takes the focus. It seems to do no real harm and we can likely address in MakeCode. You might have similar scenarios (maybe in other direction?) in samples. |
Thanks for the quick check @microbit-matt-hillsdon! Drop-downs + widgets have been historically tricky with ephemeral focus, and this definitely adds to that. It may be the case you simply would turn off the auto behavior for those situations, I suspect, and handle it manually (which isn't ideal since catching all the cases where focus is lost is annoying and precisely why If there's an API change we can make to simplify this, please do let me know. |
The basics
The details
Resolves
Fixes RaspberryPiFoundation/blockly-keyboard-experimentation#563
Proposed Changes
This expands the functionality introduced in #9213 to also include widget divs.
Reason for Changes
MakeCode makes use of widget div in several field editors, so the issues described in RaspberryPiFoundation/blockly-keyboard-experimentation#563 aren't fully mitigated with #9213 alone.
This PR essentially adds the same support for auto-closing as drop-down divs now have, and enables this functionality by default.
Note the drop-down div change: it was missed in #9123 that the API change for drop-down div's
showfunction is actually API-breaking, so this updates that API to be properly backward compatible (and reverts one test change that makes use of it).The
FocusManagerchange actually corrects an implementation issue from #9123: not updating the tracked focus status before calling the callback can result in focus being inadvertently restored if the callback triggers returning focus due to a lost focus situation. This was wrong for drop-down divs, too, but it's harder to notice there because the dismissal of the drop-down div happens on a timer (which means there's sufficient time forFocusManager's state to correct prior to attempting to return from the ephemeral focus state).Demonstration of fixed behavior (since the inline number editor uses a widget div):
Screen.recording.2025-07-08.2.12.31.PM.webm
Test Coverage
New widget div tests have been added to verify the new parameter and auto-close functionality.
The
FocusManagertest was updated to account for the new, and correct, behavior around the internal tracked ephemeral focus state.Note that some
tabindexstate has been clarified and cleaned up in the test index page andFocusManager. It's fine (and preferable) for ephemeral-used elements to always be focusable rather than making them dynamically so (which avoids state bleed across test runs which was happening one of the new tests).RaspberryPiFoundation/blockly-keyboard-experimentation#649 includes additional tests for validating widget behaviors.
Documentation
No new documentation should be needed here--the API documentation changes should be sufficient.
One documentation update was made in
dropdowndiv.tsthat corrects the documentation parameter ordering.Additional Information
Nothing further to add.