Skip to content

Commit

Permalink
fix: widget positioning (#7507)
Browse files Browse the repository at this point in the history
* chore: delete dead code

* chore: moves location updating into the block

* chore: change dragging to use update component locations

* fix: field widgets not being moved when blocks are editted

* chore: remove unnecessary resizeEditor_ calls

* chore: format

* chore: fix build

* fix: tests

* chore: PR comments

* chore: format
  • Loading branch information
BeksOmega authored Oct 26, 2023
1 parent e7dec4a commit 7d2c307
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 160 deletions.
27 changes: 11 additions & 16 deletions core/block_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {Coordinate} from './utils/coordinate.js';
import * as dom from './utils/dom.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import {hasBubble} from './interfaces/i_has_bubble.js';
import * as deprecation from './utils/deprecation.js';

/**
* Class for a block dragger. It moves blocks around the workspace when they
Expand All @@ -48,7 +49,12 @@ export class BlockDragger implements IBlockDragger {
/** Whether the block would be deleted if dropped immediately. */
protected wouldDeleteBlock_ = false;
protected startXY_: Coordinate;
protected dragIconData_: IconPositionData[];

/**
* @deprecated To be removed in v11. Updating icons is now handled by the
* block's `moveDuringDrag` method.
*/
protected dragIconData_: IconPositionData[] = [];

/**
* @param block The block to drag.
Expand All @@ -70,11 +76,6 @@ export class BlockDragger implements IBlockDragger {
*/
this.startXY_ = this.draggingBlock_.getRelativeToSurfaceXY();

/**
* A list of all of the icons (comment, warning, and mutator) that are
* on this block and its descendants. Moving an icon moves the bubble that
* extends from it if that bubble is open.
*/
this.dragIconData_ = initIconData(block, this.startXY_);
}

