From c500becf817bb3e17ad0e5b556922fb0d61a2bd9 Mon Sep 17 00:00:00 2001 From: jasonphillips Date: Thu, 14 Jun 2018 21:33:02 -0500 Subject: [PATCH] Operations update decorations (#1778) * initial simple decorations (mark-like), many tests added * allow decorators to be set by focus, anchor tags - add tests * handle one more edge case with decorations in hyperscript * apply prettier cleanup * apply linting rules * update changelog * ensure always normalize decoration ranges * reapply prettier after latest adjustments * all operations apply now update decorations with selection * ranges can now be 'atomic', will invalidate if contents change * lint, prettier cleanups * add atomic invalidation tests, update hyperscript usage * fix linter errors * minor cleanup * slight refactor for simplicity * remove a couple superfluous lines * update in response to review * drop unnecessarily committed add'l file * remove the need for explicit anchor, focus prop on decoration tags * update hyperscript use to match latest syntax in #1777 * atomic -> isAtomic --- examples/search-highlighting/index.js | 1 + packages/slate-hyperscript/src/index.js | 10 +- packages/slate/src/models/range.js | 6 + packages/slate/src/models/value.js | 18 ++ packages/slate/src/operations/apply.js | 297 +++++++++++------- packages/slate/test/helpers/h.js | 3 + .../atomic-invalidation-delete.js | 61 ++++ .../atomic-invalidation-insert.js | 61 ++++ .../apply/update-decorations/delete-before.js | 33 ++ .../apply/update-decorations/insert-before.js | 33 ++ .../apply/update-decorations/merge-node.js | 45 +++ .../apply/update-decorations/remove-node.js | 34 ++ packages/slate/test/operations/index.js | 7 +- .../serialize/preserve-selection-and-keys.js | 1 + .../raw/serialize/preserve-selection.js | 1 + 15 files changed, 502 insertions(+), 109 deletions(-) create mode 100644 packages/slate/test/operations/apply/update-decorations/atomic-invalidation-delete.js create mode 100644 packages/slate/test/operations/apply/update-decorations/atomic-invalidation-insert.js create mode 100644 packages/slate/test/operations/apply/update-decorations/delete-before.js create mode 100644 packages/slate/test/operations/apply/update-decorations/insert-before.js create mode 100644 packages/slate/test/operations/apply/update-decorations/merge-node.js create mode 100644 packages/slate/test/operations/apply/update-decorations/remove-node.js diff --git a/examples/search-highlighting/index.js b/examples/search-highlighting/index.js index c7cf59a1a..2815f2d05 100644 --- a/examples/search-highlighting/index.js +++ b/examples/search-highlighting/index.js @@ -56,6 +56,7 @@ class SearchHighlighting extends React.Component { focusKey: key, focusOffset: offset, marks: [{ type: 'highlight' }], + atomic: true, }) } diff --git a/packages/slate-hyperscript/src/index.js b/packages/slate-hyperscript/src/index.js index 5b5332e9f..6f4098c5e 100644 --- a/packages/slate-hyperscript/src/index.js +++ b/packages/slate-hyperscript/src/index.js @@ -19,9 +19,12 @@ const FOCUS = {} */ class DecoratorPoint { - constructor(key, marks) { + constructor({ key, data }, marks) { this._key = key this.marks = marks + this.attribs = data || {} + this.isAtomic = !!this.attribs.atomic + delete this.attribs.atomic return this } withPosition = offset => { @@ -45,6 +48,8 @@ class DecoratorPoint { anchorOffset: this.offset, focusOffset: focus.offset, marks: this.marks, + isAtomic: this.isAtomic, + ...this.attribs, }) } } @@ -97,7 +102,7 @@ const CREATORS = { decoration(tagName, attributes, children) { if (attributes.key) { - return new DecoratorPoint(attributes.key, [{ type: tagName }]) + return new DecoratorPoint(attributes, [{ type: tagName }]) } const nodes = createChildren(children, { key: attributes.key }) @@ -106,6 +111,7 @@ const CREATORS = { anchorOffset: 0, focusOffset: nodes.reduce((len, n) => len + n.text.length, 0), marks: [{ type: tagName }], + isAtomic: !!attributes.data.atomic, }, ]) return nodes diff --git a/packages/slate/src/models/range.js b/packages/slate/src/models/range.js index 96c379a0f..e6a6dd726 100644 --- a/packages/slate/src/models/range.js +++ b/packages/slate/src/models/range.js @@ -19,6 +19,7 @@ const DEFAULTS = { isBackward: null, isFocused: false, marks: null, + isAtomic: false, } /** @@ -84,6 +85,7 @@ class Range extends Record(DEFAULTS) { isBackward: attrs.isBackward, isFocused: attrs.isFocused, marks: attrs.marks, + isAtomic: attrs.isAtomic, } } @@ -99,6 +101,7 @@ class Range extends Record(DEFAULTS) { if ('isFocused' in attrs) props.isFocused = attrs.isFocused if ('marks' in attrs) props.marks = attrs.marks == null ? null : Mark.createSet(attrs.marks) + if ('isAtomic' in attrs) props.isAtomic = attrs.isAtomic return props } @@ -123,6 +126,7 @@ class Range extends Record(DEFAULTS) { isBackward = null, isFocused = false, marks = null, + isAtomic = false, } = object const range = new Range({ @@ -133,6 +137,7 @@ class Range extends Record(DEFAULTS) { isBackward, isFocused, marks: marks == null ? null : new Set(marks.map(Mark.fromJSON)), + isAtomic, }) return range @@ -779,6 +784,7 @@ class Range extends Record(DEFAULTS) { isFocused: this.isFocused, marks: this.marks == null ? null : this.marks.toArray().map(m => m.toJSON()), + isAtomic: this.isAtomic, } return object diff --git a/packages/slate/src/models/value.js b/packages/slate/src/models/value.js index 51047d48c..5c475ff7c 100644 --- a/packages/slate/src/models/value.js +++ b/packages/slate/src/models/value.js @@ -675,6 +675,24 @@ class Value extends Record(DEFAULTS) { delete object.selection.focusKey } + if ( + options.preserveDecorations && + object.decorations && + !options.preserveKeys + ) { + const { document } = this + object.decorations = object.decorations.map(decoration => { + const withPath = { + ...decoration, + anchorPath: document.getPath(decoration.anchorKey), + focusPath: document.getPath(decoration.focusKey), + } + delete withPath.anchorKey + delete withPath.focusKey + return withPath + }) + } + return object } diff --git a/packages/slate/src/operations/apply.js b/packages/slate/src/operations/apply.js index b354552a9..620570c05 100644 --- a/packages/slate/src/operations/apply.js +++ b/packages/slate/src/operations/apply.js @@ -10,6 +10,65 @@ import Operation from '../models/operation' const debug = Debug('slate:operation:apply') +/** + * Apply adjustments to affected ranges (selections, decorations); + * accepts (value, checking function(range) -> bool, applying function(range) -> range) + * returns value with affected ranges updated + * + * @param {Value} value + * @param {Function} checkAffected + * @param {Function} adjustRange + * @return {Value} + */ + +function applyRangeAdjustments(value, checkAffected, adjustRange) { + // check selection, apply adjustment if affected + if (value.selection && checkAffected(value.selection)) { + value = value.set('selection', adjustRange(value.selection)) + } + if (!value.decorations) return value + + // check all ranges, apply adjustment if affected + const decorations = value.decorations + .map( + decoration => + checkAffected(decoration) ? adjustRange(decoration) : decoration + ) + .filter(decoration => decoration.anchorKey !== null) + return value.set('decorations', decorations) +} + +/** + * clear any atomic ranges (in decorations) if they contain the point (key, offset, offset-end?) + * specified + * + * @param {Value} value + * @param {String} key + * @param {Number} offset + * @param {Number?} offsetEnd + * @return {Value} + */ + +function clearAtomicRangesIfContains(value, key, offset, offsetEnd = null) { + return applyRangeAdjustments( + value, + range => { + if (!range.isAtomic) return false + const { startKey, startOffset, endKey, endOffset } = range + return ( + (startKey == key && + startOffset < offset && + (endKey != key || endOffset > offset)) || + (offsetEnd && + startKey == key && + startOffset < offsetEnd && + (endKey != key || endOffset > offsetEnd)) + ) + }, + range => range.deselect() + ) +} + /** * Applying functions. * @@ -65,23 +124,37 @@ const APPLIERS = { insert_text(value, operation) { const { path, offset, text, marks } = operation - let { document, selection } = value - const { anchorKey, focusKey, anchorOffset, focusOffset } = selection + let { document } = value let node = document.assertPath(path) // Update the document node = node.insertText(offset, text, marks) document = document.updateNode(node) - // Update the selection - if (anchorKey == node.key && anchorOffset >= offset) { - selection = selection.moveAnchor(text.length) - } - if (focusKey == node.key && focusOffset >= offset) { - selection = selection.moveFocus(text.length) - } + value = value.set('document', document) + + // if insert happens within atomic ranges, clear + value = clearAtomicRangesIfContains(value, node.key, offset) + + // Update the selection, decorations + value = applyRangeAdjustments( + value, + ({ anchorKey, anchorOffset, isBackward, isAtomic }) => + anchorKey == node.key && + (anchorOffset > offset || + (anchorOffset == offset && (!isAtomic || !isBackward))), + range => range.moveAnchor(text.length) + ) + + value = applyRangeAdjustments( + value, + ({ focusKey, focusOffset, isBackward, isAtomic }) => + focusKey == node.key && + (focusOffset > offset || + (focusOffset == offset && (!isAtomic || isBackward))), + range => range.moveFocus(text.length) + ) - value = value.set('document', document).set('selection', selection) return value }, @@ -98,7 +171,7 @@ const APPLIERS = { const withPath = path .slice(0, path.length - 1) .concat([path[path.length - 1] - 1]) - let { document, selection } = value + let { document } = value const one = document.assertPath(withPath) const two = document.assertPath(path) let parent = document.getParent(one.key) @@ -108,36 +181,31 @@ const APPLIERS = { // Perform the merge in the document. parent = parent.mergeNode(oneIndex, twoIndex) document = document.updateNode(parent) + value = value.set('document', document) - // If the nodes are text nodes and the selection is inside the second node - // update it to refer to the first node instead. if (one.object == 'text') { - const { anchorKey, anchorOffset, focusKey, focusOffset } = selection - let normalize = false - - if (anchorKey == two.key) { - selection = selection.moveAnchorTo( - one.key, - one.text.length + anchorOffset - ) - normalize = true - } - - if (focusKey == two.key) { - selection = selection.moveFocusTo( - one.key, - one.text.length + focusOffset - ) - normalize = true - } - - if (normalize) { - selection = selection.normalize(document) - } + value = applyRangeAdjustments( + value, + // If the nodes are text nodes and the range is inside the second node: + ({ anchorKey, focusKey }) => + anchorKey == two.key || focusKey == two.key, + // update it to refer to the first node instead: + range => { + if (range.anchorKey == two.key) + range = range.moveAnchorTo( + one.key, + one.text.length + range.anchorOffset + ) + if (range.focusKey == two.key) + range = range.moveFocusTo( + one.key, + one.text.length + range.focusOffset + ) + return range.normalize(document) + } + ) } - // Update the document and selection. - value = value.set('document', document).set('selection', selection) return value }, @@ -222,44 +290,38 @@ const APPLIERS = { remove_node(value, operation) { const { path } = operation let { document, selection } = value - const { startKey, endKey } = selection const node = document.assertPath(path) - // If the selection is set, check to see if it needs to be updated. - if (selection.isSet) { - const hasStartNode = node.hasNode(startKey) - const hasEndNode = node.hasNode(endKey) + if (selection.isSet || value.decorations !== null) { const first = node.object == 'text' ? node : node.getFirstText() || node const last = node.object == 'text' ? node : node.getLastText() || node const prev = document.getPreviousText(first.key) const next = document.getNextText(last.key) - // If the start point was in this node, update it to be just before/after. - if (hasStartNode) { - if (prev) { - selection = selection.moveStartTo(prev.key, prev.text.length) - } else if (next) { - selection = selection.moveStartTo(next.key, 0) - } else { - selection = selection.deselect() - } - } + value = applyRangeAdjustments( + value, + // If the start or end point was in this node + ({ startKey, endKey }) => + node.hasNode(startKey) || node.hasNode(endKey), + // update it to be just before/after + range => { + const { startKey, endKey } = range - // If the end point was in this node, update it to be just before/after. - if (selection.isSet && hasEndNode) { - if (prev) { - selection = selection.moveEndTo(prev.key, prev.text.length) - } else if (next) { - selection = selection.moveEndTo(next.key, 0) - } else { - selection = selection.deselect() + if (node.hasNode(startKey)) { + range = prev + ? range.moveStartTo(prev.key, prev.text.length) + : next ? range.moveStartTo(next.key, 0) : range.deselect() + } + if (node.hasNode(endKey)) { + range = prev + ? range.moveEndTo(prev.key, prev.text.length) + : next ? range.moveEndTo(next.key, 0) : range.deselect() + } + // If the range wasn't deselected, normalize it. + if (range.isSet) return range.normalize(document) + return range } - } - - // If the selection wasn't deselected, normalize it. - if (selection.isSet) { - selection = selection.normalize(document) - } + ) } // Remove the node from the document. @@ -268,8 +330,8 @@ const APPLIERS = { parent = parent.removeNode(index) document = document.updateNode(parent) - // Update the document and selection. - value = value.set('document', document).set('selection', selection) + // Update the document and range. + value = value.set('document', document) return value }, @@ -285,29 +347,47 @@ const APPLIERS = { const { path, offset, text } = operation const { length } = text const rangeOffset = offset + length - let { document, selection } = value - const { anchorKey, focusKey, anchorOffset, focusOffset } = selection + let { document } = value + let node = document.assertPath(path) - if (anchorKey == node.key) { - if (anchorOffset >= rangeOffset) { - selection = selection.moveAnchor(-length) - } else if (anchorOffset > offset) { - selection = selection.moveAnchorTo(anchorKey, offset) - } - } + // if insert happens within atomic ranges, clear + value = clearAtomicRangesIfContains( + value, + node.key, + offset, + offset + length + ) - if (focusKey == node.key) { - if (focusOffset >= rangeOffset) { - selection = selection.moveFocus(-length) - } else if (focusOffset > offset) { - selection = selection.moveFocusTo(focusKey, offset) - } - } + value = applyRangeAdjustments( + value, + // if anchor of range is here + ({ anchorKey }) => anchorKey == node.key, + // adjust if it is in or past the removal range + range => + range.anchorOffset >= rangeOffset + ? range.moveAnchor(-length) + : range.anchorOffset > offset + ? range.moveAnchorTo(range.anchorKey, offset) + : range + ) + + value = applyRangeAdjustments( + value, + // if focus of range is here + ({ focusKey }) => focusKey == node.key, + // adjust if it is in or past the removal range + range => + range.focusOffset >= rangeOffset + ? range.moveFocus(-length) + : range.focusOffset > offset + ? range.moveFocusTo(range.focusKey, offset) + : range + ) node = node.removeText(offset, length) document = document.updateNode(node) - value = value.set('document', document).set('selection', selection) + value = value.set('document', document) return value }, @@ -400,7 +480,7 @@ const APPLIERS = { split_node(value, operation) { const { path, position, properties } = operation - let { document, selection } = value + let { document } = value // Calculate a few things... const node = document.assertPath(path) @@ -416,32 +496,37 @@ const APPLIERS = { } } document = document.updateNode(parent) - - // Determine whether we need to update the selection... - const { startKey, endKey, startOffset, endOffset } = selection const next = document.getNextText(node.key) - let normalize = false - // If the start point is after or equal to the split, update it. - if (node.key == startKey && position <= startOffset) { - selection = selection.moveStartTo(next.key, startOffset - position) - normalize = true - } + value = applyRangeAdjustments( + value, + // check if range is affected + ({ startKey, startOffset, endKey, endOffset }) => + (node.key == startKey && position <= startOffset) || + (node.key == endKey && position <= endOffset), + // update its start / end as needed + range => { + const { startKey, startOffset, endKey, endOffset } = range + let normalize = false - // If the end point is after or equal to the split, update it. - if (node.key == endKey && position <= endOffset) { - selection = selection.moveEndTo(next.key, endOffset - position) - normalize = true - } + if (node.key == startKey && position <= startOffset) { + range = range.moveStartTo(next.key, startOffset - position) + normalize = true + } - // Normalize the selection if we changed it, since the methods we use might - // leave it in a non-normalized value. - if (normalize) { - selection = selection.normalize(document) - } + if (node.key == endKey && position <= endOffset) { + range = range.moveEndTo(next.key, endOffset - position) + normalize = true + } + + // Normalize the selection if we changed it + if (normalize) return range.normalize(document) + return range + } + ) // Return the updated value. - value = value.set('document', document).set('selection', selection) + value = value.set('document', document) return value }, } diff --git a/packages/slate/test/helpers/h.js b/packages/slate/test/helpers/h.js index 4d43662f7..f4ab0d7fb 100644 --- a/packages/slate/test/helpers/h.js +++ b/packages/slate/test/helpers/h.js @@ -34,6 +34,9 @@ const h = createHyperscript({ u: 'underline', fontSize: 'font-size', }, + decorators: { + highlight: 'highlight', + }, }) /** diff --git a/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-delete.js b/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-delete.js new file mode 100644 index 000000000..b0f39bdfe --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-delete.js @@ -0,0 +1,61 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'remove_text', + path: [0, 0], + offset: 13, + text: 'on', + marks: [], + }, + { + type: 'remove_text', + path: [1, 0], + offset: 0, + text: 'This ', + marks: [], + }, + { + type: 'remove_text', + path: [2, 0], + offset: 10, + text: 'ation', + marks: [], + }, +] + +export const input = ( + + + + This decoration should be invalid,{' '} + this one shouldn't. + + + This decoration will be fine. + + + This decoration can be altered, since non-atomic. + + + +) + +export const output = ( + + + + This decorati should be invalid, this one + shouldn't. + + + decoration will be fine. + + + This decor can be altered, since non-atomic. + + + +) diff --git a/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-insert.js b/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-insert.js new file mode 100644 index 000000000..07abd5bb6 --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/atomic-invalidation-insert.js @@ -0,0 +1,61 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'insert_text', + path: [0, 0], + offset: 6, + text: 'x', + marks: [], + }, + { + type: 'insert_text', + path: [1, 0], + offset: 5, + text: 'small ', + marks: [], + }, + { + type: 'remove_text', + path: [2, 0], + offset: 10, + text: 'ation', + marks: [], + }, +] + +export const input = ( + + + + This decoration should be invalid,{' '} + this one shouldn't. + + + This decoration will be fine. + + + This decoration can be altered, since non-atomic. + + + +) + +export const output = ( + + + + This dxecoration should be invalid, this{' '} + one shouldn't. + + + This small decoration will be fine. + + + This decor can be altered, since non-atomic. + + + +) diff --git a/packages/slate/test/operations/apply/update-decorations/delete-before.js b/packages/slate/test/operations/apply/update-decorations/delete-before.js new file mode 100644 index 000000000..68395d856 --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/delete-before.js @@ -0,0 +1,33 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'remove_text', + path: [0, 0], + offset: 2, + text: ' there', + marks: [], + }, +] + +export const input = ( + + + + Hi there you person + + + +) + +export const output = ( + + + + Hi you person + + + +) diff --git a/packages/slate/test/operations/apply/update-decorations/insert-before.js b/packages/slate/test/operations/apply/update-decorations/insert-before.js new file mode 100644 index 000000000..68ebe34b5 --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/insert-before.js @@ -0,0 +1,33 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'insert_text', + path: [0, 0], + offset: 2, + text: ' added', + marks: [], + }, +] + +export const input = ( + + + + Hi there you + + + +) + +export const output = ( + + + + Hi added there you + + + +) diff --git a/packages/slate/test/operations/apply/update-decorations/merge-node.js b/packages/slate/test/operations/apply/update-decorations/merge-node.js new file mode 100644 index 000000000..bf5c39dda --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/merge-node.js @@ -0,0 +1,45 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'merge_node', + path: [1], + position: 1, + properties: {}, + target: null, + }, + // also merge the resulting leaves + { + type: 'merge_node', + path: [0, 1], + position: 1, + properties: {}, + target: null, + }, +] + +export const input = ( + + + + The decoration begins in this paragraph + + + And ends in this soon-to-be merged one. + + + +) + +export const output = ( + + + + The decoration begins in this paragraphAnd ends in + this soon-to-be merged one. + + + +) diff --git a/packages/slate/test/operations/apply/update-decorations/remove-node.js b/packages/slate/test/operations/apply/update-decorations/remove-node.js new file mode 100644 index 000000000..a6244e526 --- /dev/null +++ b/packages/slate/test/operations/apply/update-decorations/remove-node.js @@ -0,0 +1,34 @@ +/** @jsx h */ + +import h from '../../../helpers/h' + +export default [ + { + type: 'remove_node', + path: [0], + }, +] + +export const input = ( + + + + The decoration begins in this soon deleted + paragraph + + + And ends in this one. + + + +) + +export const output = ( + + + + And ends in this one. + + + +) diff --git a/packages/slate/test/operations/index.js b/packages/slate/test/operations/index.js index 907e55e03..68b10586d 100644 --- a/packages/slate/test/operations/index.js +++ b/packages/slate/test/operations/index.js @@ -33,9 +33,14 @@ describe('operations', async () => { const operations = module.default const change = input.change() change.applyOperations(operations) - const opts = { preserveSelection: true, preserveData: true } + const opts = { + preserveSelection: true, + preserveDecorations: true, + preserveData: true, + } const actual = change.value.toJSON(opts) const expected = output.toJSON(opts) + assert.deepEqual(actual, expected) }) } diff --git a/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js b/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js index 570845734..49a4f2c37 100644 --- a/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js +++ b/packages/slate/test/serializers/raw/serialize/preserve-selection-and-keys.js @@ -48,6 +48,7 @@ export const output = { isBackward: false, isFocused: false, marks: null, + isAtomic: false, }, } diff --git a/packages/slate/test/serializers/raw/serialize/preserve-selection.js b/packages/slate/test/serializers/raw/serialize/preserve-selection.js index 164fb5dce..de73a549b 100644 --- a/packages/slate/test/serializers/raw/serialize/preserve-selection.js +++ b/packages/slate/test/serializers/raw/serialize/preserve-selection.js @@ -45,6 +45,7 @@ export const output = { isBackward: false, isFocused: false, marks: null, + isAtomic: false, }, }