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: refactor CopyData interface to have the correct structure #7344

Merged
merged 4 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {FieldLabel} from './field_label.js';
import type {Input} from './inputs/input.js';
import type {IASTNodeLocationSvg} from './interfaces/i_ast_node_location_svg.js';
import type {IBoundedElement} from './interfaces/i_bounded_element.js';
import type {CopyData, ICopyable} from './interfaces/i_copyable.js';
import type {ICopyable} from './interfaces/i_copyable.js';
import type {IDraggable} from './interfaces/i_draggable.js';
import {IIcon} from './interfaces/i_icon.js';
import * as internalConstants from './internal_constants.js';
Expand All @@ -62,6 +62,7 @@ import type {WorkspaceSvg} from './workspace_svg.js';
import * as renderManagement from './render_management.js';
import * as deprecation from './utils/deprecation.js';
import {IconType} from './icons/icon_types.js';
import {BlockCopyData, BlockPaster} from './clipboard/block_paster.js';

/**
* Class for a block's SVG representation.
Expand Down Expand Up @@ -823,18 +824,17 @@ export class BlockSvg
* Encode a block for copying.
*
* @returns Copy metadata, or null if the block is an insertion marker.
* @internal
*/
toCopyData(): CopyData | null {
toCopyData(): BlockCopyData | null {
if (this.isInsertionMarker_) {
return null;
}
return {
paster: BlockPaster.TYPE,
saveInfo: blocks.save(this, {
addCoordinates: true,
addNextBlocks: false,
}) as blocks.State,
source: this.workspace,
typeCounts: common.getBlockTypeCounts(this, true),
};
}
Expand Down
31 changes: 19 additions & 12 deletions core/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@
import * as goog from '../closure/goog/goog.js';
goog.declareModuleId('Blockly.clipboard');

import type {CopyData, ICopyable} from './interfaces/i_copyable.js';
import type {ICopyData, ICopyable} from './interfaces/i_copyable.js';
import {BlockPaster} from './clipboard/block_paster.js';
import * as globalRegistry from './registry.js';
import {WorkspaceSvg} from './workspace_svg.js';
import * as registry from './clipboard/registry.js';

/** Metadata about the object that is currently on the clipboard. */
let copyData: CopyData | null = null;
let copyData: ICopyData | null = null;

let source: WorkspaceSvg | null = null;

/**
* Copy a block or workspace comment onto the local clipboard.
Expand All @@ -29,6 +33,7 @@ export function copy(toCopy: ICopyable) {
*/
function copyInternal(toCopy: ICopyable) {
copyData = toCopy.toCopyData();
source = (toCopy as any).workspace ?? null;
}

/**
Expand All @@ -43,17 +48,16 @@ export function paste(): ICopyable | null {
}
// Pasting always pastes to the main workspace, even if the copy
// started in a flyout workspace.
let workspace = copyData.source;
if (workspace.isFlyout) {
let workspace = source;
if (workspace?.isFlyout) {
workspace = workspace.targetWorkspace!;
}
if (
copyData.typeCounts &&
workspace.isCapacityAvailable(copyData.typeCounts)
) {
return workspace.paste(copyData.saveInfo);
}
return null;
if (!workspace) return null;
return (
globalRegistry
.getObject(globalRegistry.Type.PASTER, copyData.paster, false)
?.paste(copyData, workspace) ?? null
);
}

/**
Expand All @@ -74,8 +78,11 @@ export function duplicate(toDuplicate: ICopyable): ICopyable | null {
function duplicateInternal(toDuplicate: ICopyable): ICopyable | null {
const oldCopyData = copyData;
copy(toDuplicate);
if (!copyData || !source) return null;
const pastedThing =
toDuplicate.toCopyData()?.source?.paste(copyData!.saveInfo) ?? null;
globalRegistry
.getObject(globalRegistry.Type.PASTER, copyData.paster, false)
?.paste(copyData, source) ?? null;
copyData = oldCopyData;
return pastedThing;
}
Expand Down
16 changes: 9 additions & 7 deletions core/clipboard/block_paster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
*/

import {BlockSvg} from '../block_svg.js';
import {registry} from '../clipboard.js';
import {CopyData} from '../interfaces/i_copyable.js';
import * as registry from './registry.js';
import {ICopyData} from '../interfaces/i_copyable.js';
import {IPaster} from '../interfaces/i_paster.js';
import {State, append} from '../serialization/blocks.js';
import {Coordinate} from '../utils/coordinate.js';
Expand All @@ -22,15 +22,17 @@ export class BlockPaster implements IPaster<BlockCopyData, BlockSvg> {
): BlockSvg | null {
if (!workspace.isCapacityAvailable(copyData.typeCounts!)) return null;

const state = copyData.saveInfo as State;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curiosity: are you removing this just because it isn't a very useful temporary binding, or is there a type-checking reason?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The saveInfo used to be typed as Object | Element because blocks and workspace comments were using the same interface, which forced me to cast. But now they are using separate interfaces with proper types, so I could remove the cast =)

if (coordinate) {
state['x'] = coordinate.x;
state['y'] = coordinate.y;
copyData.saveInfo['x'] = coordinate.x;
copyData.saveInfo['y'] = coordinate.y;
}
return append(state, workspace, {recordUndo: true}) as BlockSvg;
return append(copyData.saveInfo, workspace, {recordUndo: true}) as BlockSvg;
}
}

