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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- chore(TS): Replace BaseFabricObject with FabricObject [#9016](https://github.com/fabricjs/fabric.js/pull/9016)
- ci(): Revert "invoke tests after changelog action (#8974)" [#9013](https://github.com/fabricjs/fabric.js/pull/9013)
- fix(IText): empty line selection [#9019](https://github.com/fabricjs/fabric.js/pull/9019)
- ci(): Added playwright testing [#8616](https://github.com/fabricjs/fabric.js/pull/8616)
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
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