Expand All @@ -85,7 +86,6 @@ export class BlockDragger implements IBlockDragger {
*/
dispose() {
this.dragIconData_.length = 0;

if (this.draggedConnectionManager_) {
this.draggedConnectionManager_.dispose();
}
Expand Down Expand Up @@ -178,7 +178,6 @@ export class BlockDragger implements IBlockDragger {
const delta = this.pixelsToWorkspaceUnits_(currentDragDeltaXY);
const newLoc = Coordinate.sum(this.startXY_, delta);
this.draggingBlock_.moveDuringDrag(newLoc);
this.dragIcons_(delta);

const oldDragTarget = this.dragTarget_;
this.dragTarget_ = this.workspace_.getDragTarget(e);
Expand Down Expand Up @@ -210,7 +209,6 @@ export class BlockDragger implements IBlockDragger {
endDrag(e: PointerEvent, currentDragDeltaXY: Coordinate) {
// Make sure internal state is fresh.
this.drag(e, currentDragDeltaXY);
this.dragIconData_ = [];
this.fireDragEndEvent_();

dom.stopTextWidthCache();
Expand Down Expand Up @@ -398,14 +396,11 @@ export class BlockDragger implements IBlockDragger {
/**
* Move all of the icons connected to this drag.
*
* @param dxy How far to move the icons from their original positions, in
* workspace units.
* @deprecated To be removed in v11. This is now handled by the block's
* `moveDuringDrag` method.
*/
protected dragIcons_(dxy: Coordinate) {
// Moving icons moves their associated bubbles.
for (const data of this.dragIconData_) {
data.icon.onLocationChange(Coordinate.sum(data.location, dxy));
}
protected dragIcons_() {
deprecation.warn('Blockly.BlockDragger.prototype.dragIcons_', 'v10', 'v11');
}

/**
Expand Down
100 changes: 42 additions & 58 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ export class BlockSvg
*/
private bumpNeighboursPid = 0;

/** Whether this block is currently being dragged. */
private dragging = false;

/**
* The location of the top left of this block (in workspace coordinates)
* relative to either its parent block, or the workspace origin if it has no
Expand Down Expand Up @@ -384,9 +387,13 @@ export class BlockSvg
event = new (eventUtils.get(eventUtils.BLOCK_MOVE)!)(this) as BlockMove;
reason && event.setReason(reason);
}
const xy = this.getRelativeToSurfaceXY();
this.translate(xy.x + dx, xy.y + dy);
this.moveConnections(dx, dy);

const delta = new Coordinate(dx, dy);
const currLoc = this.getRelativeToSurfaceXY();
const newLoc = Coordinate.sum(currLoc, delta);
this.translate(newLoc.x, newLoc.y);
this.updateComponentLocations(newLoc);

if (eventsEnabled && event) {
event!.recordNew();
eventUtils.fire(event);
Expand Down Expand Up @@ -437,6 +444,7 @@ export class BlockSvg
moveDuringDrag(newLoc: Coordinate) {
this.translate(newLoc.x, newLoc.y);
this.getSvgRoot().setAttribute('transform', this.getTranslation());
this.updateComponentLocations(newLoc);
}

/** Snap this block to the nearest grid point. */
Expand Down Expand Up @@ -649,32 +657,47 @@ export class BlockSvg
}

/**
* Move the connections for this block and all blocks attached under it.
* Also update any attached bubbles.
* Updates the locations of any parts of the block that need to know where
* they are (e.g. connections, icons).
*
* @param dx Horizontal offset from current location, in workspace units.
* @param dy Vertical offset from current location, in workspace units.
* @param blockOrigin The top-left of this block in workspace coordinates.
* @internal
*/
moveConnections(dx: number, dy: number) {
updateComponentLocations(blockOrigin: Coordinate) {
if (!this.rendered) {
// Rendering is required to lay out the blocks.
// This is probably an invisible block attached to a collapsed block.
return;
}
const myConnections = this.getConnections_(false);
for (let i = 0; i < myConnections.length; i++) {
myConnections[i].moveBy(dx, dy);

if (!this.dragging) this.updateConnectionLocations(blockOrigin);
this.updateIconLocations(blockOrigin);
this.updateFieldLocations(blockOrigin);

for (const child of this.getChildren(false)) {
child.updateComponentLocations(
Coordinate.sum(blockOrigin, child.relativeCoords),
);
}
const icons = this.getIcons();
const pos = this.getRelativeToSurfaceXY();
for (const icon of icons) {
icon.onLocationChange(pos);
}

private updateConnectionLocations(blockOrigin: Coordinate) {
for (const conn of this.getConnections_(false)) {
conn.moveToOffset(blockOrigin);
}
}

// Recurse through all blocks attached under this one.
for (let i = 0; i < this.childBlocks_.length; i++) {
(this.childBlocks_[i] as BlockSvg).moveConnections(dx, dy);
private updateIconLocations(blockOrigin: Coordinate) {
for (const icon of this.getIcons()) {
icon.onLocationChange(blockOrigin);
}
}

private updateFieldLocations(blockOrigin: Coordinate) {
for (const input of this.inputList) {
for (const field of input.fieldRow) {
field.onLocationChange(blockOrigin);
}
}
}

Expand All @@ -686,6 +709,7 @@ export class BlockSvg
* @internal
*/
setDragging(adding: boolean) {
this.dragging = adding;
if (adding) {
this.translation = '';
common.draggingConnections.push(...this.getConnections_(true));
Expand Down Expand Up @@ -1628,46 +1652,6 @@ export class BlockSvg
}
}

/**
* Update all of the connections on this block with the new locations
* calculated during rendering. Also move all of the connected blocks based
* on the new connection locations.
*
* @internal
*/
private updateConnectionAndIconLocations() {
const blockTL = this.getRelativeToSurfaceXY();
// Don't tighten previous or output connections because they are inferior
// connections.
if (this.previousConnection) {
this.previousConnection.moveToOffset(blockTL);
}
if (this.outputConnection) {
this.outputConnection.moveToOffset(blockTL);
}

for (let i = 0; i < this.inputList.length; i++) {
const conn = this.inputList[i].connection as RenderedConnection;
if (conn) {
conn.moveToOffset(blockTL);
if (conn.isConnected()) {
conn.tighten();
}
}
}

if (this.nextConnection) {
this.nextConnection.moveToOffset(blockTL);
if (this.nextConnection.isConnected()) {
this.nextConnection.tighten();
}
}

for (const icon of this.getIcons()) {
icon.onLocationChange(blockTL);
}
}

/**
* Add the cursor SVG to this block's SVG group.
*
Expand Down
8 changes: 8 additions & 0 deletions core/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,14 @@ export abstract class Field<T = any>
return new Rect(xy.y, xy.y + scaledHeight, xy.x, xy.x + scaledWidth);
}

/**
* Notifies the field that it has changed locations.
*
* @param _ The location of this field's block's top-start corner
* in workspace coordinates.
*/
onLocationChange(_: Coordinate) {}

/**
* Get the text from this field to display on the block. May differ from
* `getText` due to ellipsis, and other formatting.
Expand Down
21 changes: 11 additions & 10 deletions core/field_input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {Coordinate} from './utils/coordinate.js';
import * as userAgent from './utils/useragent.js';
import * as WidgetDiv from './widgetdiv.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import * as renderManagement from './render_management.js';
import {Size} from './utils/size.js';

/**
Expand Down Expand Up @@ -251,6 +250,14 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
return super.getSize();
}

/**
* Notifies the field that it has changed locations. Moves the widget div to
* be in the correct place if it is open.
*/
onLocationChange(): void {
if (this.isBeingEdited_) this.resizeEditor_();
}

/**
* Updates the colour of the htmlInput given the current validity of the
* field's value.
Expand All @@ -263,7 +270,6 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
// This logic is done in render_ rather than doValueInvalid_ or
// doValueUpdate_ so that the code is more centralized.
if (this.isBeingEdited_) {
this.resizeEditor_();
const htmlInput = this.htmlInput_ as HTMLElement;
if (!this.isTextValid_) {
dom.addClass(htmlInput, 'blocklyInvalidInput');
Expand Down Expand Up @@ -576,11 +582,6 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
),
);
}

// Resize the widget div after the block has finished rendering.
renderManagement.finishQueuedRenders().then(() => {
this.resizeEditor_();
});
}

/**
Expand Down Expand Up @@ -631,7 +632,7 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
/**
* Handles repositioning the WidgetDiv used for input fields when the
* workspace is resized. Will bump the block into the viewport and update the
* position of the field if necessary.
* position of the text input if necessary.
*
* @returns True for rendered workspaces, as we never want to hide the widget
* div.
Expand All @@ -642,13 +643,13 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
// rendered blocks.
if (!(block instanceof BlockSvg)) return false;

bumpObjects.bumpIntoBounds(
const bumped = bumpObjects.bumpIntoBounds(
this.workspace_!,
this.workspace_!.getMetricsManager().getViewMetrics(true),
block,
);

this.resizeEditor_();
if (!bumped) this.resizeEditor_();

return true;
}
Expand Down
Loading

0 comments on commit 7d2c307

Please sign in to comment.