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

refactor(): canvas text editing manager #8543

Merged
merged 42 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
fb24d56
itext canvas selection handler
ShaMan123 Dec 29, 2022
8a171b0
safeguard from removing while editing
ShaMan123 Dec 29, 2022
518a565
Update canvas_events.ts
ShaMan123 Dec 29, 2022
2386835
Create CanvasTextEditingManager.ts
ShaMan123 Dec 29, 2022
8edb605
Update text.class.ts
ShaMan123 Dec 29, 2022
f56fccf
cleanup
ShaMan123 Dec 29, 2022
a95f579
imports
ShaMan123 Dec 29, 2022
0d3d9b5
adapt tests
ShaMan123 Dec 29, 2022
278cd4f
verify(): better mouse up
ShaMan123 Dec 29, 2022
73dcbfd
test(): exitEditing after removed from canvas
ShaMan123 Dec 29, 2022
2ac9554
Update CHANGELOG.md
ShaMan123 Dec 29, 2022
b9542b4
cleanup
ShaMan123 Dec 29, 2022
d0bed01
fix(): add/remove from _set canvas
ShaMan123 Dec 29, 2022
1dcbb4a
rename
ShaMan123 Dec 29, 2022
e07b9d0
TS
ShaMan123 Dec 29, 2022
e623a13
test
ShaMan123 Dec 29, 2022
cb4b08c
safeguard using static canvas
ShaMan123 Dec 29, 2022
8a88120
rename for clarity
ShaMan123 Dec 29, 2022
2030691
Merge branch 'master' into itext-canvas-handler
ShaMan123 Dec 29, 2022
98f7d23
Merge branch 'master' into itext-canvas-handler
ShaMan123 Jan 6, 2023
b5e02bb
fix(): destroy
ShaMan123 Jan 6, 2023
1260c0d
Update canvas_events.js
ShaMan123 Jan 6, 2023
0b15e06
prettier
ShaMan123 Jan 6, 2023
4211c2c
Merge branch 'master' into itext-canvas-handler
asturur Jan 9, 2023
1e7634e
Merge branch 'master' into itext-canvas-handler
asturur Jan 9, 2023
ace70a1
Update CHANGELOG.md
asturur Jan 9, 2023
4c290ac
note for later
ShaMan123 Jan 9, 2023
d9280b6
fix(): blur
ShaMan123 Jan 9, 2023
f00c36f
many fixes
ShaMan123 Jan 9, 2023
1460ed1
cleanup
ShaMan123 Jan 9, 2023
fcc6ec1
rm `__isMousedown`
ShaMan123 Jan 9, 2023
288041d
safety
ShaMan123 Jan 10, 2023
5fd48af
where did this go?
ShaMan123 Jan 10, 2023
904fd82
fix tests
ShaMan123 Jan 10, 2023
88604f2
fix
ShaMan123 Jan 10, 2023
c10282a
final touch up
ShaMan123 Jan 10, 2023
536f530
Merge branch 'master' into itext-canvas-handler
ShaMan123 Jan 10, 2023
a237a9a
rename
ShaMan123 Jan 11, 2023
2b8277c
revert blur fix
ShaMan123 Jan 11, 2023
900efdf
forgot test
ShaMan123 Jan 11, 2023
2a8d545
pretty
ShaMan123 Jan 11, 2023
25e5a9b
Merge branch 'master' into itext-canvas-handler
asturur Jan 12, 2023
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]