export interface BlockCopyData extends CopyData {}
export interface BlockCopyData extends ICopyData {
saveInfo: State;
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
typeCounts: {[key: string]: number};
}

registry.register(BlockPaster.TYPE, new BlockPaster());
4 changes: 2 additions & 2 deletions core/clipboard/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {ICopyable, CopyData} from '../interfaces/i_copyable.js';
import {ICopyable, ICopyData} from '../interfaces/i_copyable.js';
import type {IPaster} from '../interfaces/i_paster.js';
import * as registry from '../registry.js';

Expand All @@ -14,7 +14,7 @@ import * as registry from '../registry.js';
* @param type The type of the paster to register, e.g. 'block', 'comment', etc.
* @param paster The paster to register.
*/
export function register<U extends CopyData, T extends ICopyable>(
export function register<U extends ICopyData, T extends ICopyable>(
type: string,
paster: IPaster<U, T>,
) {
Expand Down
15 changes: 8 additions & 7 deletions core/clipboard/workspace_comment_paster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
*/

import {IPaster} from '../interfaces/i_paster.js';
import {CopyData} from '../interfaces/i_copyable.js';
import {ICopyData} from '../interfaces/i_copyable.js';
import {Coordinate} from '../utils/coordinate.js';
import {WorkspaceSvg} from '../workspace_svg.js';
import {WorkspaceCommentSvg} from '../workspace_comment_svg.js';
import {registry} from '../clipboard.js';
import * as registry from './registry.js';

export class WorkspaceCommentPaster
implements IPaster<WorkspaceCommentCopyData, WorkspaceCommentSvg>
Expand All @@ -21,15 +21,16 @@ export class WorkspaceCommentPaster
workspace: WorkspaceSvg,
coordinate?: Coordinate,
): WorkspaceCommentSvg {
const state = copyData.saveInfo as Element;
if (coordinate) {
state.setAttribute('x', `${coordinate.x}`);
state.setAttribute('y', `${coordinate.y}`);
copyData.saveInfo.setAttribute('x', `${coordinate.x}`);
copyData.saveInfo.setAttribute('y', `${coordinate.y}`);
}
return WorkspaceCommentSvg.fromXmlRendered(state, workspace);
return WorkspaceCommentSvg.fromXmlRendered(copyData.saveInfo, workspace);
}
}

export interface WorkspaceCommentCopyData extends CopyData {}
export interface WorkspaceCommentCopyData extends ICopyData {
saveInfo: Element;
}

registry.register(WorkspaceCommentPaster.TYPE, new WorkspaceCommentPaster());
12 changes: 4 additions & 8 deletions core/interfaces/i_copyable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,21 @@
import * as goog from '../../closure/goog/goog.js';
goog.declareModuleId('Blockly.ICopyable');

