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: widget positioning #7507

Merged
merged 10 commits into from
Oct 26, 2023
Merged
65 changes: 11 additions & 54 deletions core/block_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import * as registry from './registry.js';
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 +48,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 @@ -69,13 +74,6 @@ export class BlockDragger implements IBlockDragger {
* beginning of the drag in workspace coordinates.
*/
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 @@ -84,8 +82,6 @@ export class BlockDragger implements IBlockDragger {
* @internal
*/
dispose() {
this.dragIconData_.length = 0;
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved

if (this.draggedConnectionManager_) {
this.draggedConnectionManager_.dispose();
}
Expand Down Expand Up @@ -178,7 +174,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 +205,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 +392,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 All @@ -432,38 +423,4 @@ export interface IconPositionData {
icon: Icon;
}

/**
* Make 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.
*
* @param block The root block that is being dragged.
* @param blockOrigin The top left of the given block in workspace coordinates.
* @returns The list of all icons and their locations.
*/
function initIconData(
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
block: BlockSvg,
blockOrigin: Coordinate,
): IconPositionData[] {
// Build a list of icons that need to be moved and where they started.
const dragIconData = [];

for (const icon of block.getIcons()) {
// Only bother to track icons whose bubble is visible.
if (hasBubble(icon) && !icon.bubbleIsVisible()) continue;

dragIconData.push({location: blockOrigin, icon: icon});
icon.onLocationChange(blockOrigin);
}

for (const child of block.getChildren(false)) {
dragIconData.push(
...initIconData(child, Coordinate.sum(blockOrigin, child.relativeCoords)),
);
}
// AnyDuringMigration because: Type '{ location: Coordinate | null; icon:
// Icon; }[]' is not assignable to type 'IconPositionData[]'.
return dragIconData as AnyDuringMigration;
}

registry.register(registry.Type.BLOCK_DRAGGER, registry.DEFAULT, BlockDragger);
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);
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
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
22 changes: 12 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 @@ -616,6 +617,7 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
}
const div = WidgetDiv.getDiv();
const bBox = this.getScaledBBox();
console.log(bBox);
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
div!.style.width = bBox.right - bBox.left + 'px';
div!.style.height = bBox.bottom - bBox.top + 'px';

Expand All @@ -631,7 +633,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 +644,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_();
maribethb marked this conversation as resolved.
Show resolved Hide resolved

return true;
}
Expand Down
Loading