- refactor(Canvas, IText): Handle cross instance text editing states to an EditingManager class [#8543](https://github.com/fabricjs/fabric.js/pull/8543)
- chore(TS): move to export, babel, new rollup, change import statement for fabric. [#8585](https://github.com/fabricjs/fabric.js/pull/8585);
- chore(TS): Add declare in front of properties that are type definitions. [#8574](https://github.com/fabricjs/fabric.js/pull/8574)
- refactor(Animation): BREAKING: Animation api reduction and semplification (byValue is removed, '+=' syntax is removed, callbacks fired 100%) [#8547](https://github.com/fabricjs/fabric.js/pull/8547)
Expand Down
49 changes: 49 additions & 0 deletions src/canvas/TextEditingManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { TPointerEvent } from '../EventTypeDefs';
import type { IText } from '../shapes/itext.class';
import type { Textbox } from '../shapes/textbox.class';
import { removeFromArray } from '../util/internals';

/**
* In charge of synchronizing all interactive text instances of a canvas
*/
export class TextEditingManager {
private targets: (IText | Textbox)[] = [];
private target?: IText | Textbox;

exitTextEditing() {
this.target = undefined;
this.targets.forEach((target) => {
if (target.isEditing) {
target.exitEditing();
}
});
}

add(target: IText | Textbox) {
this.targets.push(target);
}

remove(target: IText | Textbox) {
this.unregister(target);
removeFromArray(this.targets, target);
}

register(target: IText | Textbox) {
this.target = target;
}

unregister(target: IText | Textbox) {
if (target === this.target) {
this.target = undefined;
}
}

onMouseMove(e: TPointerEvent) {
this.target?.isEditing && this.target.updateSelectionOnMouseMove(e);
}

dispose() {
this.targets = [];
this.target = undefined;
}
}
9 changes: 0 additions & 9 deletions src/canvas/canvas.class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,6 @@ export class SelectableCanvas<
protected declare _isCurrentlyDrawing: boolean;
declare freeDrawingBrush?: BaseBrush;
declare _activeObject?: FabricObject;
declare _hasITextHandlers?: boolean;
declare _iTextInstances: (IText | Textbox)[];
/**
* Constructor
* @param {HTMLCanvasElement | String} el canvas element to initialize instance on
Expand Down Expand Up @@ -1523,7 +1521,6 @@ export class SelectableCanvas<
super.destroy();
wrapperEl.removeChild(upperCanvasEl);
wrapperEl.removeChild(lowerCanvasEl);
this._iTextInstances = [];
this.contextCache = null;
this.contextTop = null;
cleanUpJsdomNode(upperCanvasEl);
Expand All @@ -1543,12 +1540,6 @@ export class SelectableCanvas<
// this.discardActiveGroup();
this.discardActiveObject();
this.clearContext(this.contextTop);
if (this._hasITextHandlers) {
// @ts-ignore
this.off('mouse:up', this._mouseUpITextHandler);
this._iTextInstances = [];
this._hasITextHandlers = false;
}
super.clear();
}

Expand Down
25 changes: 24 additions & 1 deletion src/canvas/canvas_events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
isInteractiveTextObject,
} from '../util/types';
import { SelectableCanvas } from './canvas.class';
import { TextEditingManager } from './TextEditingManager';

const RIGHT_CLICK = 3,
MIDDLE_CLICK = 2,
Expand Down Expand Up @@ -123,6 +124,8 @@ export class Canvas extends SelectableCanvas {
*/
declare _previousPointer: Point;

textEditingManager = new TextEditingManager();

/**
* Adds mouse listeners to canvas
* @private
Expand All @@ -133,7 +136,6 @@ export class Canvas extends SelectableCanvas {
// this is a workaround to having double listeners.
this.removeListeners();
this._bindEvents();
// @ts-ginore
this.addOrRemove(addListener, 'add');
}

Expand Down Expand Up @@ -1225,6 +1227,7 @@ export class Canvas extends SelectableCanvas {
} else {
this._transformObject(e);
}
this.textEditingManager.onMouseMove(e);
this._handleEvent(e, 'move');
this._resetTransformEventData();
}
Expand Down Expand Up @@ -1640,6 +1643,26 @@ export class Canvas extends SelectableCanvas {
this._groupSelector = null;
}

exitTextEditing() {
this.textEditingManager.exitTextEditing();
}

/**
* @override clear {@link textEditingManager}
*/
clear() {
this.textEditingManager.dispose();
super.clear();
}

/**
* @override clear {@link textEditingManager}
*/
destroy() {
super.destroy();
this.textEditingManager.dispose();
}

/**
* Clones canvas instance without cloning existing data.
* This essentially copies canvas dimensions since loadFromJSON does not affect canvas size.
Expand Down
110 changes: 11 additions & 99 deletions src/mixins/itext_behavior.mixin.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
// @ts-nocheck

import { getEnv } from '../env';
import { ObjectEvents, TEvent } from '../EventTypeDefs';
import { ObjectEvents, TEvent, TPointerEvent } from '../EventTypeDefs';
import { Point } from '../point.class';
import { Text } from '../shapes/text.class';
import { TPointerEvent } from '../typedefs';
ShaMan123 marked this conversation as resolved.
Show resolved Hide resolved
import { setStyle } from '../util/dom_style';
import { removeFromArray } from '../util/internals';
import { createCanvasElement } from '../util/misc/dom';
import { transformPoint } from '../util/misc/matrix';
import type { Canvas } from '../canvas/canvas_events';
import { TextStyleDeclaration } from './text_style.mixin';

// extend this regex to support non english languages
Expand Down Expand Up @@ -47,7 +44,6 @@ export abstract class ITextBehaviorMixin<
};
protected declare _currentCursorOpacity: number;
private declare _textBeforeEdit: string;
protected declare __isMousedown: boolean;
protected declare __selectionStartOnMouseDown: number;
private declare __dragImageDisposer: VoidFunction;
private declare __dragStartFired: boolean;
Expand Down Expand Up @@ -92,11 +88,10 @@ export abstract class ITextBehaviorMixin<
* Initializes all the interactive behavior of IText
*/
initBehavior() {
this.initAddedHandler();
this.initRemovedHandler();
this.initCursorSelectionHandlers();
this.initDoubleClickSimulation();
this.mouseMoveHandler = this.mouseMoveHandler.bind(this);
this.updateSelectionOnMouseMove =
this.updateSelectionOnMouseMove.bind(this);
this.dragEnterHandler = this.dragEnterHandler.bind(this);
this.dragOverHandler = this.dragOverHandler.bind(this);
this.dragLeaveHandler = this.dragLeaveHandler.bind(this);
Expand All @@ -114,62 +109,6 @@ export abstract class ITextBehaviorMixin<
this.selected = false;
}

/**
* Initializes "added" event handler
*/
initAddedHandler() {
this.on('added', (opt) => {
// make sure we listen to the canvas added event
const canvas = opt.target;
if (canvas) {
if (!canvas._hasITextHandlers) {
canvas._hasITextHandlers = true;
this._initCanvasHandlers(canvas);
}
canvas._iTextInstances = canvas._iTextInstances || [];
canvas._iTextInstances.push(this);
}
});
}

initRemovedHandler() {
this.on('removed', (opt) => {
// make sure we listen to the canvas removed event
const canvas = opt.target;
if (canvas) {
canvas._iTextInstances = canvas._iTextInstances || [];
removeFromArray(canvas._iTextInstances, this);
if (canvas._iTextInstances.length === 0) {
canvas._hasITextHandlers = false;
this._removeCanvasHandlers(canvas);
}
}
});
}

/**
* register canvas event to manage exiting on other instances
* @private
*/
_initCanvasHandlers(canvas: Canvas) {
canvas._mouseUpITextHandler = function () {
if (canvas._iTextInstances) {
canvas._iTextInstances.forEach((tObj) => {
tObj.__isMousedown = false;
});
}
};
canvas.on('mouse:up', canvas._mouseUpITextHandler);
}

/**
* remove canvas event to manage exiting on other instances
* @private
*/
_removeCanvasHandlers(canvas: Canvas) {
canvas.off('mouse:up', canvas._mouseUpITextHandler);
}

/**
* @private
*/
Expand All @@ -193,7 +132,7 @@ export abstract class ITextBehaviorMixin<
},
};

obj.animate('_currentCursorOpacity', targetOpacity, {
obj._animate('_currentCursorOpacity', targetOpacity, {
ShaMan123 marked this conversation as resolved.
Show resolved Hide resolved
duration: duration,
onComplete: function () {
if (!tickState.isAborted) {
Expand Down Expand Up @@ -448,7 +387,7 @@ export abstract class ITextBehaviorMixin<
}
if (this.canvas) {
this.canvas.calcOffset();
this.exitEditingOnOthers(this.canvas);
this.canvas.textEditingManager.exitTextEditing();
}

this.isEditing = true;
Expand All @@ -464,46 +403,21 @@ export abstract class ITextBehaviorMixin<
this._tick();
this.fire('editing:entered');
this._fireSelectionChanged();
if (!this.canvas) {
return this;
}
this.canvas.fire('text:editing:entered', { target: this });
this.initMouseMoveHandler();
this.canvas.requestRenderAll();
return this;
}

exitEditingOnOthers(canvas: Canvas) {
if (canvas._iTextInstances) {
canvas._iTextInstances.forEach((obj) => {
obj.selected = false;
if (obj.isEditing) {
obj.exitEditing();
}
});
if (this.canvas) {
this.canvas.fire('text:editing:entered', { target: this });
this.canvas.requestRenderAll();
}
}

/**
* Initializes "mousemove" event handler
*/
initMouseMoveHandler() {
this.canvas.on('mouse:move', this.mouseMoveHandler);
}

/**
* @private
* called by {@link canvas#textEditingManager}
*/
mouseMoveHandler(options) {
if (!this.__isMousedown || !this.isEditing) {
return;
}

updateSelectionOnMouseMove(e: TPointerEvent) {
// regain focus
getEnv().document.activeElement !== this.hiddenTextarea &&
this.hiddenTextarea.focus();

const newSelectionStart = this.getSelectionStartFromPointer(options.e),
const newSelectionStart = this.getSelectionStartFromPointer(e),
currentStart = this.selectionStart,
currentEnd = this.selectionEnd;
if (
Expand All @@ -524,7 +438,6 @@ export abstract class ITextBehaviorMixin<
this.selectionStart !== currentStart ||
this.selectionEnd !== currentEnd
) {
this.restartCursorIfNeeded();
Copy link
Contributor Author

@ShaMan123 ShaMan123 Jan 10, 2023

Choose a reason for hiding this comment

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

important fix
no reason to do this here, in case of a cursor the animation will be started from mouse up

this._fireSelectionChanged();
this._updateTextarea();
this.renderCursorOrSelection();
Expand Down Expand Up @@ -1065,7 +978,6 @@ export abstract class ITextBehaviorMixin<
this.fire('editing:exited');
isTextChanged && this.fire('modified');
if (this.canvas) {
this.canvas.off('mouse:move', this.mouseMoveHandler);
ShaMan123 marked this conversation as resolved.
Show resolved Hide resolved
this.canvas.fire('text:editing:exited', { target: this });
isTextChanged && this.canvas.fire('object:modified', { target: this });
}
Expand Down
24 changes: 12 additions & 12 deletions src/mixins/itext_click_behavior.mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export abstract class ITextClickBehaviorMixin<
return;
}

this.__isMousedown = true;
this.canvas.textEditingManager.register(this);

if (this.selected) {
this.inCompositionMode = false;
Expand Down Expand Up @@ -172,7 +172,17 @@ export abstract class ITextClickBehaviorMixin<
* @private
*/
mouseUpHandler(options: TransformEvent) {
this.__isMousedown = false;
if (this.canvas) {
this.canvas.textEditingManager.unregister(this);

const activeObject = this.canvas._activeObject;
if (activeObject && activeObject !== this) {
// avoid running this logic when there is an active object
// this because is possible with shift click and fast clicks,
// to rapidly deselect and reselect this object and trigger an enterEdit
return;
}
}
if (
!this.editable ||
(this.group && !this.group.interactive) ||
Expand All @@ -182,16 +192,6 @@ export abstract class ITextClickBehaviorMixin<
return;
}

if (this.canvas) {
const currentActive = this.canvas._activeObject;
if (currentActive && currentActive !== this) {
// avoid running this logic when there is an active object
// this because is possible with shift click and fast clicks,
// to rapidly deselect and reselect this object and trigger an enterEdit
return;
}
}

if (this.__lastSelected && !this.__corner) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this bit is really harsh, hard to understand logic

this.selected = false;
this.__lastSelected = false;
Expand Down
Loading