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
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ export class BlockSvg
const menuOptions = this.generateContextMenu();

if (menuOptions && menuOptions.length) {
ContextMenu.show(e, menuOptions, this.RTL);
ContextMenu.show(e, menuOptions, this.RTL, this.workspace);
ContextMenu.setCurrentBlock(this);
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/comments/rendered_workspace_comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ export class RenderedWorkspaceComment
ContextMenuRegistry.ScopeType.COMMENT,
{comment: this},
);
contextMenu.show(e, menuOptions, this.workspace.RTL);
contextMenu.show(e, menuOptions, this.workspace.RTL, this.workspace);
}

/** Snap this comment to the nearest grid point. */
Expand Down
5 changes: 4 additions & 1 deletion core/contextmenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {Rect} from './utils/rect.js';
import * as serializationBlocks from './serialization/blocks.js';
import * as svgMath from './utils/svg_math.js';
import * as WidgetDiv from './widgetdiv.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import * as Xml from './xml.js';
import * as common from './common.js';

Expand Down Expand Up @@ -62,13 +63,15 @@ let menu_: Menu | null = null;
* @param e Mouse event.
* @param options Array of menu options.
* @param rtl True if RTL, false if LTR.
* @param workspace The workspace associated with the context menu, if any.
*/
export function show(
e: PointerEvent,
options: (ContextMenuOption | LegacyContextMenuOption)[],
rtl: boolean,
workspace?: WorkspaceSvg,
) {
WidgetDiv.show(dummyOwner, rtl, dispose);
WidgetDiv.show(dummyOwner, rtl, dispose, workspace);
if (!options.length) {
hide();
return;
Expand Down
13 changes: 9 additions & 4 deletions core/field_input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,12 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
if (!block) {
throw new UnattachedFieldError();
}
WidgetDiv.show(this, block.RTL, this.widgetDispose_.bind(this));
WidgetDiv.show(
this,
block.RTL,
this.widgetDispose_.bind(this),
this.workspace_,
);
this.htmlInput_ = this.widgetCreate_() as HTMLInputElement;
this.isBeingEdited_ = true;
this.valueWhenEditorWasOpened_ = this.value_;
Expand Down Expand Up @@ -546,17 +551,17 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
*/
protected onHtmlInputKeyDown_(e: KeyboardEvent) {
if (e.key === 'Enter') {
WidgetDiv.hide();
WidgetDiv.hideIfOwner(this);
dropDownDiv.hideWithoutAnimation();
} else if (e.key === 'Escape') {
this.setValue(
this.htmlInput_!.getAttribute('data-untyped-default-value'),
false,
);
WidgetDiv.hide();
WidgetDiv.hideIfOwner(this);
dropDownDiv.hideWithoutAnimation();
} else if (e.key === 'Tab') {
WidgetDiv.hide();
WidgetDiv.hideIfOwner(this);
dropDownDiv.hideWithoutAnimation();
(this.sourceBlock_ as BlockSvg).tab(this, !e.shiftKey);
e.preventDefault();
Expand Down
2 changes: 1 addition & 1 deletion core/flyout_horizontal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ export class HorizontalFlyout extends Flyout {
this.workspace_.scrollbar?.setX(pos);
// When the flyout moves from a wheel event, hide WidgetDiv and
// dropDownDiv.
WidgetDiv.hide();
WidgetDiv.hideIfOwnerIsInWorkspace(this.workspace_);
dropDownDiv.hideWithoutAnimation();
}
// Don't scroll the page.
Expand Down
2 changes: 1 addition & 1 deletion core/flyout_vertical.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export class VerticalFlyout extends Flyout {
this.workspace_.scrollbar?.setY(pos);
// When the flyout moves from a wheel event, hide WidgetDiv and
// dropDownDiv.
WidgetDiv.hide();
WidgetDiv.hideIfOwnerIsInWorkspace(this.workspace_);
dropDownDiv.hideWithoutAnimation();
}
// Don't scroll the page.
Expand Down
39 changes: 34 additions & 5 deletions core/widgetdiv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@

import * as common from './common.js';
import * as dom from './utils/dom.js';
import type {Field} from './field.js';
import {Field} from './field.js';
import type {Rect} from './utils/rect.js';
import type {Size} from './utils/size.js';
import type {WorkspaceSvg} from './workspace_svg.js';

/** The object currently using this container. */
let owner: unknown = null;

/** The workspace associated with the owner currently using this container. */
let ownerWorkspace: WorkspaceSvg | null = null;

/** Optional cleanup function set by whichever object uses the widget. */
let dispose: (() => void) | null = null;

Expand Down Expand Up @@ -76,18 +79,31 @@ export function createDom() {
* @param rtl Right-to-left (true) or left-to-right (false).
* @param newDispose Optional cleanup function to be run when the widget is
* closed.
* @param workspace The workspace associated with the widget owner.
*/
export function show(newOwner: unknown, rtl: boolean, newDispose: () => void) {
export function show(
newOwner: unknown,
rtl: boolean,
newDispose: () => void,
workspace?: WorkspaceSvg | null,
) {
hide();
owner = newOwner;
dispose = newDispose;
const div = containerDiv;
if (!div) return;
div.style.direction = rtl ? 'rtl' : 'ltr';
div.style.display = 'block';
const mainWorkspace = common.getMainWorkspace() as WorkspaceSvg;
rendererClassName = mainWorkspace.getRenderer().getClassName();
themeClassName = mainWorkspace.getTheme().getClassName();
if (!workspace && newOwner instanceof Field) {
// For backward compatibility with plugin fields that do not provide a
// workspace to this function, attempt to derive it from the field.
workspace = (newOwner as Field).getSourceBlock()?.workspace as WorkspaceSvg;
}
ownerWorkspace = workspace ?? null;
const rendererWorkspace =
workspace ?? (common.getMainWorkspace() as WorkspaceSvg);
rendererClassName = rendererWorkspace.getRenderer().getClassName();
themeClassName = rendererWorkspace.getTheme().getClassName();
if (rendererClassName) {
dom.addClass(div, rendererClassName);
}
Expand Down Expand Up @@ -145,6 +161,19 @@ export function hideIfOwner(oldOwner: unknown) {
hide();
}
}

/**
* Destroy the widget and hide the div if it is being used by an object in the
* specified workspace.
*
* @param oldOwnerWorkspace The workspace that was using this container.
*/
export function hideIfOwnerIsInWorkspace(oldOwnerWorkspace: WorkspaceSvg) {
if (ownerWorkspace === oldOwnerWorkspace) {
johnnesky marked this conversation as resolved.
Show resolved Hide resolved
hide();
}
}

/**
* Set the widget div's position and height. This function does nothing clever:
* it will not ensure that your widget div ends up in the visible window.
Expand Down
4 changes: 2 additions & 2 deletions core/workspace_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1686,7 +1686,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
this.configureContextMenu(menuOptions, e);
}

ContextMenu.show(e, menuOptions, this.RTL);
ContextMenu.show(e, menuOptions, this.RTL, this);
}

/**
Expand Down Expand Up @@ -2376,7 +2376,7 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg {
*/
hideChaff(onlyClosePopups = false) {
Tooltip.hide();
WidgetDiv.hide();
WidgetDiv.hideIfOwnerIsInWorkspace(this);
dropDownDiv.hideWithoutAnimation();

this.hideComponents(onlyClosePopups);
Expand Down
Loading