Skip to content

Commit

Permalink
fix: Only hide WidgetDiv if it is associated with the affected worksp…
Browse files Browse the repository at this point in the history
…ace. (#8150)

* Associate a workspace with WidgetDiv.

* Minor fixes after merging.

* Hide widget if owner is in an unknown workspace.
  • Loading branch information
johnnesky authored May 20, 2024
1 parent 3ac2fb9 commit 9b2ab79
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 16 deletions.
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 @@ -274,7 +274,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, or if it is used by an unknown workspace.
*
* @param oldOwnerWorkspace The workspace that was using this container.
*/
export function hideIfOwnerIsInWorkspace(oldOwnerWorkspace: WorkspaceSvg) {
if (ownerWorkspace === null || ownerWorkspace === oldOwnerWorkspace) {
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

0 comments on commit 9b2ab79

Please sign in to comment.