From 3c7add57eed7e2564a69f650d8381db3d1f2eadd Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Fri, 18 Jul 2025 14:27:49 -0700 Subject: [PATCH] fix: Make non-autoclosing flyouts stay open. (#9245) * chore: Add tests for toolbox/flyout/focus autoclose behavior. * fix: Don't force-close non-autoclosing flyouts. --- core/toolbox/toolbox.ts | 10 ++------ core/workspace_svg.ts | 4 +-- tests/mocha/toolbox_test.js | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index 4979fdfa4..f34034d33 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -22,10 +22,7 @@ import '../events/events_toolbox_item_select.js'; import {EventType} from '../events/type.js'; import * as eventUtils from '../events/utils.js'; import {getFocusManager} from '../focus_manager.js'; -import { - isAutoHideable, - type IAutoHideable, -} from '../interfaces/i_autohideable.js'; +import {type IAutoHideable} from '../interfaces/i_autohideable.js'; import type {ICollapsibleToolboxItem} from '../interfaces/i_collapsible_toolbox_item.js'; import {isDeletable} from '../interfaces/i_deletable.js'; import type {IDraggable} from '../interfaces/i_draggable.js'; @@ -1150,10 +1147,7 @@ export class Toolbox // If navigating to anything other than the toolbox's flyout then clear the // selection so that the toolbox's flyout can automatically close. if (!nextTree || nextTree !== this.flyout?.getWorkspace()) { - this.clearSelection(); - if (this.flyout && isAutoHideable(this.flyout)) { - this.flyout.autoHide(false); - } + this.autoHide(false); } } } diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index b666dc97a..6c6b59301 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -2908,11 +2908,9 @@ export class WorkspaceSvg // Only hide the flyout if the flyout's workspace is losing focus and that // focus isn't returning to the flyout itself, the toolbox, or ephemeral. if (getFocusManager().ephemeralFocusTaken()) return; - const flyout = this.targetWorkspace.getFlyout(); const toolbox = this.targetWorkspace.getToolbox(); if (toolbox && nextTree === toolbox) return; - if (toolbox) toolbox.clearSelection(); - if (flyout && isAutoHideable(flyout)) flyout.autoHide(false); + if (isAutoHideable(toolbox)) toolbox.autoHide(false); } } diff --git a/tests/mocha/toolbox_test.js b/tests/mocha/toolbox_test.js index f32319c67..4e92cd28f 100644 --- a/tests/mocha/toolbox_test.js +++ b/tests/mocha/toolbox_test.js @@ -183,6 +183,57 @@ suite('Toolbox', function () { }); }); + suite('focus management', function () { + setup(function () { + this.toolbox = getInjectedToolbox(); + }); + teardown(function () { + this.toolbox.dispose(); + }); + + test('Losing focus hides autoclosing flyout', function () { + // Focus the toolbox and select a category to open the flyout. + const target = this.toolbox.HtmlDiv.querySelector( + '.blocklyToolboxCategory', + ); + Blockly.getFocusManager().focusNode(this.toolbox); + target.dispatchEvent( + new PointerEvent('pointerdown', { + target, + bubbles: true, + }), + ); + assert.isTrue(this.toolbox.getFlyout().isVisible()); + + // Focus the workspace to trigger the toolbox to close the flyout. + Blockly.getFocusManager().focusNode(this.toolbox.getWorkspace()); + assert.isFalse(this.toolbox.getFlyout().isVisible()); + }); + + test('Losing focus does not hide non-autoclosing flyout', function () { + // Make the toolbox's flyout non-autoclosing. + this.toolbox.getFlyout().setAutoClose(false); + + // Focus the toolbox and select a category to open the flyout. + const target = this.toolbox.HtmlDiv.querySelector( + '.blocklyToolboxCategory', + ); + Blockly.getFocusManager().focusNode(this.toolbox); + target.dispatchEvent( + new PointerEvent('pointerdown', { + target, + bubbles: true, + }), + ); + assert.isTrue(this.toolbox.getFlyout().isVisible()); + + // Focus the workspace; this should *not* trigger the toolbox to close the + // flyout, which should remain visible. + Blockly.getFocusManager().focusNode(this.toolbox.getWorkspace()); + assert.isTrue(this.toolbox.getFlyout().isVisible()); + }); + }); + suite('onClick_', function () { setup(function () { this.toolbox = getInjectedToolbox();