Merge pull request #3349 from matrix-org/bwindels/tab-complete-name

New composer: support forcing auto complete on name by hitting tab

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;

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; }
 }
 

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;

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;

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);
+        const range = model.startRange(caretPosition);
-        // expand range max 8 characters backwards from caret,
+        // 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 {
                 />
             </div>);
         }
-        return (<div className="mx_BasicMessageComposer">
+        const classes = classNames("mx_BasicMessageComposer", {
+            "mx_BasicMessageComposer_input_error": this.state.showVisualBell,
+        });
+        return (<div className={classes}>
             { autoComplete }
             <div
                 className="mx_BasicMessageComposer_input"

src/components/views/rooms/SendMessageComposer.js

@@ -279,22 +279,33 @@ export default class SendMessageComposer extends React.Component {
     };
 
     _insertMention(userId) {
+        const {model} = this;
+        const {partCreator} = model;
         const member = this.props.room.getMember(userId);
         const displayName = member ?
             member.rawDisplayName : userId;
-        const userPillPart = this.model.partCreator.userPill(displayName, userId);
+        const userPillPart = partCreator.userPill(displayName, userId);
-        this.model.insertPartsAt([userPillPart], this._editorRef.getCaret());
+        const caret = this._editorRef.getCaret();
+        const position = model.positionForOffset(caret.offset, caret.atNodeEnd);
+        model.transform(() => {
+            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();
     }

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) {

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
+        return newTextLength;
-        const lastPartIndex = insertIndex + parts.length - 1;
-        const newPosition = new DocumentPosition(lastPartIndex, newTextLength);
-        this._updateCallback(newPosition);
     }
 
     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;
+        // convert it back to position once split at start
-        // if both position are in the same part, and we split it at start position,
+        endPosition = endOffset.asPosition(this);
-        // the offset of the end position needs to be decreased by the offset of the start position
+        const newEndPartIndex = this._splitAt(endPosition);
-        const removedOffset = startPosition.index === endPosition.index ? startPosition.offset : 0;
-        const adjustedEndPosition = new DocumentPosition(
-            endPosition.index + idxDiff,
-            endPosition.offset - removedOffset,
-        );
-        const newEndPartIndex = this._splitAt(adjustedEndPosition);
         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;
     }
 }

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);
+    }
+}

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)};
     }
 

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);
+    }
 }

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;

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);
     });
 });