From a6b069603f7aa21254e9dd9ad331c14eff35ba95 Mon Sep 17 00:00:00 2001 From: Justin Weiss Date: Tue, 9 Jan 2018 11:13:44 -0800 Subject: [PATCH 1/2] Move selection to the offset of remove_text if it's in its bounds If selection is inside a block of text that's removed, the part of the selection that's inside the bounds should clamp to the offset of the removed text. So if your cursor is at offset 5, and you're deleting from 2-10, your cursor should end up at offset 2. --- packages/slate/src/operations/apply.js | 16 +++++-- packages/slate/test/index.js | 1 + .../remove_text/cursor-inside-removed-text.js | 33 +++++++++++++++ packages/slate/test/operations/index.js | 42 +++++++++++++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js create mode 100644 packages/slate/test/operations/index.js diff --git a/packages/slate/src/operations/apply.js b/packages/slate/src/operations/apply.js index 067d6d31a..9fe5a608e 100644 --- a/packages/slate/src/operations/apply.js +++ b/packages/slate/src/operations/apply.js @@ -287,12 +287,20 @@ const APPLIERS = { const { anchorKey, focusKey, anchorOffset, focusOffset } = selection let node = document.assertPath(path) - if (anchorKey == node.key && anchorOffset >= rangeOffset) { - selection = selection.moveAnchor(-length) + if (anchorKey == node.key) { + if (anchorOffset >= rangeOffset) { + selection = selection.moveAnchor(-length) + } else if (anchorOffset > offset) { + selection = selection.moveAnchorTo(anchorKey, offset) + } } - if (focusKey == node.key && focusOffset >= rangeOffset) { - selection = selection.moveFocus(-length) + if (focusKey == node.key) { + if (focusOffset >= rangeOffset) { + selection = selection.moveFocus(-length) + } else if (focusOffset > offset) { + selection = selection.moveFocusTo(focusKey, offset) + } } node = node.removeText(offset, length) diff --git a/packages/slate/test/index.js b/packages/slate/test/index.js index 6871abd08..bfc424731 100644 --- a/packages/slate/test/index.js +++ b/packages/slate/test/index.js @@ -20,6 +20,7 @@ describe('slate', () => { require('./schema') require('./changes') require('./history') + require('./operations') }) /** diff --git a/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js b/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js new file mode 100644 index 000000000..a7dd0c7fa --- /dev/null +++ b/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js @@ -0,0 +1,33 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default function (change) { + return change.applyOperation({ + type: 'remove_text', + path: [0, 0], + offset: 2, + text: 'is is some text inside ', + marks: [] + }) +} + +export const input = ( + + + + This is some text inside a paragraph. + + + +) + +export const output = ( + + + + Tha paragraph. + + + +) diff --git a/packages/slate/test/operations/index.js b/packages/slate/test/operations/index.js new file mode 100644 index 000000000..7a74a4049 --- /dev/null +++ b/packages/slate/test/operations/index.js @@ -0,0 +1,42 @@ + +import assert from 'assert' +import fs from 'fs-promise' // eslint-disable-line import/no-extraneous-dependencies +import toCamel from 'to-camel-case' // eslint-disable-line import/no-extraneous-dependencies +import { basename, extname, resolve } from 'path' + +/** + * Tests. + */ + +describe('operations', async () => { + const dir = resolve(__dirname) + const categories = fs.readdirSync(dir).filter(c => c[0] != '.' && c != 'index.js') + + for (const category of categories) { + describe(category, () => { + const categoryDir = resolve(dir, category) + const methods = fs.readdirSync(categoryDir).filter(c => c[0] != '.') + + for (const method of methods) { + describe(toCamel(method), () => { + const testDir = resolve(categoryDir, method) + const tests = fs.readdirSync(testDir).filter(t => t[0] != '.' && !!~t.indexOf('.js')).map(t => basename(t, extname(t))) + + for (const test of tests) { + it(test, async () => { + const module = require(resolve(testDir, test)) + const { input, output } = module + const fn = module.default + const change = input.change() + fn(change) + const opts = { preserveSelection: true, preserveData: true } + const actual = change.value.toJSON(opts) + const expected = output.toJSON(opts) + assert.deepEqual(actual, expected) + }) + } + }) + } + }) + } +}) From 4781a0ea66f62a68aedf0a5806a56970835dca19 Mon Sep 17 00:00:00 2001 From: Justin Weiss Date: Wed, 10 Jan 2018 15:36:09 -0800 Subject: [PATCH 2/2] Change operation tests to take a list of operations --- .../remove_text/cursor-inside-removed-text.js | 16 +++++++--------- packages/slate/test/operations/index.js | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js b/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js index a7dd0c7fa..cfc446704 100644 --- a/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js +++ b/packages/slate/test/operations/apply/remove_text/cursor-inside-removed-text.js @@ -2,15 +2,13 @@ import h from '../../../helpers/h' -export default function (change) { - return change.applyOperation({ - type: 'remove_text', - path: [0, 0], - offset: 2, - text: 'is is some text inside ', - marks: [] - }) -} +export default [{ + type: 'remove_text', + path: [0, 0], + offset: 2, + text: 'is is some text inside ', + marks: [] +}] export const input = ( diff --git a/packages/slate/test/operations/index.js b/packages/slate/test/operations/index.js index 7a74a4049..1615d21c6 100644 --- a/packages/slate/test/operations/index.js +++ b/packages/slate/test/operations/index.js @@ -26,9 +26,9 @@ describe('operations', async () => { it(test, async () => { const module = require(resolve(testDir, test)) const { input, output } = module - const fn = module.default + const operations = module.default const change = input.change() - fn(change) + change.applyOperations(operations) const opts = { preserveSelection: true, preserveData: true } const actual = change.value.toJSON(opts) const expected = output.toJSON(opts)