fix: focus the workspace itself on first focus#9974
Merged
Conversation
mikeharv
approved these changes
Jun 10, 2026
mikeharv
reviewed
Jun 11, 2026
mikeharv
approved these changes
Jun 11, 2026
mikeharv
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the detailed notes. Pulled and manually tested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The basics
The details
Resolves
Fixes #9969
Proposed Changes
Reason for Changes
See issue description for why the first change.
As to why the change to
getFocusableTreeinworkspace_svg:At an intermediate step in this PR, I had mutator workspaces also returning the workspace itself for initial focus rather than the first block. This revealed problems with
getFocusableTreereturning the parent workspace. It caused the escape shortcut to behave incorrectly, T to open the main workspace toolbox instead of the flyout toolbox, etc. Basically there was almost no way previously to actually put the focus on the mutator workspace itself (since mutator workspaces are never empty) and so the condition whereworkspace.getFocusableTreewas actually hitting the mutator case never occurred. blocks on the mutator workspace did not participate in the same lie where they said that their tree was the main workspace tree. they correctly reported the mutator workspace tree.I audited the cases where the mutator workspace being focused and
getFocusableTreeis called. Here are the results:All of these are actually the wrong behavior. It just never mattered before since the mutator workspace was never focused.
Then after I fixed these problems, I realized it actually made more sense for mutator workspaces to continue focusing their first block, unlike the main workspace. So this again goes back to not mattering since it is very difficult to actually focus the mutator workspace again. But, this behavior is more correct, and if we change our mind in the future about focusing the first block for mutator workspaces, we can easily change that without hitting these same problems.
Test Coverage
added unit tests, fixed existing ones (that were relying on implementation details to pass). manually tested main workspaces, flyout workspaces, and mutator workspaces.
Documentation
n/a
Additional Information