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 behaviour of inputs editing in block settings #1123

Merged
merged 11 commits into from
Jun 2, 2020
2 changes: 1 addition & 1 deletion dist/editor.js

Large diffs are not rendered by default.

19 changes: 1 addition & 18 deletions src/components/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,24 +191,7 @@ export default class Block {
return this.cachedInputs;
}

const content = this.holder;
const allowedInputTypes = ['text', 'password', 'email', 'number', 'search', 'tel', 'url'];

const selector = '[contenteditable], textarea, input:not([type]), ' +
allowedInputTypes.map((type) => `input[type="${type}"]`).join(', ');

let inputs = _.array(content.querySelectorAll(selector));

/**
* If contenteditable element contains block elements, treat them as inputs.
*/
inputs = inputs.reduce((result, input) => {
if ($.isNativeInput(input) || $.containsOnlyInlineElements(input)) {
return [...result, input];
}

return [...result, ...$.getDeepestBlockElements(input)];
}, []);
const inputs = $.findAllInputs(this.holder);

/**
* If inputs amount was changed we need to check if input index is bigger then inputs array length
Expand Down
26 changes: 26 additions & 0 deletions src/components/dom.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as _ from "./utils";

/**
* DOM manipulations helper
*/
Expand Down Expand Up @@ -194,6 +196,30 @@ export default class Dom {
return el.querySelectorAll(selector);
}

/**
* Find all contendeditable, textarea and editable input elements passed holder contains
*
* @param holder - element where to find inputs
*/
public static findAllInputs(holder: Element): HTMLElement[] {
const allowedInputTypes = ['text', 'password', 'email', 'number', 'search', 'tel', 'url'];

const selector = '[contenteditable], textarea, input:not([type]), ' +
allowedInputTypes.map((type) => `input[type="${type}"]`).join(', ');

return _.array(holder.querySelectorAll(selector))
/**
* If contenteditable element contains block elements, treat them as inputs.
*/
.reduce((result, input) => {
if (Dom.isNativeInput(input) || Dom.containsOnlyInlineElements(input)) {
return [...result, input];
}

return [...result, ...Dom.getDeepestBlockElements(input)];
}, []);
}

/**
* Search for deepest node which is Leaf.
* Leaf is the vertex that doesn't have any child nodes
Expand Down
3 changes: 2 additions & 1 deletion src/components/domIterator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ export default class DomIterator {
focusedButtonIndex = (this.items.length + focusedButtonIndex - 1) % this.items.length;
}

if (Dom.isNativeInput(this.items[focusedButtonIndex])) {
if (Dom.canSetCaret(this.items[focusedButtonIndex])) {

Copy link
Member

Choose a reason for hiding this comment

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

there can be contenteditable element, so you need to use custom focus() method

/**
* Focus input
*/
Expand Down
127 changes: 66 additions & 61 deletions src/components/flipper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface FlipperOptions {
/**
* Optional callback for button click
*/
activateCallback?: () => void;
activateCallback?: (item: HTMLElement) => void;
}

/**
Expand Down Expand Up @@ -62,7 +62,7 @@ export default class Flipper {
/**
* Call back for button click/enter
*/
private readonly activateCallback: () => void;
private readonly activateCallback: (item: HTMLElement) => void;

/**
* @class
Expand All @@ -73,44 +73,6 @@ export default class Flipper {
this.allowArrows = typeof options.allowArrows === 'boolean' ? options.allowArrows : true;
this.iterator = new DomIterator(options.items, options.focusedItemClass);
this.activateCallback = options.activateCallback;

/**
* Listening all keydowns on document and react on TAB/Enter press
* TAB will leaf iterator items
* ENTER will click the focused item
*/
document.addEventListener('keydown', (event) => {
const isReady = this.isEventReadyForHandling(event);

if (!isReady) {
return;
}

/**
* Prevent only used keys default behaviour
* (allows to navigate by ARROW DOWN, for example)
*/
if (Flipper.usedKeys.includes(event.keyCode)) {
event.preventDefault();
}

switch (event.keyCode) {
case _.keyCodes.TAB:
this.handleTabPress(event);
break;
case _.keyCodes.LEFT:
case _.keyCodes.UP:
this.flipLeft();
break;
case _.keyCodes.RIGHT:
case _.keyCodes.DOWN:
this.flipRight();
break;
case _.keyCodes.ENTER:
this.handleEnterPress(event);
break;
}
}, false);
}

/**
Expand Down Expand Up @@ -141,6 +103,13 @@ export default class Flipper {
if (items) {
this.iterator.setItems(items);
}

/**
* Listening all keydowns on document and react on TAB/Enter press
* TAB will leaf iterator items
* ENTER will click the focused item
*/
document.addEventListener('keydown', this.onKeyDown);
}

/**
Expand All @@ -149,6 +118,8 @@ export default class Flipper {
public deactivate(): void {
this.activated = false;
this.dropCursor();

document.removeEventListener('keydown', this.onKeyDown);
}

/**
Expand All @@ -168,6 +139,20 @@ export default class Flipper {
this.flipRight();
}

/**
* Focuses previous flipper iterator item
*/
public flipLeft(): void {
this.iterator.previous();
}

