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

Replace BaseFabricObject with FabricObject #9016

Merged
merged 11 commits into from
Jul 5, 2023
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## [next]

- refactor(svImport): remove the css/gradient/clipPath global definitions [#9030](https://github.com/fabricjs/fabric.js/pull/9030)
- chore(TS): Replace BaseFabricObject with FabricObject [#9016](https://github.com/fabricjs/fabric.js/pull/9016)
- refactor(svgImport): remove the css/gradient/clipPath global definitions [#9030](https://github.com/fabricjs/fabric.js/pull/9030)
- fix(): tweaks to type getter [#9022](https://github.com/fabricjs/fabric.js/pull/9022)
- ci() Refactor GHA actions for caching and reuse [#9029](https://github.com/fabricjs/fabric.js/pull/9029)
- ci(): install dev deps types [#9039](https://github.com/fabricjs/fabric.js/pull/9039)
Expand Down
52 changes: 26 additions & 26 deletions src/Collection.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,39 @@
import type { Constructor, TBBox } from './typedefs';
import type { BaseFabricObject } from './EventTypeDefs';
import { removeFromArray } from './util/internals';
import { Point } from './Point';
import type { InteractiveFabricObject } from './shapes/Object/InteractiveObject';
import type { FabricObject } from './shapes/Object/FabricObject';

export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
class Collection extends Base {
/**
* @type {BaseFabricObject[]}
* @type {FabricObject[]}
* @TODO needs to end up in the constructor too
*/
_objects: BaseFabricObject[] = [];
_objects: FabricObject[] = [];

// eslint-disable-next-line @typescript-eslint/no-unused-vars
_onObjectAdded(object: BaseFabricObject) {
_onObjectAdded(object: FabricObject) {
// subclasses should override this method
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
_onObjectRemoved(object: BaseFabricObject) {
_onObjectRemoved(object: FabricObject) {
// subclasses should override this method
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
_onStackOrderChanged(object: BaseFabricObject) {
_onStackOrderChanged(object: FabricObject) {
// subclasses should override this method
}

/**
* Adds objects to collection
* Objects should be instances of (or inherit from) BaseFabricObject
* @param {...BaseFabricObject[]} objects to add
* Objects should be instances of (or inherit from) FabricObject
* @param {...FabricObject[]} objects to add
* @returns {number} new array length
*/
add(...objects: BaseFabricObject[]): number {
add(...objects: FabricObject[]): number {
const size = this._objects.push(...objects);
objects.forEach((object) => this._onObjectAdded(object));
return size;
Expand All @@ -42,10 +42,10 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
/**
* Inserts an object into collection at specified index
* @param {number} index Index to insert object at
* @param {...BaseFabricObject[]} objects Object(s) to insert
* @param {...FabricObject[]} objects Object(s) to insert
* @returns {number} new array length
*/
insertAt(index: number, ...objects: BaseFabricObject[]) {
insertAt(index: number, ...objects: FabricObject[]) {
this._objects.splice(index, 0, ...objects);
objects.forEach((object) => this._onObjectAdded(object));
return this._objects.length;
Expand All @@ -54,12 +54,12 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
/**
* Removes objects from a collection, then renders canvas (if `renderOnAddRemove` is not `false`)
* @private
* @param {...BaseFabricObject[]} objects objects to remove
* @returns {BaseFabricObject[]} removed objects
* @param {...FabricObject[]} objects objects to remove
* @returns {FabricObject[]} removed objects
*/
remove(...objects: BaseFabricObject[]) {
remove(...objects: FabricObject[]) {
const array = this._objects,
removed: BaseFabricObject[] = [];
removed: FabricObject[] = [];
objects.forEach((object) => {
const index = array.indexOf(object);
// only call onObjectRemoved if an object was actually removed
Expand All @@ -82,9 +82,9 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
*/
forEachObject(
callback: (
object: BaseFabricObject,
object: FabricObject,
index: number,
array: BaseFabricObject[]
array: FabricObject[]
) => any
) {
this.getObjects().forEach((object, index, objects) =>
Expand Down Expand Up @@ -131,13 +131,13 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {

/**
* Returns true if collection contains an object.\
* **Prefer using {@link `BaseFabricObject#isDescendantOf`} for performance reasons**
* **Prefer using {@link `FabricObject#isDescendantOf`} for performance reasons**
* instead of `a.contains(b)` use `b.isDescendantOf(a)`
* @param {Object} object Object to check against
* @param {Boolean} [deep=false] `true` to check all descendants, `false` to check only `_objects`
* @return {Boolean} `true` if collection contains an object
*/
contains(object: BaseFabricObject, deep?: boolean): boolean {
contains(object: FabricObject, deep?: boolean): boolean {
if (this._objects.includes(object)) {
return true;
} else if (deep) {
Expand Down Expand Up @@ -167,7 +167,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
* @param {fabric.Object} object Object to send to back
* @returns {boolean} true if change occurred
*/
sendObjectToBack(object: BaseFabricObject) {
sendObjectToBack(object: FabricObject) {
if (!object || object === this._objects[0]) {
return false;
}
Expand All @@ -183,7 +183,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
* @param {fabric.Object} object Object to send
* @returns {boolean} true if change occurred
*/
bringObjectToFront(object: BaseFabricObject) {
bringObjectToFront(object: FabricObject) {
if (!object || object === this._objects[this._objects.length - 1]) {
return false;
}
Expand All @@ -203,7 +203,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
* @param {boolean} [intersecting] If `true`, send object behind next lower intersecting object
* @returns {boolean} true if change occurred
*/
sendObjectBackwards(object: BaseFabricObject, intersecting?: boolean) {
sendObjectBackwards(object: FabricObject, intersecting?: boolean) {
if (!object) {
return false;
}
Expand All @@ -229,7 +229,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
* @param {boolean} [intersecting] If `true`, send object in front of next upper intersecting object
* @returns {boolean} true if change occurred
*/
bringObjectForward(object: BaseFabricObject, intersecting?: boolean) {
bringObjectForward(object: FabricObject, intersecting?: boolean) {
if (!object) {
return false;
}
Expand All @@ -251,7 +251,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
* @param {number} index Position to move to
* @returns {boolean} true if change occurred
*/
moveObjectTo(object: BaseFabricObject, index: number) {
moveObjectTo(object: FabricObject, index: number) {
if (object === this._objects[index]) {
return false;
}
Expand All @@ -262,7 +262,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
}

findNewLowerIndex(
object: BaseFabricObject,
object: FabricObject,
idx: number,
intersecting?: boolean
) {
Expand All @@ -285,7 +285,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
}

findNewUpperIndex(
object: BaseFabricObject,
object: FabricObject,
idx: number,
intersecting?: boolean
) {
Expand Down
11 changes: 2 additions & 9 deletions src/EventTypeDefs.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
import type { Control } from './controls/Control';
import type { Point } from './Point';
import type { FabricObject } from './shapes/Object/FabricObject';
import type { FabricObject as StaticFabricObject } from './shapes/Object/Object';
import type { FabricObjectSVGExportMixin } from './shapes/Object/FabricObjectSVGExportMixin';
import type { Group } from './shapes/Group';
import type { TOriginX, TOriginY, TRadian } from './typedefs';
import type { saveObjectTransform } from './util/misc/objectTransforms';
import type { Canvas } from './canvas/Canvas';
import type { IText } from './shapes/IText/IText';
import type { StaticCanvas } from './canvas/StaticCanvas';

// eslint-disable-next-line @typescript-eslint/no-empty-interface, @typescript-eslint/no-unused-vars
export interface BaseFabricObject
extends StaticFabricObject,
FabricObjectSVGExportMixin {}

export type ModifierKey = keyof Pick<
MouseEvent | PointerEvent | TouchEvent,
'altKey' | 'shiftKey' | 'ctrlKey' | 'metaKey'
Expand Down Expand Up @@ -201,8 +194,8 @@ interface CanvasSelectionEvents {
}

export interface CollectionEvents {
'object:added': { target: StaticFabricObject };
'object:removed': { target: StaticFabricObject };
'object:added': { target: FabricObject };
'object:removed': { target: FabricObject };
}

type BeforeSuffix<T extends string> = `${T}:before`;
Expand Down
8 changes: 4 additions & 4 deletions src/canvas/StaticCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { createCollectionMixin } from '../Collection';
import { CommonMethods } from '../CommonMethods';
import type { Pattern } from '../Pattern';
import { Point } from '../Point';
import type { BaseFabricObject as FabricObject } from '../EventTypeDefs';
import type { TCachedFabricObject } from '../shapes/Object/Object';
import type { Rect } from '../shapes/Rect';
import type {
Expand Down Expand Up @@ -44,6 +43,7 @@ import {
} from '../util/typeAssertions';
import { StaticCanvasDOMManager } from './DOMManagers/StaticCanvasDOMManager';
import type { CSSDimensions } from './DOMManagers/util';
import type { FabricObject } from '../shapes/Object/FabricObject';
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this actually

Copy link
Contributor

Choose a reason for hiding this comment

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

but it will be solved when we manage to type Collection


export type TCanvasSizeOptions = {
backstoreOnly?: boolean;
Expand Down Expand Up @@ -336,7 +336,7 @@ export class StaticCanvas<
}

_onObjectAdded(obj: FabricObject) {
if (obj.canvas && obj.canvas !== this) {
if (obj.canvas && (obj.canvas as StaticCanvas) !== this) {
/* _DEV_MODE_START_ */
console.warn(
'fabric.Canvas: trying to add an object that belongs to a different canvas.\n' +
Expand Down Expand Up @@ -408,7 +408,7 @@ export class StaticCanvas<
* @param {Boolean} [options.cssOnly=false] Set the given dimensions only as css dimensions
* @deprecated will be removed in 7.0
*/
setWidth(value: number, options: TCanvasSizeOptions) {
setWidth(value: number, options: TCanvasSizeOptions = {}) {
return this.setDimensions({ width: value }, options);
}

Expand All @@ -420,7 +420,7 @@ export class StaticCanvas<
* @param {Boolean} [options.cssOnly=false] Set the given dimensions only as css dimensions
* @deprecated will be removed in 7.0
*/
setHeight(value: number, options: TCanvasSizeOptions) {
setHeight(value: number, options: TCanvasSizeOptions = {}) {
return this.setDimensions({ height: value }, options);
}

Expand Down
2 changes: 1 addition & 1 deletion src/shapes/IText/ITextBehavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {
TPointerEventInfo,
} from '../../EventTypeDefs';
import { Point } from '../../Point';
import type { FabricObject } from '../Object/Object';
import type { FabricObject } from '../Object/FabricObject';
Copy link
Contributor

Choose a reason for hiding this comment

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

used only by onDeselect

import { Text } from '../Text/Text';
import { animate } from '../../util/animation/animate';
import type { TOnAnimationChangeCallback } from '../../util/animation/types';
Expand Down
6 changes: 4 additions & 2 deletions src/shapes/Object/InteractiveObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,9 +613,11 @@ export class InteractiveFabricObject<
* @param {TPointerEvent} [options.e] event if the process is generated by an event
* @param {FabricObject} [options.object] next object we are setting as active, and reason why
* this is being deselected

*/
onDeselect(options?: { e?: TPointerEvent; object?: FabricObject }): boolean {
onDeselect(options?: {
e?: TPointerEvent;
object?: InteractiveFabricObject;
}): boolean {
// implemented by sub-classes, as needed.
return false;
}
Expand Down
21 changes: 12 additions & 9 deletions src/typedefs.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// https://www.typescriptlang.org/docs/handbook/utility-types.html
import type { BaseFabricObject } from './EventTypeDefs';
import type { Gradient } from './gradient/Gradient';
import type { Pattern } from './Pattern';
import type { XY, Point } from './Point';
import type { FabricObject as BaseFabricObject } from './shapes/Object/Object';

interface NominalTag<T> {
nominalTag?: T;
Expand Down Expand Up @@ -97,20 +97,23 @@ export type TCacheCanvasDimensions = {

export type TRectBounds = [min: XY, max: XY];

export type TToCanvasElementOptions = {
export type TToCanvasElementOptions<
T extends BaseFabricObject = BaseFabricObject
> = {
left?: number;
top?: number;
width?: number;
height?: number;
filter?: (object: BaseFabricObject) => boolean;
filter?: (object: T) => boolean;
};

export type TDataUrlOptions = TToCanvasElementOptions & {
multiplier: number;
format?: ImageFormat;
quality?: number;
enableRetinaScaling?: boolean;
};
export type TDataUrlOptions<T extends BaseFabricObject = BaseFabricObject> =
TToCanvasElementOptions<T> & {
multiplier: number;
format?: ImageFormat;
quality?: number;
enableRetinaScaling?: boolean;
};

export type Abortable = {
/**
Expand Down