Split blocks #246

Nikitos9I · 2018-01-08T10:05:46Z

closes #229
closes #247

split is ready

khaydarov · 2018-01-08T10:17:12Z

src/components/modules/keyboard.js

+
+        super({config});
+
+        document.body.addEventListener('keydown', this.keyBoardListener.bind(this));


можно коллбэк передать так: (event) => { this.keyBoardListener(event) });

khaydarov · 2018-01-08T10:18:25Z

src/components/modules/keyboard.js

+
+        switch(event.keyCode) {
+
+            case (8):


в utils у нас есть константы:
_.keyCodes.ENTER, _.keyCodes.BACKSPACE ....

khaydarov · 2018-01-08T10:19:12Z

src/components/modules/keyboard.js

+
+            case (13):
+                console.log('enter pressed');
+


много пустых строк

khaydarov · 2018-01-08T10:20:11Z

src/components/modules/keyboard.js

+
+                event.preventDefault();
+
+                this.Editor.BlockManager.insert('text', this.getDataFromRange());


Мне кажется лучше добавить еще одну функцию, например, enterPressed()
а в ней уже делать insert. Просто возможно при нажатии на ентер будут происходить еще другие события.

khaydarov · 2018-01-08T10:20:50Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Gets data from blocks


Функцию стоит нормально задокументировать. Описать что происходит и как

khaydarov · 2018-01-08T10:21:54Z

src/components/modules/keyboard.js

+     */
+    getDataFromRange() {
+
+        let selection = window.getSelection();


Для работы с селекшном у нас есть класс, добавь его перед строчкой с exports
import Selection from '../Selection'

let selection = Selection.get()

khaydarov · 2018-01-08T10:22:53Z

src/components/modules/keyboard.js

+    getDataFromRange() {
+
+        let selection = window.getSelection();
+        let range = new Range();


можно через запятую объявить переменную, let selection= ..., range = new Range()

khaydarov · 2018-01-08T10:24:02Z

src/components/modules/keyboard.js

+        let selection = window.getSelection();
+        let range = new Range();
+
+        let cnt = this.Editor.BlockManager.currentBlock.pluginsContent,


название переменной не очень удачное. Можно javascript let pluginsContent = ....., lastNode = $.getDeepestNode(pluginsContent, true) -> описать почему true

khaydarov · 2018-01-08T10:26:42Z

src/components/modules/keyboard.js

+        selection.addRange(range);
+
+        let fragm = range.extractContents();
+        let div = document.createElement('div');


let extractedFragment = range.extractContenst(), wrapper = $.make('div', []. {}); wrapper.append(extractedFragment.cloneNode(true)); .....```

neSpecc · 2018-01-08T10:23:52Z

src/components/modules/keyboard.js

+
+        super({config});
+
+        document.body.addEventListener('keydown', this.keyBoardListener.bind(this));


document.body.addEventListener('keydown', event => this.keyBoardListener(event));

neSpecc · 2018-01-08T10:24:27Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * handler processes special keyboard keys


What does it mean?

neSpecc · 2018-01-08T10:25:59Z

src/components/modules/keyboard.js

+
+        switch(event.keyCode) {
+
+            case (8):


use Utils#keyCodes instead of Magic numbers

neSpecc · 2018-01-08T10:27:12Z

src/components/modules/keyboard.js

+
+                event.preventDefault();
+
+                this.Editor.BlockManager.insert('text', this.getDataFromRange());


new block will be inserted and the end of the Editor or below current block?

Ниже активного блока. Функция insert() сама это делает

neSpecc · 2018-01-08T10:28:27Z

src/components/modules/keyboard.js

+    /**
+     * Gets data from blocks
+     */
+    getDataFromRange() {


This method should be in the BlockManager class. And better name it split

khaydarov · 2018-01-08T10:36:11Z

А вообще, мне кажется будет правильнее сделать так:
модуль Caret отвечает за работу с кареткой, следовательно, вынести выделение и extract в метод каретки, например,

extractDataFromCaretPosition (или как-то так) () {
       // сюда перенести только выделение и extractContents 
      return fragment; // вернуть только вырезанный фрагмент
}

// В блок-менеджере будет функция split

split() {
     let extractedFragment = this.Editor.Caret.extractFromCaretPosition();
    
     // создание data 
    this.insert('text', data);
}

В keyboards:

enterPressed() {
    
    this.Editor.BlockManager.split(); 

   // дальше делает что-то другое, например, переносим тулбар

}

khaydarov · 2018-01-08T14:03:14Z

src/components/modules/blockManager.js

+        selection.removeAllRanges();
+        selection.addRange(range);
+
+        let extractedFragment = range.extractContents(),


нужно в модуле Caret создать универсальную функцию, которая вырежет любой фрагмент из блока. Для начала можно просто создать extractFragmentFromCaretPosition() и вынести код до extractContents в эту функцию (extractFragmentFromCaretPosition)

khaydarov · 2018-01-08T14:03:53Z

src/components/modules/keyboard.js

+
+        super({config});
+
+        document.body.addEventListener('keydown', event => this.keyBoardListener(event));


javascript this.Editor.Listener.on(document.body, 'keydown', event => this.keyBoardListener(event));

khaydarov · 2018-01-08T14:05:04Z

src/components/modules/keyboard.js

+            case _.keyCodes.ENTER:
+
+                _.log('Enter key pressed');
+                event.preventDefault();


preventDefault лучше занести в enterPressed(). Не всегда нужно прерывать дефолтное поведение

khaydarov · 2018-01-08T14:06:47Z

src/components/modules/keyboard.js

+            case _.keyCodes.RIGHT:
+
+                _.log('Right key pressed');
+                this.Editor.BlockManager.navigateNext();


case _.keyCodes.DOWN: case _.keyCodes.RIGHT: _.log('Right and DOWN key pressed'); this.arrowRightAndDownPressed(); break; }

arrowRightAndDownPressed() { this.Editor.BlockManager.navigateNext(); }

то же самое с UP и LEFT

khaydarov · 2018-01-08T15:41:45Z

src/components/modules/blockManager.js

        let block = this.composeBlock(toolName, data);

        this._blocks[++this.currentBlockIndex] = block;
        this.Editor.Caret.setToBlock(block);

    }

+    /**
+     * Insert extract content form current block to below current block


insert extracted content from current block to block that is below

or next to the current

khaydarov · 2018-01-08T15:43:07Z

src/components/modules/caret.js

@@ -116,4 +116,25 @@ export default class Caret extends Module {

    }

+    /**
+     * Extract fragment of content current block form caret position


Extract content fragment of current block from caret position

khaydarov · 2018-01-08T15:43:39Z

src/components/modules/caret.js

+        let selection = Selection.get(),
+            range = new Range();
+
+        let cnt = this.Editor.BlockManager.currentBlock.pluginsContent,


cnt -> pluginsContent

khaydarov · 2018-01-08T15:44:13Z

src/components/modules/keyboard.js

@@ -0,0 +1,98 @@
+import Selection from '../Selection';


импорт тут значит не нужен, и это отлично

gohabereg · 2018-01-08T15:52:49Z

src/components/modules/blockManager.js

        let block = this.composeBlock(toolName, data);

        this._blocks[++this.currentBlockIndex] = block;
        this.Editor.Caret.setToBlock(block);

    }

+    /**
+     * Insert extract content form current block to block that is below


extracted

и вроде можно убрать that is

И коммент немного не понятный.
Вставляем контент из текущего блока в следующий. Разве этим split занимается?

gohabereg · 2018-01-08T15:53:12Z

src/components/modules/blockManager.js

+        let extractedFragment = this.Editor.Caret.extractFragmentFromCaretPosition(),
+            wrapper = $.make('div');
+
+        wrapper.append(extractedFragment.cloneNode(true));


Поясни зачем клонирование

gohabereg · 2018-01-08T15:54:50Z

src/components/modules/blockManager.js

+
+        wrapper.append(extractedFragment.cloneNode(true));
+
+        let data = {


Оставь здесь @todo

gohabereg · 2018-01-08T15:55:52Z

src/components/modules/caret.js

+            range = new Range();
+
+        let pluginsContent = this.Editor.BlockManager.currentBlock.pluginsContent,
+            lastNode = $.getDeepestNode(pluginsContent, true);


Коммент, что означает true

gohabereg · 2018-01-08T15:56:43Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Should be called after Editor.BlockManager preparation


А почем именно после менеджера?

gohabereg · 2018-01-08T15:57:49Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Handler on Editor for keyboard keys


Не понятно что ловишь. keyup, keydown или еще что.

gohabereg · 2018-01-08T15:58:13Z

src/components/modules/keyboard.js

+     */
+    prepare() {
+
+        this.Editor.Listeners.on(document.body, 'keydown', event => {


Точно необходимо вешать на body?

gohabereg · 2018-01-08T15:58:50Z

src/components/modules/keyboard.js

+     */
+    enterPressed(event) {
+
+        event.preventDefault();


Оставь @todo, надо будет проверять настройку плагина allowLinebreaks

gohabereg · 2018-01-08T15:59:08Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Hand right and down keyboard keys


gohabereg · 2018-01-08T16:00:15Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Insert new block with data below current block


Функция же не только для этого

neSpecc · 2018-01-08T16:08:16Z

src/components/modules/keyboard.js

+     */
+    prepare() {
+
+        this.Editor.Listeners.on(document.body, 'keydown', event => {


раньше это событие вешалось на блок, а не на весь документ. Это точно корректное поведение?

меня это тоже смущает, потому что один keydown на весь редактор. Можно это сделать при формировании блока (в BlockManager#composeBlock) - вешать событие здесь.

gohabereg · 2018-01-08T19:24:54Z

src/components/modules/blockManager.js

        let block = this.composeBlock(toolName, data);

        this._blocks[++this.currentBlockIndex] = block;
        this.Editor.Caret.setToBlock(block);

    }

+    /**
+     * Create new block below current block and insert extracted content form current block to new block


Очень сложно. В таких случаях лучше писать прям по пунктам

Split current block 1. Extract content from caret position to block`s end 2. Insert new block below current one with extracted content

gohabereg · 2018-01-08T19:26:36Z

src/components/modules/caret.js

@@ -116,4 +116,28 @@ export default class Caret extends Module {

    }

+    /**
+     * Extract content fragment of current block form caret position


... from caret position to the end of the block

gohabereg · 2018-01-08T19:27:34Z

src/components/modules/keyboard.js

+ * @class Keyboard
+ * @classdesc Сlass to handle the keystrokes
+ *
+ * @module Keyboard


Это можно убрать

gohabereg · 2018-01-08T19:27:51Z

src/components/modules/keyboard.js

+ * @author CodeX Team (team@ifmo.su)
+ * @copyright CodeX Team 2017
+ * @license The MIT License (MIT)
+ * @version 2.0.0


Это новый класс, 1 версия

gohabereg · 2018-01-08T19:28:36Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Handler on Editor for keyboard keys at keydown event


Что такое Editor в этом контексте?
И обработчик ты на блок же вешаешь

gohabereg · 2018-01-08T19:29:46Z

src/components/modules/keyboard.js

+    enterPressed(event) {
+
+        /**
+         * @todo check settings of "allowLinebreaks" plugin


Что-то не понятно, что написано
check plugin`s configuration for allowLinebreaks property

gohabereg · 2018-01-08T19:30:48Z

src/components/modules/keyboard.js

+         */
+        event.preventDefault();
+        /**
+         * Insert new block with data below current block


Оставь просто split current block into two ones

neSpecc · 2018-01-08T21:07:20Z

src/components/modules/blockManager.js

-                break;
-
-        }
+        this.Editor.Listeners.on(block.pluginsContent, 'keydown', (event) => this.Editor.Keyboard.keyboardListener(event));


blockKeydownsListener

neSpecc · 2018-01-08T21:07:43Z

src/components/modules/blockManager.js

        let block = this.composeBlock(toolName, data);

        this._blocks[++this.currentBlockIndex] = block;
        this.Editor.Caret.setToBlock(block);

    }

+    /**
+     * Split current block 
+     * 1. Extract content from caret position to block`s end


Caret , Block

neSpecc · 2018-01-08T21:08:37Z

src/components/modules/blockManager.js

+    /**
+     * Split current block 
+     * 1. Extract content from caret position to block`s end
+     * 2. Insert new block below current one with extracted content


a new Block

neSpecc · 2018-01-08T21:09:02Z

src/components/modules/blockManager.js

+        wrapper.append(extractedFragment);
+
+        /**
+         * @todo make object in accordance with the plugin


neSpecc · 2018-01-08T21:09:21Z

src/components/modules/blockManager.js

+            text: wrapper.innerHTML,
+        };
+
+        this.insert('text', data);


inititalTool

neSpecc · 2018-01-08T21:09:44Z

src/components/modules/caret.js

@@ -116,4 +116,28 @@ export default class Caret extends Module {

    }

+    /**
+     * Extract content fragment of current block from caret position to the end of the block


Block, Caret,

neSpecc · 2018-01-08T21:12:11Z

src/components/modules/keyboard.js

@@ -0,0 +1,103 @@
+/**
+ * @class Keyboard
+ * @classdesc Сlass to handle the keystrokes


neSpecc · 2018-01-08T21:12:28Z

src/components/modules/keyboard.js

+    }
+
+    /**
+     * Handler on block for keyboard keys at keydown event


neSpecc · 2018-01-08T21:13:05Z

src/components/modules/keyboard.js

+    enterPressed(event) {
+
+        /**
+         * @todo check plugin`s configuration for allowLinebreaks property


neSpecc · 2018-01-08T21:14:13Z

src/components/modules/keyboard.js

+         */
+        event.preventDefault();
+        /**
+         * Split current block into two ones


Split the Current Block.

neSpecc

Некорректное поведение при попытке сплита с выделенным текстом (Safari)
https://giphy.com/gifs/3oFzmnols9OJd6AKPe

Nikita added 7 commits January 5, 2018 12:50

split func upd

4f8d089

split blocks update

1a9167f

Merge branch 'rewriting-version2.0' into Split-Merge-blocks

ef24b23

split is ready

342705b

start to make merge

d46e823

upd

97af4c8

split general upd

6a4de5f

split is ready

Nikitos9I requested review from gohabereg, khaydarov, neSpecc and talyguryn as code owners January 8, 2018 10:05

khaydarov requested changes Jan 8, 2018

View reviewed changes

neSpecc requested changes Jan 8, 2018

View reviewed changes

Nikita added 2 commits January 8, 2018 16:48

keyboard module update

df75c79

BlockManager removed keyboard handler

45dabb7

khaydarov requested changes Jan 8, 2018

View reviewed changes

Nikita added 3 commits January 8, 2018 17:30

commit before merging rewriting2.0

0869a64

Merge branch 'rewriting-version2.0' into Split-Merge-blocks

543b73a

general upd split

0c4c33e

khaydarov requested changes Jan 8, 2018

View reviewed changes

Nikitos9I changed the title ~~Split merge blocks~~ Split blocks Jan 8, 2018

Documentation upd

92a316e

khaydarov approved these changes Jan 8, 2018

View reviewed changes

gohabereg requested changes Jan 8, 2018

View reviewed changes

khaydarov assigned Nikitos9I Jan 8, 2018

neSpecc requested changes Jan 8, 2018

View reviewed changes

document + listener upd

909080a

codex-team deleted a comment from gohabereg Jan 8, 2018

upd doc

b4d9485

gohabereg requested changes Jan 8, 2018

View reviewed changes

documentation upd

740349c

gohabereg approved these changes Jan 8, 2018

View reviewed changes

neSpecc requested changes Jan 8, 2018

View reviewed changes

Nikita added 2 commits January 9, 2018 00:28

doc upd

05b7663

listener upd

2cc8578

neSpecc requested changes Jan 9, 2018

View reviewed changes

Nikitos9I added 3 commits January 11, 2018 20:05

update algh extract fragm

fe5c99f

upd extractRangeContent

25f5b1d

upd dom.js

a4099ca

khaydarov approved these changes Jan 11, 2018

View reviewed changes

neSpecc approved these changes Jan 11, 2018

View reviewed changes

gohabereg approved these changes Jan 11, 2018

View reviewed changes

Nikitos9I merged commit c73ce70 into rewriting-version2.0 Jan 11, 2018

Nikitos9I deleted the Split-Merge-blocks branch January 11, 2018 17:45

neSpecc mentioned this pull request Jan 12, 2018

Split block #247

Closed

Nikitos9I restored the Split-Merge-blocks branch January 12, 2018 13:13


		super({config});

		document.body.addEventListener('keydown', this.keyBoardListener.bind(this));


		event.preventDefault();

		this.Editor.BlockManager.insert('text', this.getDataFromRange());


		wrapper.append(extractedFragment.cloneNode(true));

		let data = {

Split blocks #246

Split blocks #246

Conversation

Nikitos9I commented Jan 8, 2018 • edited by neSpecc Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khaydarov Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikitos9I Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

khaydarov commented Jan 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neSpecc left a comment

Choose a reason for hiding this comment

Nikitos9I commented Jan 8, 2018 •

edited by neSpecc

Loading

khaydarov Jan 8, 2018 •

edited

Loading

Nikitos9I Jan 8, 2018 •

edited

Loading

khaydarov commented Jan 8, 2018 •

edited

Loading