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

chore(TS): fix type of modifed event that could cause unexpected behaviour in dev code #9596

Merged
merged 5 commits into from
Jan 14, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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): fix type of modifed event that could cause unexpected behaviour in dev code [#9596](https://github.com/fabricjs/fabric.js/pull/9596)
- fix(LayoutManager): remove unnecessary check [#9591](https://github.com/fabricjs/fabric.js/pull/9591)
- fix(Text) Fix style transfer issue on a line that is not empty [#9461](https://github.com/fabricjs/fabric.js/pull/9461)
- ci(): add a vue template [#9502](https://github.com/fabricjs/fabric.js/pull/9502)
Expand Down
6 changes: 4 additions & 2 deletions src/EventTypeDefs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export type TModificationEvents =
| 'resizing'
| 'modifyPoly';

export interface ModifiedEvent<E extends Event = TPointerEvent>
extends TEvent<E> {
export interface ModifiedEvent<E extends Event = TPointerEvent> {
e?: E;
transform: Transform;
target: FabricObject;
action?: string;
Expand All @@ -123,6 +123,8 @@ type ObjectModificationEvents = ModificationEventsSpec;
type CanvasModificationEvents = ModificationEventsSpec<
'object:',
BasicTransformEvent & { target: FabricObject },
// TODO: this typing makes not possible to use properties from modified event
// in object:modified
ModifiedEvent | { target: FabricObject }
> & {
'before:transform': TEvent & { transform: Transform };
Expand Down
42 changes: 0 additions & 42 deletions src/canvas/Canvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -920,48 +920,6 @@ export class Canvas extends SelectableCanvas implements CanvasOptions {
}
}

/**
* End the current transform.
* You don't usually need to call this method unless you are interrupting a user initiated transform
* because of some other event ( a press of key combination, or something that block the user UX )
* @param {Event} [e] send the mouse event that generate the finalize down, so it can be used in the event
*/
endCurrentTransform(e: TPointerEvent) {
const transform = this._currentTransform;
this._finalizeCurrentTransform(e);
if (transform && transform.target) {
// this could probably go inside _finalizeCurrentTransform
transform.target.isMoving = false;
}
this._currentTransform = null;
}

/**
* @private
* @param {Event} e send the mouse event that generate the finalize down, so it can be used in the event
*/
_finalizeCurrentTransform(e: TPointerEvent) {
const transform = this._currentTransform!,
target = transform.target,
options = {
e,
target,
transform,
action: transform.action,
};

if (target._scaling) {
target._scaling = false;
}

target.setCoords();

if (transform.actionPerformed) {
this.fire('object:modified', options);
target.fire('modified', options);
}
}

/**
* @private
* @param {Event} e Event object fired on mousedown
Expand Down
43 changes: 42 additions & 1 deletion src/canvas/SelectableCanvas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,6 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
return false;
}
if (this._currentTransform && this._currentTransform.target === obj) {
// @ts-expect-error this method exists in the subclass - should be moved or declared as abstract
this.endCurrentTransform(e);
}
this._activeObject = undefined;
Expand Down Expand Up @@ -1188,6 +1187,48 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
return discarded;
}

/**
* End the current transform.
* You don't usually need to call this method unless you are interrupting a user initiated transform
* because of some other event ( a press of key combination, or something that block the user UX )
* @param {Event} [e] send the mouse event that generate the finalize down, so it can be used in the event
*/
endCurrentTransform(e?: TPointerEvent) {
const transform = this._currentTransform;
this._finalizeCurrentTransform(e);
if (transform && transform.target) {
// this could probably go inside _finalizeCurrentTransform
transform.target.isMoving = false;
}
this._currentTransform = null;
}

/**
* @private
* @param {Event} e send the mouse event that generate the finalize down, so it can be used in the event
*/
_finalizeCurrentTransform(e?: TPointerEvent) {
const transform = this._currentTransform!,
target = transform.target,
options = {
e,
target,
transform,
action: transform.action,
};

if (target._scaling) {
target._scaling = false;
}

target.setCoords();

if (transform.actionPerformed) {
this.fire('object:modified', options);
target.fire('modified', options);
}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

moved so typescript can check

/**
* Sets viewport transformation of this canvas instance
* @param {Array} vpt a Canvas 2D API transform matrix
Expand Down
7 changes: 4 additions & 3 deletions src/shapes/IText/ITextBehavior.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { SerializedTextProps, TextProps } from '../Text/Text';
import type { TOptions } from '../../typedefs';
import { getDocumentFromElement } from '../../util/dom_misc';
import { LEFT, RIGHT, reNewline } from '../../constants';
import type { Canvas } from '../../canvas/Canvas';
import type { IText } from './IText';

/**
* extend this regex to support non english languages
Expand Down Expand Up @@ -690,8 +690,9 @@ export abstract class ITextBehavior<
this.fire('editing:exited');
isTextChanged && this.fire('modified');
if (this.canvas) {
// @ts-expect-error in reality it is an IText instance
this.canvas.fire('text:editing:exited', { target: this });
this.canvas.fire('text:editing:exited', {
target: this as unknown as IText,
});
isTextChanged && this.canvas.fire('object:modified', { target: this });
}
return this;
Expand Down
Loading