Skip to content
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: Only hide WidgetDiv if it is associated with the affected workspace. #8150

Merged
merged 5 commits into from
May 20, 2024

Conversation

johnnesky
Copy link
Member

@johnnesky johnnesky commented May 17, 2024

The basics

The details

Resolves

Fixes google/blockly-samples#2131

Proposed Changes

This associates a workspace with the current WidgetDiv, so that when a workspace needs to clean up its chaff, it can detect whether the current WidgetDiv is associated with it and only clean it up if it's affected.

I added a workspace parameter to WidgetDiv.show() and ContextMenu.show(), and a hideIfOwnerIsInWorkspace method to WorkspaceDiv. I also updated some calls from WidgetDiv.hide() to WidgetDiv.hideIfOwner(this).

Reason for Changes

In google/blockly-samples#2131 the minimap plugin calls this.minimapWorkspace.zoomToFit() in response to changes, which calls WorkspaceSvg.hideChaff() which calls the static class method WidgetDiv.hide() which unfocuses inputs in the main workspace because the input widget is shared between the main workspace and the minimap workspace. Intermediate change events do not trigger the issue, so most text input fields work fine, but standard BlockChange events trigger the issue and editing procedure names and parameters will update caller blocks in the same workspace which fires BlockChange events which will trigger the issue and immediately unfocus the input field after typing a single character.

Test Coverage

I manually tested with the minimap plugin to make sure that procedures and procedure parameters could be renamed without immediately losing focus (and that basic widget hiding still seems to work). All existing unit tests pass.

Documentation

N/A

@johnnesky johnnesky requested a review from a team as a code owner May 17, 2024 19:20
@johnnesky johnnesky requested a review from NeilFraser May 17, 2024 19:20
@github-actions github-actions bot added the PR: fix Fixes a bug label May 17, 2024
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 17, 2024
Copy link
Contributor

@NeilFraser NeilFraser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to both me and Christopher. Up to you whether you want to merge it now, or wait until after V11.

core/widgetdiv.ts Outdated Show resolved Hide resolved
@johnnesky johnnesky merged commit 9b2ab79 into google:develop May 20, 2024
6 checks passed
@johnnesky johnnesky deleted the nesky_widget_workspace branch May 20, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rename of function fails with minimap and a call function
3 participants