From 713205e0ab11f05eabffaae71face5d130ccb5b3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:10:11 +0200 Subject: [PATCH 01/14] close autocomplete when removing auto-completed part --- src/editor/model.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/editor/model.js b/src/editor/model.js index 9d129afa69..7f87bdea23 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -90,10 +90,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; } } From 0f6465a1dbc1af6efde1638bbcd565bdceb6d0a2 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:11:18 +0200 Subject: [PATCH 02/14] don't close autocomplete when hitting tab that's not what the slate impl does and it's not an improvement --- src/editor/autocomplete.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/editor/autocomplete.js b/src/editor/autocomplete.js index ac662c32d8..cf3082ce13 100644 --- a/src/editor/autocomplete.js +++ b/src/editor/autocomplete.js @@ -52,9 +52,6 @@ export default class AutocompleteWrapperModel { } else { await acComponent.moveSelection(e.shiftKey ? -1 : +1); } - this._updateCallback({ - close: true, - }); } onUpArrow() { From f76a23d5dd3c78985c86bf9e255c7f8b19b47a33 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:12:44 +0200 Subject: [PATCH 03/14] return promise from updating autocomplete so one can await if needed --- src/components/views/rooms/BasicMessageComposer.js | 2 +- src/editor/model.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 662167b714..ad0e76c3e4 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -304,7 +304,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 diff --git a/src/editor/model.js b/src/editor/model.js index 7f87bdea23..b020bd8fb5 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -186,13 +186,14 @@ 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) { @@ -218,13 +219,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}) => { From 68c2bb7ca60caccc67d146fbfcaa05c2fb3ec114 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:15:10 +0200 Subject: [PATCH 04/14] introduce `transform` method so update can be called with a position and also for multiple transformations at once. This removes the need to call the update callback from `replaceRange()` as well --- src/editor/model.js | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/editor/model.js b/src/editor/model.js index b020bd8fb5..4c657f3168 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; } /** @@ -170,7 +174,6 @@ export default class EditorModel { } 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; @@ -191,7 +194,6 @@ export default class EditorModel { const transformAddedLen = this._transform(newPosition, inputType, diff); newPosition = this.positionForOffset(caretOffset + transformAddedLen, true); } - this._updateInProgress = false; this._updateCallback(newPosition, inputType, diff); return acPromise; } @@ -422,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; } } From f02713d08eac01dba489f572f63899a400c508f7 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:16:43 +0200 Subject: [PATCH 05/14] force completion when hitting tab by replacing word before caret with pill-candidate and forcing auto complete --- .../views/rooms/BasicMessageComposer.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index ad0e76c3e4..10fa676989 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -269,6 +269,9 @@ export default class BasicMessageEditor extends React.Component { default: return; // don't preventDefault on anything else } + } else if (event.key === "Tab") { + this._tabCompleteName(event); + handled = true; } } if (handled) { @@ -277,6 +280,22 @@ export default class BasicMessageEditor extends React.Component { } } + async _tabCompleteName(event) { + 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 model.transform(() => { + const addedLen = range.replace([partCreator.pillCandidate(range.text)]); + return model.positionForOffset(caret.offset + addedLen, true); + }); + await model.autoComplete.onTab(); + } + isModified() { return this._modifiedFlag; } From f5bb872efa6623bab3fe03036be87471a8f0ea3b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:17:41 +0200 Subject: [PATCH 06/14] some cleanup --- src/components/views/rooms/BasicMessageComposer.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 10fa676989..4aa622b6c2 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -75,10 +75,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 +91,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 +100,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 +161,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) { From e0ec827a64eacb70cb45dc4540f69f136f07fca5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:18:09 +0200 Subject: [PATCH 07/14] extra docs --- src/editor/parts.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/editor/parts.js b/src/editor/parts.js index f9b4243de4..bc420ecde1 100644 --- a/src/editor/parts.js +++ b/src/editor/parts.js @@ -366,6 +366,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)}; } From 8e66d382deee95f1e2bb82a1171d23846e39aeae Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:18:22 +0200 Subject: [PATCH 08/14] don't crash on race with room members and initial composer render not ideal, but for now this prevents a crash at startup when a user-pill is persisted in local storage --- src/editor/parts.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/editor/parts.js b/src/editor/parts.js index bc420ecde1..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( From d8bb9ecedfa4f0e8d7d87dea6b4f5c6fe59ab1c0 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:37:04 +0200 Subject: [PATCH 09/14] bring insert method inline with transform callback, add docs before the insertPartsAt method would call the update callback on its own, but now we have the concept of a transformation session, so lets bring the API in line --- .../views/rooms/SendMessageComposer.js | 19 +++++++++++++++---- src/editor/model.js | 17 ++++++++++------- src/editor/range.js | 6 ++++++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/components/views/rooms/SendMessageComposer.js b/src/components/views/rooms/SendMessageComposer.js index c8fac0b667..698356a175 100644 --- a/src/components/views/rooms/SendMessageComposer.js +++ b/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); - this.model.insertPartsAt([userPillPart], this._editorRef.getCaret()); + const userPillPart = partCreator.userPill(displayName, userId); + 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(); } diff --git a/src/editor/model.js b/src/editor/model.js index 4c657f3168..ca04d9fdd0 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -158,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) { @@ -167,10 +173,7 @@ 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) { @@ -403,7 +406,7 @@ export default class EditorModel { return new Range(this, position); } - // called from Range.replace + //mostly internal, called from Range.replace replaceRange(startPosition, endPosition, parts) { const newStartPartIndex = this._splitAt(startPosition); const idxDiff = newStartPartIndex - startPosition.index; 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; From 994bcb5c85058ffbe071976c1c3620eca74d8a00 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 27 Aug 2019 16:39:30 +0200 Subject: [PATCH 10/14] dont expect rendered to be called from `range.replace()` anymore as this is now called from the `transform` method, unused in this test --- test/editor/range-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/editor/range-test.js b/test/editor/range-test.js index 5a95da952d..e5fa48ea15 100644 --- a/test/editor/range-test.js +++ b/test/editor/range-test.js @@ -60,7 +60,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(); @@ -83,6 +82,5 @@ 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); }); }); From c44fbb73d0d2f27415330eb172613cc4cc93b9c5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 28 Aug 2019 15:52:39 +0200 Subject: [PATCH 11/14] fix bug when replacing range starting at end of previous part --- src/editor/model.js | 15 ++++++--------- src/editor/offset.js | 26 ++++++++++++++++++++++++++ src/editor/position.js | 16 ++++++++++++++++ test/editor/range-test.js | 20 ++++++++++++++++++-- 4 files changed, 66 insertions(+), 11 deletions(-) create mode 100644 src/editor/offset.js diff --git a/src/editor/model.js b/src/editor/model.js index ca04d9fdd0..59371cc3e6 100644 --- a/src/editor/model.js +++ b/src/editor/model.js @@ -408,16 +408,13 @@ export default class EditorModel { //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); } 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/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/test/editor/range-test.js b/test/editor/range-test.js index e5fa48ea15..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"); @@ -73,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,4 +81,22 @@ describe('editor/range', function() { expect(model.parts[2].text).toBe(" me"); expect(model.parts.length).toBe(3); }); + // 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); + }); }); From 85efb71a23033842f5a540f7e1d4a0ed2ac2a847 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 28 Aug 2019 15:53:16 +0200 Subject: [PATCH 12/14] add visual bell when no replacements are available also add try/catch in _tabCompleteName so errors don't get swallowed --- .../views/rooms/_BasicMessageComposer.scss | 9 +++ .../views/rooms/BasicMessageComposer.js | 56 +++++++++++++------ src/editor/autocomplete.js | 6 +- 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/res/css/views/rooms/_BasicMessageComposer.scss b/res/css/views/rooms/_BasicMessageComposer.scss index b6035e5859..a4b5bb51d0 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: #faa; } + 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/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 4aa622b6c2..19304ec557 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'; @@ -271,7 +273,7 @@ export default class BasicMessageEditor extends React.Component { return; // don't preventDefault on anything else } } else if (event.key === "Tab") { - this._tabCompleteName(event); + this._tabCompleteName(); handled = true; } } @@ -281,20 +283,30 @@ export default class BasicMessageEditor extends React.Component { } } - async _tabCompleteName(event) { - 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 model.transform(() => { - const addedLen = range.replace([partCreator.pillCandidate(range.text)]); - return model.positionForOffset(caret.offset + addedLen, true); - }); - await model.autoComplete.onTab(); + 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() { @@ -324,7 +336,14 @@ 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 => new Promise(resolve => this.setState({query}, resolve)), + query => { + return new Promise(resolve => this.setState({query}, resolve)); + // if setState + // if (this.state.query === query) { + // return Promise.resolve(); + // } else { + // } + }, )); this.historyManager = new HistoryManager(partCreator); // initial render of model @@ -365,7 +384,10 @@ export default class BasicMessageEditor extends React.Component { /> ); } - return (
+ const classes = classNames("mx_BasicMessageComposer", { + "mx_BasicMessageComposer_input_error": this.state.showVisualBell, + }); + return (
{ autoComplete }
Date: Wed, 28 Aug 2019 17:53:03 +0200 Subject: [PATCH 13/14] remove leftover code --- src/components/views/rooms/BasicMessageComposer.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 19304ec557..49815c6f23 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -336,14 +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 => { - return new Promise(resolve => this.setState({query}, resolve)); - // if setState - // if (this.state.query === query) { - // return Promise.resolve(); - // } else { - // } - }, + query => new Promise(resolve => this.setState({query}, resolve)), )); this.historyManager = new HistoryManager(partCreator); // initial render of model From eddaece43e871975d112735e544857d42ca3ef9d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 28 Aug 2019 18:00:57 +0200 Subject: [PATCH 14/14] add visual bell color to theme + choose better value for dark mode --- res/css/views/rooms/_BasicMessageComposer.scss | 2 +- res/css/views/rooms/_MessageComposer.scss | 2 +- res/themes/dark/css/_dark.scss | 2 ++ res/themes/light/css/_light.scss | 2 ++ 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/res/css/views/rooms/_BasicMessageComposer.scss b/res/css/views/rooms/_BasicMessageComposer.scss index a4b5bb51d0..bce0ecf325 100644 --- a/res/css/views/rooms/_BasicMessageComposer.scss +++ b/res/css/views/rooms/_BasicMessageComposer.scss @@ -28,7 +28,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/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;