import type {WorkspaceSvg} from '../workspace_svg.js';
import type {ISelectable} from './i_selectable.js';

export interface ICopyable extends ISelectable {
/**
* Encode for copying.
*
* @returns Copy metadata.
* @internal
*/
toCopyData(): CopyData | null;
toCopyData(): ICopyData | null;
}

export namespace ICopyable {
export interface CopyData {
saveInfo: Object | Element;
source: WorkspaceSvg;
typeCounts: {[key: string]: number} | null;
export interface ICopyData {
paster: string;
}
}

export type CopyData = ICopyable.CopyData;
export type ICopyData = ICopyable.ICopyData;
6 changes: 3 additions & 3 deletions core/interfaces/i_paster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

import {Coordinate} from '../utils/coordinate.js';
import {WorkspaceSvg} from '../workspace_svg.js';
import {ICopyable, CopyData} from './i_copyable.js';
import {ICopyable, ICopyData} from './i_copyable.js';

/** An object that can paste data into a workspace. */
export interface IPaster<U extends CopyData, T extends ICopyable> {
export interface IPaster<U extends ICopyData, T extends ICopyable> {
paste(
copyData: U,
workspace: WorkspaceSvg,
Expand All @@ -18,6 +18,6 @@ export interface IPaster<U extends CopyData, T extends ICopyable> {
}

/** @returns True if the given object is a paster. */
export function isPaster(obj: any): obj is IPaster<CopyData, ICopyable> {
export function isPaster(obj: any): obj is IPaster<ICopyData, ICopyable> {
return obj.paste !== undefined;
}
4 changes: 2 additions & 2 deletions core/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {Renderer} from './renderers/common/renderer.js';
import type {Theme} from './theme.js';
import type {ToolboxItem} from './toolbox/toolbox_item.js';
import {IPaster} from './interfaces/i_paster.js';
import {CopyData, ICopyable} from './interfaces/i_copyable.js';
import {ICopyData, ICopyable} from './interfaces/i_copyable.js';

/**
* A map of maps. With the keys being the type and name of the class we are
Expand Down Expand Up @@ -100,7 +100,7 @@ export class Type<_T> {
static ICON = new Type<IIcon>('icon');

/** @internal */
static PASTER = new Type<IPaster<CopyData, ICopyable>>('paster');
static PASTER = new Type<IPaster<ICopyData, ICopyable>>('paster');
}

/**
Expand Down
12 changes: 7 additions & 5 deletions core/workspace_comment_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {CommentMove} from './events/events_comment_move.js';
import * as eventUtils from './events/utils.js';
import type {IBoundedElement} from './interfaces/i_bounded_element.js';
import type {IBubble} from './interfaces/i_bubble.js';
import type {CopyData, ICopyable} from './interfaces/i_copyable.js';
import type {ICopyable} from './interfaces/i_copyable.js';
import * as Touch from './touch.js';
import {Coordinate} from './utils/coordinate.js';
import * as dom from './utils/dom.js';
Expand All @@ -32,6 +32,10 @@ import {Svg} from './utils/svg.js';
import * as svgMath from './utils/svg_math.js';
import {WorkspaceComment} from './workspace_comment.js';
import type {WorkspaceSvg} from './workspace_svg.js';
import {
WorkspaceCommentCopyData,
WorkspaceCommentPaster,
} from './clipboard/workspace_comment_paster.js';

/** Size of the resize icon. */
const RESIZE_SIZE = 8;
Expand Down Expand Up @@ -566,13 +570,11 @@ export class WorkspaceCommentSvg
* Encode a comment for copying.
*
* @returns Copy metadata.
* @internal
*/
toCopyData(): CopyData {
toCopyData(): WorkspaceCommentCopyData {
return {
paster: WorkspaceCommentPaster.TYPE,
saveInfo: this.toXmlWithXY(),
source: this.workspace,
typeCounts: null,
};
}

Expand Down