/**
* Focuses next flipper iterator item
*/
public flipRight(): void {
this.iterator.next();
}

/**
* Drops flipper's iterator cursor
*
Expand All @@ -177,6 +162,44 @@ export default class Flipper {
this.iterator.dropCursor();
}

/**
* KeyDown event handler
*
* @param event - keydown event
*/
private onKeyDown = (event): void => {
const isReady = this.isEventReadyForHandling(event);

if (!isReady) {
return;
}

/**
* Prevent only used keys default behaviour
* (allows to navigate by ARROW DOWN, for example)
*/
if (Flipper.usedKeys.includes(event.keyCode)) {
event.preventDefault();
}

switch (event.keyCode) {
case _.keyCodes.TAB:
this.handleTabPress(event);
break;
case _.keyCodes.LEFT:
case _.keyCodes.UP:
this.flipLeft();
break;
case _.keyCodes.RIGHT:
case _.keyCodes.DOWN:
this.flipRight();
break;
case _.keyCodes.ENTER:
this.handleEnterPress(event);
break;
}
};

/**
* This function is fired before handling flipper keycodes
* The result of this function defines if it is need to be handled or not
Expand All @@ -190,7 +213,7 @@ export default class Flipper {
_.keyCodes.ENTER,
];

if (this.allowArrows) {
if (this.allowArrows && this.iterator.currentItem !== document.activeElement) {
Copy link
Member

Choose a reason for hiding this comment

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

need to describe this case or move the second condition to the variable

handlingKeyCodeList.push(
_.keyCodes.LEFT,
_.keyCodes.RIGHT,
Expand All @@ -199,11 +222,7 @@ export default class Flipper {
);
}

if (!this.activated || handlingKeyCodeList.indexOf(event.keyCode) === -1) {
return false;
}

return true;
return (this.activated && handlingKeyCodeList.indexOf(event.keyCode) !== -1);
Copy link
Member

Choose a reason for hiding this comment

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

extra parentheses

}

/**
Expand All @@ -226,20 +245,6 @@ export default class Flipper {
}
}

/**
* Focuses previous flipper iterator item
*/
private flipLeft(): void {
this.iterator.previous();
}

/**
* Focuses next flipper iterator item
*/
private flipRight(): void {
this.iterator.next();
}

/**
* Enter press will click current item if flipper is activated
*
Expand All @@ -255,7 +260,7 @@ export default class Flipper {
}

if (typeof this.activateCallback === 'function') {
this.activateCallback();
this.activateCallback(this.iterator.currentItem);
}

event.preventDefault();
Expand Down
28 changes: 0 additions & 28 deletions src/components/modules/blockEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ export default class BlockEvents extends Module {
case _.keyCodes.TAB:
this.tabPressed(event);
break;

case _.keyCodes.ESC:
this.escapePressed(event);
break;
}
}

Expand Down Expand Up @@ -158,30 +154,6 @@ export default class BlockEvents extends Module {
}
}

/**
* Escape pressed
* If some of Toolbar components are opened, then close it otherwise close Toolbar
*
* @param {Event} event - escape keydown event
*/
public escapePressed(event): void {
/**
* Clear blocks selection by ESC
*/
this.Editor.BlockSelection.clearSelection(event);

if (this.Editor.Toolbox.opened) {
this.Editor.Toolbox.close();
} else if (this.Editor.BlockSettings.opened) {
this.Editor.BlockSettings.close();
} else if (this.Editor.ConversionToolbar.opened) {
this.Editor.ConversionToolbar.close();
} else if (this.Editor.InlineToolbar.opened) {
this.Editor.InlineToolbar.close();
} else {
this.Editor.Toolbar.close();
}
}

/**
* Add drop target styles
Expand Down
7 changes: 5 additions & 2 deletions src/components/modules/blockSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,17 +188,20 @@ export default class BlockSelection extends Module {
this.nativeInputSelected = false;
this.readyToBlockSelection = false;

const isKeyboard = reason && (reason instanceof KeyboardEvent);
const isPrintableKey = isKeyboard && _.isPrintableKey((reason as KeyboardEvent).keyCode);

/**
* If reason caused clear of the selection was printable key and any block is selected,
* remove selected blocks and insert pressed key
*/
if (this.anyBlockSelected && reason && reason instanceof KeyboardEvent && _.isPrintableKey(reason.keyCode)) {
if (this.anyBlockSelected && isKeyboard && isPrintableKey && !SelectionUtils.exists) {
Copy link
Member

Choose a reason for hiding this comment

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

SelectionUtils.exists - bad naming. Exists what?

const indexToInsert = BlockManager.removeSelectedBlocks();

BlockManager.insertInitialBlockAtIndex(indexToInsert, true);
Caret.setToBlock(BlockManager.currentBlock);
_.delay(() => {
Caret.insertContentAtCaretPosition(reason.key);
Caret.insertContentAtCaretPosition((reason as KeyboardEvent).key);
}, 20)();
}

Expand Down
Loading