diff --git a/res/css/views/rooms/_BasicMessageComposer.scss b/res/css/views/rooms/_BasicMessageComposer.scss index b6035e5859..bce0ecf325 100644 --- a/res/css/views/rooms/_BasicMessageComposer.scss +++ b/res/css/views/rooms/_BasicMessageComposer.scss @@ -27,6 +27,15 @@ limitations under the License. white-space: nowrap; } + @keyframes visualbell { + from { background-color: $visual-bell-bg-color; } + to { background-color: $primary-bg-color; } + } + + &.mx_BasicMessageComposer_input_error { + animation: 0.2s visualbell; + } + .mx_BasicMessageComposer_input { white-space: pre-wrap; word-wrap: break-word; diff --git a/res/css/views/rooms/_MessageComposer.scss b/res/css/views/rooms/_MessageComposer.scss index 6e17251cb0..5b4a9b764b 100644 --- a/res/css/views/rooms/_MessageComposer.scss +++ b/res/css/views/rooms/_MessageComposer.scss @@ -129,7 +129,7 @@ limitations under the License. } @keyframes visualbell { - from { background-color: #faa; } + from { background-color: $visual-bell-bg-color; } to { background-color: $primary-bg-color; } } diff --git a/res/themes/dark/css/_dark.scss b/res/themes/dark/css/_dark.scss index 90cd8e8558..f54d25ab29 100644 --- a/res/themes/dark/css/_dark.scss +++ b/res/themes/dark/css/_dark.scss @@ -146,6 +146,8 @@ $button-danger-disabled-bg-color: #f5b6bb; // TODO: Verify color $button-link-fg-color: $accent-color; $button-link-bg-color: transparent; +$visual-bell-bg-color: #800; + $room-warning-bg-color: $header-panel-bg-color; $dark-panel-bg-color: $header-panel-bg-color; diff --git a/res/themes/light/css/_light.scss b/res/themes/light/css/_light.scss index d8d4b0a11b..be46367fbb 100644 --- a/res/themes/light/css/_light.scss +++ b/res/themes/light/css/_light.scss @@ -247,6 +247,8 @@ $button-danger-disabled-bg-color: #f5b6bb; // TODO: Verify color $button-link-fg-color: $accent-color; $button-link-bg-color: transparent; +$visual-bell-bg-color: #faa; + // Toggle switch $togglesw-off-color: #c1c9d6; $togglesw-on-color: $accent-color; diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 662167b714..49815c6f23 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ + +import classNames from 'classnames'; import React from 'react'; import PropTypes from 'prop-types'; import EditorModel from '../../../editor/model'; @@ -75,10 +77,10 @@ export default class BasicMessageEditor extends React.Component { this._modifiedFlag = false; } - _replaceEmoticon = (caret, inputType, diff) => { + _replaceEmoticon = (caretPosition, inputType, diff) => { const {model} = this.props; - const range = model.startRange(caret); - // expand range max 8 characters backwards from caret, + const range = model.startRange(caretPosition); + // expand range max 8 characters backwards from caretPosition, // as a space to look for an emoticon let n = 8; range.expandBackwardsWhile((index, offset) => { @@ -91,6 +93,7 @@ export default class BasicMessageEditor extends React.Component { const query = emoticonMatch[1].toLowerCase().replace("-", ""); const data = EMOJIBASE.find(e => e.emoticon ? e.emoticon.toLowerCase() === query : false); if (data) { + const {partCreator} = model; const hasPrecedingSpace = emoticonMatch[0][0] === " "; // we need the range to only comprise of the emoticon // because we'll replace the whole range with an emoji, @@ -99,7 +102,7 @@ export default class BasicMessageEditor extends React.Component { range.moveStart(emoticonMatch.index + (hasPrecedingSpace ? 1 : 0)); // this returns the amount of added/removed characters during the replace // so the caret position can be adjusted. - return range.replace([this.props.model.partCreator.plain(data.unicode + " ")]); + return range.replace([partCreator.plain(data.unicode + " ")]); } } } @@ -160,7 +163,7 @@ export default class BasicMessageEditor extends React.Component { } _refreshLastCaretIfNeeded() { - // TODO: needed when going up and down in editing messages ... not sure why yet + // XXX: needed when going up and down in editing messages ... not sure why yet // because the editors should stop doing this when when blurred ... // maybe it's on focus and the _editorRef isn't available yet or something. if (!this._editorRef) { @@ -269,6 +272,9 @@ export default class BasicMessageEditor extends React.Component { default: return; // don't preventDefault on anything else } + } else if (event.key === "Tab") { + this._tabCompleteName(); + handled = true; } } if (handled) { @@ -277,6 +283,32 @@ export default class BasicMessageEditor extends React.Component { } } + async _tabCompleteName() { + try { + await new Promise(resolve => this.setState({showVisualBell: false}, resolve)); + const {model} = this.props; + const caret = this.getCaret(); + const position = model.positionForOffset(caret.offset, caret.atNodeEnd); + const range = model.startRange(position); + range.expandBackwardsWhile((index, offset, part) => { + return part.text[offset] !== " " && (part.type === "plain" || part.type === "pill-candidate"); + }); + const {partCreator} = model; + // await for auto-complete to be open + await model.transform(() => { + const addedLen = range.replace([partCreator.pillCandidate(range.text)]); + return model.positionForOffset(caret.offset + addedLen, true); + }); + await model.autoComplete.onTab(); + if (!model.autoComplete.hasSelection()) { + this.setState({showVisualBell: true}); + model.autoComplete.close(); + } + } catch (err) { + console.error(err); + } + } + isModified() { return this._modifiedFlag; } @@ -304,7 +336,7 @@ export default class BasicMessageEditor extends React.Component { // not really, but we could not serialize the parts, and just change the autoCompleter partCreator.setAutoCompleteCreator(autoCompleteCreator( () => this._autocompleteRef, - query => this.setState({query}), + query => new Promise(resolve => this.setState({query}, resolve)), )); this.historyManager = new HistoryManager(partCreator); // initial render of model @@ -345,7 +377,10 @@ export default class BasicMessageEditor extends React.Component { /> ); } - return (
+ const classes = classNames("mx_BasicMessageComposer", { + "mx_BasicMessageComposer_input_error": this.state.showVisualBell, + }); + return (
{ autoComplete }
{ + const addedLen = model.insert([userPillPart], position); + return model.positionForOffset(caret.offset + addedLen, true); + }); // refocus on composer, as we just clicked "Mention" this._editorRef && this._editorRef.focus(); } _insertQuotedMessage(event) { - const {partCreator} = this.model; + const {model} = this; + const {partCreator} = model; const quoteParts = parseEvent(event, partCreator, { isQuotedMessage: true }); // add two newlines quoteParts.push(partCreator.newline()); quoteParts.push(partCreator.newline()); - this.model.insertPartsAt(quoteParts, {offset: 0}); + model.transform(() => { + const addedLen = model.insert(quoteParts, model.positionForOffset(0)); + return model.positionForOffset(addedLen, true); + }); // refocus on composer, as we just clicked "Quote" this._editorRef && this._editorRef.focus(); } diff --git a/src/editor/autocomplete.js b/src/editor/autocomplete.js index ac662c32d8..79a69c07a6 100644 --- a/src/editor/autocomplete.js +++ b/src/editor/autocomplete.js @@ -33,6 +33,10 @@ export default class AutocompleteWrapperModel { }); } + close() { + this._updateCallback({close: true}); + } + hasSelection() { return this._getAutocompleterComponent().hasSelection(); } @@ -52,9 +56,6 @@ export default class AutocompleteWrapperModel { } else { await acComponent.moveSelection(e.shiftKey ? -1 : +1); } - this._updateCallback({ - close: true, - }); } onUpArrow() { @@ -70,7 +71,7 @@ export default class AutocompleteWrapperModel { // so we can restore it in onComponentSelectionChange when the value is undefined (meaning it should be the typed text) this._queryPart = part; this._queryOffset = offset; - this._updateQuery(part.text); + return this._updateQuery(part.text); } onComponentSelectionChange(completion) { diff --git a/src/editor/model.js b/src/editor/model.js index 9d129afa69..59371cc3e6 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -35,6 +35,11 @@ import Range from "./range"; * This is used to adjust the caret position. */ +/** + * @callback ManualTransformCallback + * @return the caret position + */ + export default class EditorModel { constructor(parts, partCreator, updateCallback = null) { this._parts = parts; @@ -44,7 +49,6 @@ export default class EditorModel { this._autoCompletePartIdx = null; this._transformCallback = null; this.setUpdateCallback(updateCallback); - this._updateInProgress = false; } /** @@ -90,10 +94,14 @@ export default class EditorModel { _removePart(index) { this._parts.splice(index, 1); - if (this._activePartIdx >= index) { + if (index === this._activePartIdx) { + this._activePartIdx = null; + } else if (this._activePartIdx > index) { --this._activePartIdx; } - if (this._autoCompletePartIdx >= index) { + if (index === this._autoCompletePartIdx) { + this._autoCompletePartIdx = null; + } else if (this._autoCompletePartIdx > index) { --this._autoCompletePartIdx; } } @@ -150,8 +158,14 @@ export default class EditorModel { this._updateCallback(caret, inputType); } - insertPartsAt(parts, caret) { - const position = this.positionForOffset(caret.offset, caret.atNodeEnd); + /** + * Inserts the given parts at the given position. + * Should be run inside a `model.transform()` callback. + * @param {Part[]} parts the parts to replace the range with + * @param {DocumentPosition} position the position to start inserting at + * @return {Number} the amount of characters added + */ + insert(parts, position) { const insertIndex = this._splitAt(position); let newTextLength = 0; for (let i = 0; i < parts.length; ++i) { @@ -159,14 +173,10 @@ export default class EditorModel { newTextLength += part.text.length; this._insertPart(insertIndex + i, part); } - // put caret after new part - const lastPartIndex = insertIndex + parts.length - 1; - const newPosition = new DocumentPosition(lastPartIndex, newTextLength); - this._updateCallback(newPosition); + return newTextLength; } update(newValue, inputType, caret) { - this._updateInProgress = true; const diff = this._diff(newValue, inputType, caret); const position = this.positionForOffset(diff.at, caret.atNodeEnd); let removedOffsetDecrease = 0; @@ -182,13 +192,13 @@ export default class EditorModel { this._mergeAdjacentParts(); const caretOffset = diff.at - removedOffsetDecrease + addedLen; let newPosition = this.positionForOffset(caretOffset, true); - this._setActivePart(newPosition, canOpenAutoComplete); + const acPromise = this._setActivePart(newPosition, canOpenAutoComplete); if (this._transformCallback) { const transformAddedLen = this._transform(newPosition, inputType, diff); newPosition = this.positionForOffset(caretOffset + transformAddedLen, true); } - this._updateInProgress = false; this._updateCallback(newPosition, inputType, diff); + return acPromise; } _transform(newPosition, inputType, diff) { @@ -214,13 +224,14 @@ export default class EditorModel { } // not _autoComplete, only there if active part is autocomplete part if (this.autoComplete) { - this.autoComplete.onPartUpdate(part, pos.offset); + return this.autoComplete.onPartUpdate(part, pos.offset); } } else { this._activePartIdx = null; this._autoComplete = null; this._autoCompletePartIdx = null; } + return Promise.resolve(); } _onAutoComplete = ({replacePart, caretOffset, close}) => { @@ -395,18 +406,15 @@ export default class EditorModel { return new Range(this, position); } - // called from Range.replace + //mostly internal, called from Range.replace replaceRange(startPosition, endPosition, parts) { + // convert end position to offset, so it is independent of how the document is split into parts + // which we'll change when splitting up at the start position + const endOffset = endPosition.asOffset(this); const newStartPartIndex = this._splitAt(startPosition); - const idxDiff = newStartPartIndex - startPosition.index; - // if both position are in the same part, and we split it at start position, - // the offset of the end position needs to be decreased by the offset of the start position - const removedOffset = startPosition.index === endPosition.index ? startPosition.offset : 0; - const adjustedEndPosition = new DocumentPosition( - endPosition.index + idxDiff, - endPosition.offset - removedOffset, - ); - const newEndPartIndex = this._splitAt(adjustedEndPosition); + // convert it back to position once split at start + endPosition = endOffset.asPosition(this); + const newEndPartIndex = this._splitAt(endPosition); for (let i = newEndPartIndex - 1; i >= newStartPartIndex; --i) { this._removePart(i); } @@ -416,8 +424,18 @@ export default class EditorModel { insertIdx += 1; } this._mergeAdjacentParts(); - if (!this._updateInProgress) { - this._updateCallback(); - } + } + + /** + * Performs a transformation not part of an update cycle. + * Modifying the model should only happen inside a transform call if not part of an update call. + * @param {ManualTransformCallback} callback to run the transformations in + * @return {Promise} a promise when auto-complete (if applicable) is done updating + */ + transform(callback) { + const pos = callback(); + const acPromise = this._setActivePart(pos, true); + this._updateCallback(pos); + return acPromise; } } diff --git a/src/editor/offset.js b/src/editor/offset.js new file mode 100644 index 0000000000..7054836bdc --- /dev/null +++ b/src/editor/offset.js @@ -0,0 +1,26 @@ +/* +Copyright 2019 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export default class DocumentOffset { + constructor(offset, atEnd) { + this.offset = offset; + this.atEnd = atEnd; + } + + asPosition(model) { + return model.positionForOffset(this.offset, this.atEnd); + } +} diff --git a/src/editor/parts.js b/src/editor/parts.js index f9b4243de4..8d0fe36c28 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -284,6 +284,9 @@ class UserPillPart extends PillPart { } setAvatar(node) { + if (!this._member) { + return; + } const name = this._member.name || this._member.userId; const defaultAvatarUrl = Avatar.defaultAvatarUrlForString(this._member.userId); let avatarUrl = Avatar.avatarUrlForMember( @@ -366,6 +369,8 @@ export class PartCreator { constructor(room, client, autoCompleteCreator = null) { this._room = room; this._client = client; + // pre-create the creator as an object even without callback so it can already be passed + // to PillCandidatePart (e.g. while deserializing) and set later on this._autoCompleteCreator = {create: autoCompleteCreator && autoCompleteCreator(this)}; } diff --git a/src/editor/position.js b/src/editor/position.js index 5dcb31fe65..98b158e547 100644 --- a/src/editor/position.js +++ b/src/editor/position.js @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ +import DocumentOffset from "./offset"; + export default class DocumentPosition { constructor(index, offset) { this._index = index; @@ -104,4 +106,18 @@ export default class DocumentPosition { } } } + + asOffset(model) { + if (this.index === -1) { + return new DocumentOffset(0, true); + } + let offset = 0; + for (let i = 0; i < this.index; ++i) { + offset += model.parts[i].text.length; + } + offset += this.offset; + const lastPart = model.parts[this.index]; + const atEnd = offset >= lastPart.text.length; + return new DocumentOffset(offset, atEnd); + } } diff --git a/src/editor/range.js b/src/editor/range.js index e2ecc5d12b..1aaf480733 100644 --- a/src/editor/range.js +++ b/src/editor/range.js @@ -41,6 +41,12 @@ export default class Range { return text; } + /** + * Splits the model at the range boundaries and replaces with the given parts. + * Should be run inside a `model.transform()` callback. + * @param {Part[]} parts the parts to replace the range with + * @return {Number} the net amount of characters added, can be negative. + */ replace(parts) { const newLength = parts.reduce((sum, part) => sum + part.text.length, 0); let oldLength = 0; diff --git a/test/editor/range-test.js b/test/editor/range-test.js index 5a95da952d..468cb60c76 100644 --- a/test/editor/range-test.js +++ b/test/editor/range-test.js @@ -52,7 +52,6 @@ describe('editor/range', function() { range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " "); expect(range.text).toBe("world"); range.replace([pc.roomPill(pillChannel)]); - console.log({parts: JSON.stringify(model.serializeParts())}); expect(model.parts[0].type).toBe("plain"); expect(model.parts[0].text).toBe("hello "); expect(model.parts[1].type).toBe("room-pill"); @@ -60,7 +59,6 @@ describe('editor/range', function() { expect(model.parts[2].type).toBe("plain"); expect(model.parts[2].text).toBe("!!!!"); expect(model.parts.length).toBe(3); - expect(renderer.count).toBe(1); }); it('range replace across parts', function() { const renderer = createRenderer(); @@ -74,7 +72,6 @@ describe('editor/range', function() { const range = model.startRange(model.positionForOffset(14)); // after "replace" range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " "); expect(range.text).toBe("replace"); - console.log("range.text", {text: range.text}); range.replace([pc.roomPill(pillChannel)]); expect(model.parts[0].type).toBe("plain"); expect(model.parts[0].text).toBe("try to "); @@ -83,6 +80,23 @@ describe('editor/range', function() { expect(model.parts[2].type).toBe("plain"); expect(model.parts[2].text).toBe(" me"); expect(model.parts.length).toBe(3); - expect(renderer.count).toBe(1); + }); + // bug found while implementing tab completion + it('replace a part with an identical part with start position at end of previous part', function() { + const renderer = createRenderer(); + const pc = createPartCreator(); + const model = new EditorModel([ + pc.plain("hello "), + pc.pillCandidate("man"), + ], pc, renderer); + const range = model.startRange(model.positionForOffset(9, true)); // before "man" + range.expandBackwardsWhile((index, offset) => model.parts[index].text[offset] !== " "); + expect(range.text).toBe("man"); + range.replace([pc.pillCandidate(range.text)]); + expect(model.parts[0].type).toBe("plain"); + expect(model.parts[0].text).toBe("hello "); + expect(model.parts[1].type).toBe("pill-candidate"); + expect(model.parts[1].text).toBe("man"); + expect(model.parts.length).toBe(2); }); });