Skip to content

Commit e6e57dd

Browse files
authored
fix: Fix bug that caused blocks dragged from non-primary flyouts to be misplaced. (#8753)
* fix: Fix bug that caused blocks dragged from non-primary flyouts to be misplaced. * chore: Fix docstring.
1 parent 343c2f5 commit e6e57dd

6 files changed

Lines changed: 45 additions & 51 deletions

File tree

core/block_flyout_inflater.ts

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ const BLOCK_TYPE = 'block';
3535
export class BlockFlyoutInflater implements IFlyoutInflater {
3636
protected permanentlyDisabledBlocks = new Set<BlockSvg>();
3737
protected listeners = new Map<string, browserEvents.Data[]>();
38-
protected flyoutWorkspace?: WorkspaceSvg;
3938
protected flyout?: IFlyout;
4039
private capacityWrapper: (event: AbstractEvent) => void;
4140

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

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

159157
/**
160-
* Updates this inflater's flyout workspace.
158+
* Updates this inflater's flyout.
161159
*
162-
* @param workspace The workspace of the flyout that owns this inflater.
160+
* @param flyout The flyout that owns this inflater.
163161
*/
164-
protected setFlyoutWorkspace(workspace: WorkspaceSvg) {
165-
if (this.flyoutWorkspace === workspace) return;
162+
protected setFlyout(flyout: IFlyout) {
163+
if (this.flyout === flyout) return;
166164

167-
if (this.flyoutWorkspace) {
168-
this.flyoutWorkspace.targetWorkspace?.removeChangeListener(
169-
this.capacityWrapper,
170-
);
165+
if (this.flyout) {
166+
this.flyout.targetWorkspace?.removeChangeListener(this.capacityWrapper);
171167
}
172-
this.flyoutWorkspace = workspace;
173-
this.flyoutWorkspace.targetWorkspace?.addChangeListener(
174-
this.capacityWrapper,
175-
);
168+
this.flyout = flyout;
169+
this.flyout.targetWorkspace?.addChangeListener(this.capacityWrapper);
176170
}
177171

178172
/**
@@ -182,7 +176,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
182176
* @param block The block to update the enabled/disabled state of.
183177
*/
184178
private updateStateBasedOnCapacity(block: BlockSvg) {
185-
const enable = this.flyoutWorkspace?.targetWorkspace?.isCapacityAvailable(
179+
const enable = this.flyout?.targetWorkspace?.isCapacityAvailable(
186180
common.getBlockTypeCounts(block),
187181
);
188182
let currentBlock: BlockSvg | null = block;
@@ -209,26 +203,25 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
209203
'pointerdown',
210204
block,
211205
(e: PointerEvent) => {
212-
const gesture = this.flyoutWorkspace?.targetWorkspace?.getGesture(e);
213-
const flyout = this.flyoutWorkspace?.targetWorkspace?.getFlyout();
214-
if (gesture && flyout) {
206+
const gesture = this.flyout?.targetWorkspace?.getGesture(e);
207+
if (gesture && this.flyout) {
215208
gesture.setStartBlock(block);
216-
gesture.handleFlyoutStart(e, flyout);
209+
gesture.handleFlyoutStart(e, this.flyout);
217210
}
218211
},
219212
),
220213
);
221214

222215
blockListeners.push(
223216
browserEvents.bind(block.getSvgRoot(), 'pointerenter', null, () => {
224-
if (!this.flyoutWorkspace?.targetWorkspace?.isDragging()) {
217+
if (!this.flyout?.targetWorkspace?.isDragging()) {
225218
block.addSelect();
226219
}
227220
}),
228221
);
229222
blockListeners.push(
230223
browserEvents.bind(block.getSvgRoot(), 'pointerleave', null, () => {
231-
if (!this.flyoutWorkspace?.targetWorkspace?.isDragging()) {
224+
if (!this.flyout?.targetWorkspace?.isDragging()) {
232225
block.removeSelect();
233226
}
234227
}),
@@ -245,7 +238,7 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
245238
*/
246239
private filterFlyoutBasedOnCapacity(event: AbstractEvent) {
247240
if (
248-
!this.flyoutWorkspace ||
241+
!this.flyout ||
249242
(event &&
250243
!(
251244
event.type === EventType.BLOCK_CREATE ||
@@ -254,11 +247,14 @@ export class BlockFlyoutInflater implements IFlyoutInflater {
254247
)
255248
return;
256249

257-
this.flyoutWorkspace.getTopBlocks(false).forEach((block) => {
258-
if (!this.permanentlyDisabledBlocks.has(block)) {
259-
this.updateStateBasedOnCapacity(block);
260-
}
261-
});
250+
this.flyout
251+
.getWorkspace()
252+
.getTopBlocks(false)
253+
.forEach((block) => {
254+
if (!this.permanentlyDisabledBlocks.has(block)) {
255+
this.updateStateBasedOnCapacity(block);
256+
}
257+
});
262258
}
263259

264260
/**

core/button_flyout_inflater.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66

77
import {FlyoutButton} from './flyout_button.js';
88
import {FlyoutItem} from './flyout_item.js';
9+
import type {IFlyout} from './interfaces/i_flyout.js';
910
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
1011
import * as registry from './registry.js';
1112
import {ButtonOrLabelInfo} from './utils/toolbox.js';
12-
import type {WorkspaceSvg} from './workspace_svg.js';
1313

1414
const BUTTON_TYPE = 'button';
1515

@@ -21,13 +21,13 @@ export class ButtonFlyoutInflater implements IFlyoutInflater {
2121
* Inflates a flyout button from the given state and adds it to the flyout.
2222
*
2323
* @param state A JSON representation of a flyout button.
24-
* @param flyoutWorkspace The workspace to create the button on.
24+
* @param flyout The flyout to create the button on.
2525
* @returns A newly created FlyoutButton.
2626
*/
27-
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
27+
load(state: object, flyout: IFlyout): FlyoutItem {
2828
const button = new FlyoutButton(
29-
flyoutWorkspace,
30-
flyoutWorkspace.targetWorkspace!,
29+
flyout.getWorkspace(),
30+
flyout.targetWorkspace!,
3131
state as ButtonOrLabelInfo,
3232
false,
3333
);

core/flyout_base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ export abstract class Flyout
678678
const type = info['kind'].toLowerCase();
679679
const inflater = this.getInflaterForType(type);
680680
if (inflater) {
681-
contents.push(inflater.load(info, this.getWorkspace()));
681+
contents.push(inflater.load(info, this));
682682
const gap = inflater.gapForItem(info, defaultGap);
683683
if (gap) {
684684
contents.push(

core/interfaces/i_flyout_inflater.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type {FlyoutItem} from '../flyout_item.js';
2-
import type {WorkspaceSvg} from '../workspace_svg.js';
2+
import type {IFlyout} from './i_flyout.js';
33

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

2121
/**
2222
* Returns the amount of spacing that should follow the element corresponding

core/label_flyout_inflater.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66

77
import {FlyoutButton} from './flyout_button.js';
88
import {FlyoutItem} from './flyout_item.js';
9+
import type {IFlyout} from './interfaces/i_flyout.js';
910
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
1011
import * as registry from './registry.js';
1112
import {ButtonOrLabelInfo} from './utils/toolbox.js';
12-
import type {WorkspaceSvg} from './workspace_svg.js';
13-
1413
const LABEL_TYPE = 'label';
1514

1615
/**
@@ -21,13 +20,13 @@ export class LabelFlyoutInflater implements IFlyoutInflater {
2120
* Inflates a flyout label from the given state and adds it to the flyout.
2221
*
2322
* @param state A JSON representation of a flyout label.
24-
* @param flyoutWorkspace The workspace to create the label on.
23+
* @param flyout The flyout to create the label on.
2524
* @returns A FlyoutButton configured as a label.
2625
*/
27-
load(state: object, flyoutWorkspace: WorkspaceSvg): FlyoutItem {
26+
load(state: object, flyout: IFlyout): FlyoutItem {
2827
const label = new FlyoutButton(
29-
flyoutWorkspace,
30-
flyoutWorkspace.targetWorkspace!,
28+
flyout.getWorkspace(),
29+
flyout.targetWorkspace!,
3130
state as ButtonOrLabelInfo,
3231
true,
3332
);

core/separator_flyout_inflater.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66

77
import {FlyoutItem} from './flyout_item.js';
88
import {FlyoutSeparator, SeparatorAxis} from './flyout_separator.js';
9+
import type {IFlyout} from './interfaces/i_flyout.js';
910
import type {IFlyoutInflater} from './interfaces/i_flyout_inflater.js';
1011
import * as registry from './registry.js';
1112
import type {SeparatorInfo} from './utils/toolbox.js';
12-
import type {WorkspaceSvg} from './workspace_svg.js';
1313

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

0 commit comments

Comments
 (0)