Skip to content

Commit

Permalink
fix: Fix bug that caused blocks dragged from non-primary flyouts to b…
Browse files Browse the repository at this point in the history
…e misplaced. (#8753)

* fix: Fix bug that caused blocks dragged from non-primary flyouts to be misplaced.

* chore: Fix docstring.
  • Loading branch information
gonfunko authored Feb 4, 2025
1 parent 343c2f5 commit e6e57dd
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 51 deletions.
58 changes: 27 additions & 31 deletions core/block_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const BLOCK_TYPE = 'block';
export class BlockFlyoutInflater implements IFlyoutInflater {
protected permanentlyDisabledBlocks = new Set<BlockSvg>();
protected listeners = new Map<string, browserEvents.Data[]>();
protected flyoutWorkspace?: WorkspaceSvg;
protected flyout?: IFlyout;
private capacityWrapper: (event: AbstractEvent) => void;

Expand All @@ -50,13 +49,12 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
* Inflates a flyout block from the given state and adds it to the flyout.
*
* @param state A JSON representation of a flyout block.
* @param flyoutWorkspace The workspace to create the block on.
* @param flyout The flyout to create the block on.
* @returns A newly created block.
*/
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
this.setFlyoutWorkspace(flyoutWorkspace);
this.flyout = flyoutWorkspace.targetWorkspace?.getFlyout() ?? undefined;
const block = this.createBlock(state as BlockInfo, flyoutWorkspace);
load(state: object, flyout: IFlyout): FlyoutItem {
this.setFlyout(flyout);
const block = this.createBlock(state as BlockInfo, flyout.getWorkspace());

if (!block.isEnabled()) {
// Record blocks that were initially disabled.
Expand Down Expand Up @@ -157,22 +155,18 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
}

/**
* Updates this inflater's flyout workspace.
* Updates this inflater's flyout.
*
* @param workspace The workspace of the flyout that owns this inflater.
* @param flyout The flyout that owns this inflater.
*/
protected setFlyoutWorkspace(workspace: WorkspaceSvg) {
if (this.flyoutWorkspace === workspace) return;
protected setFlyout(flyout: IFlyout) {
if (this.flyout === flyout) return;

if (this.flyoutWorkspace) {
this.flyoutWorkspace.targetWorkspace?.removeChangeListener(
this.capacityWrapper,
);
if (this.flyout) {
this.flyout.targetWorkspace?.removeChangeListener(this.capacityWrapper);
}
this.flyoutWorkspace = workspace;
this.flyoutWorkspace.targetWorkspace?.addChangeListener(
this.capacityWrapper,
);
this.flyout = flyout;
this.flyout.targetWorkspace?.addChangeListener(this.capacityWrapper);
}

/**
Expand All @@ -182,7 +176,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
* @param block The block to update the enabled/disabled state of.
*/
private updateStateBasedOnCapacity(block: BlockSvg) {
const enable = this.flyoutWorkspace?.targetWorkspace?.isCapacityAvailable(
const enable = this.flyout?.targetWorkspace?.isCapacityAvailable(
common.getBlockTypeCounts(block),
);
let currentBlock: BlockSvg | null = block;
Expand All @@ -209,26 +203,25 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
'pointerdown',
block,
(e: PointerEvent) => {
const gesture = this.flyoutWorkspace?.targetWorkspace?.getGesture(e);
const flyout = this.flyoutWorkspace?.targetWorkspace?.getFlyout();
if (gesture && flyout) {
const gesture = this.flyout?.targetWorkspace?.getGesture(e);
if (gesture && this.flyout) {
gesture.setStartBlock(block);
gesture.handleFlyoutStart(e, flyout);
gesture.handleFlyoutStart(e, this.flyout);
}
},
),
);

blockListeners.push(
browserEvents.bind(block.getSvgRoot(), 'pointerenter', null, () => {
if (!this.flyoutWorkspace?.targetWorkspace?.isDragging()) {
if (!this.flyout?.targetWorkspace?.isDragging()) {
block.addSelect();
}
}),
);
blockListeners.push(
browserEvents.bind(block.getSvgRoot(), 'pointerleave', null, () => {
if (!this.flyoutWorkspace?.targetWorkspace?.isDragging()) {
if (!this.flyout?.targetWorkspace?.isDragging()) {
block.removeSelect();
}
}),
Expand All @@ -245,7 +238,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
*/
private filterFlyoutBasedOnCapacity(event: AbstractEvent) {
if (
!this.flyoutWorkspace ||
!this.flyout ||
(event &&
!(
event.type === EventType.BLOCK_CREATE ||
Expand All @@ -254,11 +247,14 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
)
return;

this.flyoutWorkspace.getTopBlocks(false).forEach((block) => {
if (!this.permanentlyDisabledBlocks.has(block)) {
this.updateStateBasedOnCapacity(block);
}
});
this.flyout
.getWorkspace()
.getTopBlocks(false)
.forEach((block) => {
if (!this.permanentlyDisabledBlocks.has(block)) {
this.updateStateBasedOnCapacity(block);
}
});
}

/**
Expand Down
10 changes: 5 additions & 5 deletions core/button_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

import {FlyoutButton} from './flyout_button.js';
import {FlyoutItem} from './flyout_item.js';
import type {IFlyout} from './interfaces/i_flyout.js';
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
import * as registry from './registry.js';
import {ButtonOrLabelInfo} from './utils/toolbox.js';
import type {WorkspaceSvg} from './workspace_svg.js';

const BUTTON_TYPE = 'button';

Expand All @@ -21,13 +21,13 @@ export class ButtonFlyoutInflater implements IFlyoutInflater {
* Inflates a flyout button from the given state and adds it to the flyout.
*
* @param state A JSON representation of a flyout button.
* @param flyoutWorkspace The workspace to create the button on.
* @param flyout The flyout to create the button on.
* @returns A newly created FlyoutButton.
*/
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
load(state: object, flyout: IFlyout): FlyoutItem {
const button = new FlyoutButton(
flyoutWorkspace,
flyoutWorkspace.targetWorkspace!,
flyout.getWorkspace(),
flyout.targetWorkspace!,
state as ButtonOrLabelInfo,
false,
);
Expand Down
2 changes: 1 addition & 1 deletion core/flyout_base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ export abstract class Flyout
const type = info['kind'].toLowerCase();
const inflater = this.getInflaterForType(type);
if (inflater) {
contents.push(inflater.load(info, this.getWorkspace()));
contents.push(inflater.load(info, this));
const gap = inflater.gapForItem(info, defaultGap);
if (gap) {
contents.push(
Expand Down
6 changes: 3 additions & 3 deletions core/interfaces/i_flyout_inflater.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {FlyoutItem} from '../flyout_item.js';
import type {WorkspaceSvg} from '../workspace_svg.js';
import type {IFlyout} from './i_flyout.js';

export interface IFlyoutInflater {
/**
Expand All @@ -9,14 +9,14 @@ export interface IFlyoutInflater {
* allow for code reuse.
*
* @param state A JSON representation of an element to inflate on the flyout.
* @param flyoutWorkspace The flyout's workspace, where the inflated element
* @param flyout The flyout on whose workspace the inflated element
* should be created. If the inflated element is an `IRenderedElement` it
* itself or the inflater should append it to the workspace; the flyout
* will not do so itself. The flyout is responsible for positioning the
* element, however.
* @returns The newly inflated flyout element.
*/
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem;
load(state: object, flyout: IFlyout): FlyoutItem;

/**
* Returns the amount of spacing that should follow the element corresponding
Expand Down
11 changes: 5 additions & 6 deletions core/label_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@

import {FlyoutButton} from './flyout_button.js';
import {FlyoutItem} from './flyout_item.js';
import type {IFlyout} from './interfaces/i_flyout.js';
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
import * as registry from './registry.js';
import {ButtonOrLabelInfo} from './utils/toolbox.js';
import type {WorkspaceSvg} from './workspace_svg.js';

const LABEL_TYPE = 'label';

/**
Expand All @@ -21,13 +20,13 @@ export class LabelFlyoutInflater implements IFlyoutInflater {
* Inflates a flyout label from the given state and adds it to the flyout.
*
* @param state A JSON representation of a flyout label.
* @param flyoutWorkspace The workspace to create the label on.
* @param flyout The flyout to create the label on.
* @returns A FlyoutButton configured as a label.
*/
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
load(state: object, flyout: IFlyout): FlyoutItem {
const label = new FlyoutButton(
flyoutWorkspace,
flyoutWorkspace.targetWorkspace!,
flyout.getWorkspace(),
flyout.targetWorkspace!,
state as ButtonOrLabelInfo,
true,
);
Expand Down
9 changes: 4 additions & 5 deletions core/separator_flyout_inflater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

import {FlyoutItem} from './flyout_item.js';
import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js';
import type {IFlyout} from './interfaces/i_flyout.js';
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
import * as registry from './registry.js';
import type {SeparatorInfo} from './utils/toolbox.js';
import type {WorkspaceSvg} from './workspace_svg.js';

/**
* @internal
Expand All @@ -35,12 +35,11 @@ export class SeparatorFlyoutInflater implements IFlyoutInflater {
* returned by gapForElement, which knows the default gap, unlike this method.
*
* @param _state A JSON representation of a flyout separator.
* @param flyoutWorkspace The workspace the separator belongs to.
* @param flyout The flyout to create the separator for.
* @returns A newly created FlyoutSeparator.
*/
load(_state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
const flyoutAxis = flyoutWorkspace.targetWorkspace?.getFlyout()
?.horizontalLayout
load(_state: object, flyout: IFlyout): FlyoutItem {
const flyoutAxis = flyout.horizontalLayout
? SeparatorAxis.X
: SeparatorAxis.Y;
const separator = new FlyoutSeparator(0, flyoutAxis);
Expand Down

0 comments on commit e6e57dd

Please sign in to comment.