From 1a5fcb72d3f08acba0cc4b08fd20509f17fdbb8f Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Wed, 28 Dec 2016 22:23:50 +0100 Subject: [PATCH 1/5] FEATURE: Preserve cursor in editor upload --- .../discourse/components/d-editor.js.es6 | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index f9fc95e31f..52e35b8b31 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -525,11 +525,43 @@ export default Ember.Component.extend({ _replaceText(oldVal, newVal) { const val = this.get('value'); - const loc = val.indexOf(oldVal); - if (loc !== -1) { - this.set('value', val.replace(oldVal, newVal)); - this._selectText(loc + newVal.length, 0); + const needleStart = val.indexOf(oldVal); + + if (needleStart === -1) { + // Nothing to replace. + return; } + + const textarea = this.$('textarea.d-editor-input')[0]; + + // Remember cursor/selection. + const selectionStart = textarea.selectionStart; + const selectionEnd = textarea.selectionEnd; + const needleEnd = needleStart + oldValue.length; + const replacementEnd = needleStart + newVal.length; + + // Replace value (side effect: cursor at end). + this.set('value', val.replace(oldVal, newVal)); + + // Determine cursor/selection. + const newSelectionStart, newSelectionEnd; + if (selectionEnd <= needleEnd) { + // Selection before needle. + newSelectionStart = selectionStart; + newSelectionEnd = selectionEnd; + } else if (selectionStart < needleEnd) { + // Selection within needle. + newSelectionStart = replacementEnd; + newSelectionEnd = replacementEnd; + } else { + // Selection behind needle. + const lengthDiff = replacementEnd - needleStart; + newSelectionStart = selectionStart + lengthDiff; + newSelectionEnd = selectionEnd + lengthDiff; + } + + // Restore cursor. + this._selectText(newSelectionStart, newSelectionEnd - newSelectionStart); }, _addText(sel, text) { From ba2db48dbb683db81f8fe171f81c86868d616be2 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Wed, 28 Dec 2016 22:37:07 +0100 Subject: [PATCH 2/5] Fix refactoring artifacts. --- app/assets/javascripts/discourse/components/d-editor.js.es6 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 52e35b8b31..457ccda02c 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -537,14 +537,14 @@ export default Ember.Component.extend({ // Remember cursor/selection. const selectionStart = textarea.selectionStart; const selectionEnd = textarea.selectionEnd; - const needleEnd = needleStart + oldValue.length; + const needleEnd = needleStart + oldVal.length; const replacementEnd = needleStart + newVal.length; // Replace value (side effect: cursor at end). this.set('value', val.replace(oldVal, newVal)); // Determine cursor/selection. - const newSelectionStart, newSelectionEnd; + let newSelectionStart, newSelectionEnd; if (selectionEnd <= needleEnd) { // Selection before needle. newSelectionStart = selectionStart; From 43c1dd82f6307fa2b321fba26840a131e75b98ea Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Thu, 29 Dec 2016 09:54:15 +0100 Subject: [PATCH 3/5] Add cursor tests, fix algorithm and extract method. --- .../discourse/components/d-editor.js.es6 | 62 ++++++++----- .../components/d-editor-test.js.es6 | 93 ++++++++++++++++++- 2 files changed, 129 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 457ccda02c..97fd9b2074 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -534,34 +534,48 @@ export default Ember.Component.extend({ const textarea = this.$('textarea.d-editor-input')[0]; - // Remember cursor/selection. - const selectionStart = textarea.selectionStart; - const selectionEnd = textarea.selectionEnd; - const needleEnd = needleStart + oldVal.length; - const replacementEnd = needleStart + newVal.length; + // Determine post-replace selection. + const newSelection = this._determinePostReplaceSelection({ + selection: { start: textarea.selectionStart, end: textarea.selectionEnd }, + needle: { start: needleStart, end: needleStart + oldVal.length }, + replacement: { start: needleStart, end: needleStart + newVal.length } + }); - // Replace value (side effect: cursor at end). + // Replace value (side effect: cursor at the end). this.set('value', val.replace(oldVal, newVal)); - // Determine cursor/selection. - let newSelectionStart, newSelectionEnd; - if (selectionEnd <= needleEnd) { - // Selection before needle. - newSelectionStart = selectionStart; - newSelectionEnd = selectionEnd; - } else if (selectionStart < needleEnd) { - // Selection within needle. - newSelectionStart = replacementEnd; - newSelectionEnd = replacementEnd; - } else { - // Selection behind needle. - const lengthDiff = replacementEnd - needleStart; - newSelectionStart = selectionStart + lengthDiff; - newSelectionEnd = selectionEnd + lengthDiff; - } - // Restore cursor. - this._selectText(newSelectionStart, newSelectionEnd - newSelectionStart); + this._selectText(newSelection.start, newSelection.end - newSelection.start); + }, + + _determinePostReplaceSelection({ selection, needle, replacement }) { + const diff = (replacement.end - replacement.start) - (needle.end - needle.start); + + if (selection.end <= needle.start) { + // Selection ends (and starts) before needle. + return { start: selection.start, end: selection.end }; + } else if (selection.start <= needle.start) { + // Selection starts before needle... + if (selection.end < needle.end) { + // ... and ends inside needle. + return { start: selection.start, end: needle.start }; + } else { + // ... and spans needle completely. + return { start: selection.start, end: selection.end + diff }; + } + } else if (selection.start < needle.end) { + // Selection starts inside needle... + if (selection.end <= needle.end) { + // ... and ends inside needle. + return { start: replacement.end, end: replacement.end }; + } else { + // ... and spans end of needle. + return { start: replacement.end, end: selection.end + diff }; + } + } else { + // Selection starts (and ends) behind needle. + return { start: selection.start + diff, end: selection.end + diff }; + } }, _addText(sel, text) { diff --git a/test/javascripts/components/d-editor-test.js.es6 b/test/javascripts/components/d-editor-test.js.es6 index 4fefd8c819..f8e1037132 100644 --- a/test/javascripts/components/d-editor-test.js.es6 +++ b/test/javascripts/components/d-editor-test.js.es6 @@ -770,7 +770,96 @@ testCase("replace-text event", function(assert, textarea) { andThen(() => { assert.equal(this.get('value'), 'red yellow blue'); - assert.equal(textarea.selectionStart, 10); - assert.equal(textarea.selectionEnd, 10); }); }); + +(() => { + // Tests to check cursor/selection after replace-text event. + const BEFORE = 'red green blue'; + const NEEDLE = 'green'; + const REPLACE = 'yellow'; + const AFTER = BEFORE.replace(NEEDLE, REPLACE); + + const CASES = [ + { + description: 'cursor at start remains there', + before: [0, 0], + after: [0, 0] + },{ + description: 'cursor before needle becomes cursor before replacement', + before: [BEFORE.indexOf(NEEDLE), 0], + after: [AFTER.indexOf(REPLACE), 0] + },{ + description: 'cursor at needle start + 1 moves behind replacement', + before: [BEFORE.indexOf(NEEDLE) + 1, 0], + after: [AFTER.indexOf(REPLACE) + REPLACE.length, 0] + },{ + description: 'cursor at needle end - 1 stays behind replacement', + before: [BEFORE.indexOf(NEEDLE) + NEEDLE.length - 1, 0], + after: [AFTER.indexOf(REPLACE) + REPLACE.length, 0] + },{ + description: 'cursor behind needle becomes cursor behind replacement', + before: [BEFORE.indexOf(NEEDLE) + NEEDLE.length, 0], + after: [AFTER.indexOf(REPLACE) + REPLACE.length, 0] + },{ + description: 'cursor at end remains there', + before: [BEFORE.length, 0], + after: [AFTER.length, 0] + },{ + description: 'selection spanning needle start becomes selection until replacement start', + before: [BEFORE.indexOf(NEEDLE) - 1, 2], + after: [AFTER.indexOf(REPLACE) - 1, 1] + },{ + description: 'selection spanning needle end becomes selection from replacement end', + before: [BEFORE.indexOf(NEEDLE) + NEEDLE.length - 1, 2], + after: [AFTER.indexOf(REPLACE) + REPLACE.length, 1] + },{ + description: 'selection spanning needle becomes selection spanning replacement', + before: [BEFORE.indexOf(NEEDLE) - 1, NEEDLE.length + 2], + after: [AFTER.indexOf(REPLACE) - 1, REPLACE.length + 2] + },{ + description: 'complete selection remains complete', + before: [0, BEFORE.length], + after: [0, AFTER.length] + } + ]; + + function setSelection(textarea, [start, len]) { + textarea.selectionStart = start; + textarea.selectionEnd = start + len; + } + + function getSelection(textarea) { + const start = textarea.selectionStart; + const end = textarea.selectionEnd + return [start, end - start]; + } + + function formatTextWithSelection(text, [start, len]) { + return [ + '"', + text.substr(0, start), + '<', + text.substr(start, len), + '>', + text.substr(start+len), + '"', + ].join(''); + } + + for (let i = 0; i < CASES.length; i++) { + const CASE = CASES[i]; + testCase(`replace-text event: ${CASE.description}`, function(assert, textarea) { + this.set('value', BEFORE); + setSelection(textarea, CASE.before); + andThen(() => { + this.container.lookup('app-events:main').trigger('composer:replace-text', 'green', 'yellow'); + }); + andThen(() => { + let expect = formatTextWithSelection(AFTER, CASE.after); + let actual = formatTextWithSelection(this.get('value'), getSelection(textarea)); + assert.equal(actual, expect); + }); + }); + } +})(); From 37386faff2cac8579ea8ea6714fed4b6b7cdbcca Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Thu, 29 Dec 2016 10:05:07 +0100 Subject: [PATCH 4/5] Fix eslint nits. --- test/javascripts/components/d-editor-test.js.es6 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/javascripts/components/d-editor-test.js.es6 b/test/javascripts/components/d-editor-test.js.es6 index f8e1037132..1a594cd2da 100644 --- a/test/javascripts/components/d-editor-test.js.es6 +++ b/test/javascripts/components/d-editor-test.js.es6 @@ -760,7 +760,7 @@ componentTest('emoji', { } }); -testCase("replace-text event", function(assert, textarea) { +testCase("replace-text event", function(assert) { this.set('value', "red green blue"); @@ -831,7 +831,7 @@ testCase("replace-text event", function(assert, textarea) { function getSelection(textarea) { const start = textarea.selectionStart; - const end = textarea.selectionEnd + const end = textarea.selectionEnd; return [start, end - start]; } From 2e9bbccea93db110b4bc8d39dd760db46445c8c0 Mon Sep 17 00:00:00 2001 From: Claas Augner Date: Thu, 29 Dec 2016 10:16:17 +0100 Subject: [PATCH 5/5] Move cursor/selection algo to lib/utilities. --- .../discourse/components/d-editor.js.es6 | 33 ++----------------- .../discourse/lib/utilities.js.es6 | 30 +++++++++++++++++ 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/app/assets/javascripts/discourse/components/d-editor.js.es6 b/app/assets/javascripts/discourse/components/d-editor.js.es6 index 97fd9b2074..d522832496 100644 --- a/app/assets/javascripts/discourse/components/d-editor.js.es6 +++ b/app/assets/javascripts/discourse/components/d-editor.js.es6 @@ -12,6 +12,7 @@ import { emojiSearch } from 'pretty-text/emoji'; import { emojiUrlFor } from 'discourse/lib/text'; import { getRegister } from 'discourse-common/lib/get-owner'; import { findRawTemplate } from 'discourse/lib/raw-templates'; +import { determinePostReplaceSelection } from 'discourse/lib/utilities'; import deprecated from 'discourse-common/lib/deprecated'; // Our head can be a static string or a function that returns a string @@ -535,7 +536,7 @@ export default Ember.Component.extend({ const textarea = this.$('textarea.d-editor-input')[0]; // Determine post-replace selection. - const newSelection = this._determinePostReplaceSelection({ + const newSelection = determinePostReplaceSelection({ selection: { start: textarea.selectionStart, end: textarea.selectionEnd }, needle: { start: needleStart, end: needleStart + oldVal.length }, replacement: { start: needleStart, end: needleStart + newVal.length } @@ -548,36 +549,6 @@ export default Ember.Component.extend({ this._selectText(newSelection.start, newSelection.end - newSelection.start); }, - _determinePostReplaceSelection({ selection, needle, replacement }) { - const diff = (replacement.end - replacement.start) - (needle.end - needle.start); - - if (selection.end <= needle.start) { - // Selection ends (and starts) before needle. - return { start: selection.start, end: selection.end }; - } else if (selection.start <= needle.start) { - // Selection starts before needle... - if (selection.end < needle.end) { - // ... and ends inside needle. - return { start: selection.start, end: needle.start }; - } else { - // ... and spans needle completely. - return { start: selection.start, end: selection.end + diff }; - } - } else if (selection.start < needle.end) { - // Selection starts inside needle... - if (selection.end <= needle.end) { - // ... and ends inside needle. - return { start: replacement.end, end: replacement.end }; - } else { - // ... and spans end of needle. - return { start: replacement.end, end: selection.end + diff }; - } - } else { - // Selection starts (and ends) behind needle. - return { start: selection.start + diff, end: selection.end + diff }; - } - }, - _addText(sel, text) { const $textarea = this.$('textarea.d-editor-input'); const insert = `${sel.pre}${text}`; diff --git a/app/assets/javascripts/discourse/lib/utilities.js.es6 b/app/assets/javascripts/discourse/lib/utilities.js.es6 index fcf6737527..27b55bbdd7 100644 --- a/app/assets/javascripts/discourse/lib/utilities.js.es6 +++ b/app/assets/javascripts/discourse/lib/utilities.js.es6 @@ -300,5 +300,35 @@ export function defaultHomepage() { return Discourse.SiteSettings.top_menu.split("|")[0].split(",")[0]; } +export function determinePostReplaceSelection({ selection, needle, replacement }) { + const diff = (replacement.end - replacement.start) - (needle.end - needle.start); + + if (selection.end <= needle.start) { + // Selection ends (and starts) before needle. + return { start: selection.start, end: selection.end }; + } else if (selection.start <= needle.start) { + // Selection starts before needle... + if (selection.end < needle.end) { + // ... and ends inside needle. + return { start: selection.start, end: needle.start }; + } else { + // ... and spans needle completely. + return { start: selection.start, end: selection.end + diff }; + } + } else if (selection.start < needle.end) { + // Selection starts inside needle... + if (selection.end <= needle.end) { + // ... and ends inside needle. + return { start: replacement.end, end: replacement.end }; + } else { + // ... and spans end of needle. + return { start: replacement.end, end: selection.end + diff }; + } + } else { + // Selection starts (and ends) behind needle. + return { start: selection.start + diff, end: selection.end + diff }; + } +} + // This prevents a mini racer crash export default {};