From e3f2eb5232f1195248ed6d0da0cda536d5f8c360 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 4 Jul 2017 14:44:55 +0100 Subject: [PATCH 1/9] =?UTF-8?q?Take=20RTE=20out=20of=20labs!=20?= =?UTF-8?q?=F0=9F=8E=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This stops react-sdk from tracking any state previously stored for the purposes of enabling or disabling the lab feature that enabled the new MessageComposer. It is now enabled permanently. This is being done with the hope that we can get more feedback for it so that when we release we can be confident that people will be OK with the changes it brings. --- src/UserSettingsStore.js | 11 ++--- src/components/views/rooms/MessageComposer.js | 48 +++++++------------ .../views/rooms/MessageComposerInput-test.js | 4 -- 3 files changed, 20 insertions(+), 43 deletions(-) diff --git a/src/UserSettingsStore.js b/src/UserSettingsStore.js index 009fdabb53..bbc436a1f6 100644 --- a/src/UserSettingsStore.js +++ b/src/UserSettingsStore.js @@ -27,11 +27,7 @@ export default { LABS_FEATURES: [ { name: "-", - id: 'rich_text_editor', - default: false, - }, - { - name: "-", + _tName: "Matrix Apps", // Translated! id: 'matrix_apps', default: false, }, @@ -39,8 +35,9 @@ export default { // horrible but it works. The locality makes this somewhat more palatable. doTranslations: function() { - this.LABS_FEATURES[0].name = _t("New Composer & Autocomplete"); - this.LABS_FEATURES[1].name = _t("Matrix Apps"); + this.LABS_FEATURES.forEach((f) => { + f.name = _t(f._tName); + }); }, loadProfileInfo: function() { diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index c83e32d9a8..7ca08a5a05 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -268,8 +268,7 @@ export default class MessageComposer extends React.Component { const uploadInputStyle = {display: 'none'}; const MemberAvatar = sdk.getComponent('avatars.MemberAvatar'); const TintableSvg = sdk.getComponent("elements.TintableSvg"); - const MessageComposerInput = sdk.getComponent("rooms.MessageComposerInput" + - (UserSettingsStore.isFeatureEnabled('rich_text_editor') ? "" : "Old")); + const MessageComposerInput = sdk.getComponent("rooms.MessageComposerInput"); const controls = []; @@ -352,8 +351,7 @@ export default class MessageComposer extends React.Component { title={_t("Show Text Formatting Toolbar")} src="img/button-text-formatting.svg" onClick={this.onToggleFormattingClicked} - style={{visibility: this.state.showFormatting || - !UserSettingsStore.isFeatureEnabled('rich_text_editor') ? 'hidden' : 'visible'}} + style={{visibility: this.state.showFormatting ? 'hidden' : 'visible'}} key="controls_formatting" /> ); @@ -390,18 +388,6 @@ export default class MessageComposer extends React.Component { ); } - let autoComplete; - if (UserSettingsStore.isFeatureEnabled('rich_text_editor')) { - autoComplete =
- -
; - } - - const {style, blockType} = this.state.inputState; const formatButtons = ["bold", "italic", "strike", "underline", "code", "quote", "bullet", "numbullet"].map( (name) => { @@ -429,22 +415,20 @@ export default class MessageComposer extends React.Component { {controls} - {UserSettingsStore.isFeatureEnabled('rich_text_editor') ? -
-
- {formatButtons} -
- - -
-
: null - } +
+
+ {formatButtons} +
+ + +
+
); } diff --git a/test/components/views/rooms/MessageComposerInput-test.js b/test/components/views/rooms/MessageComposerInput-test.js index 80fd158608..6d4b4e69cc 100644 --- a/test/components/views/rooms/MessageComposerInput-test.js +++ b/test/components/views/rooms/MessageComposerInput-test.js @@ -27,14 +27,10 @@ describe('MessageComposerInput', () => { mci = null, room = testUtils.mkStubRoom('!DdJkzRliezrwpNebLk:matrix.org'); - // TODO Remove when RTE is out of labs. - beforeEach(function() { testUtils.beforeEach(this); sandbox = testUtils.stubClient(sandbox); client = MatrixClientPeg.get(); - UserSettingsStore.isFeatureEnabled = sinon.stub() - .withArgs('rich_text_editor').returns(true); parentDiv = document.createElement('div'); document.body.appendChild(parentDiv); From e6ec5742bee6416ab35c2b7469de0840303733f9 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 4 Jul 2017 15:06:24 +0100 Subject: [PATCH 2/9] _t should be used on string literals For scripts to easily find translations --- src/UserSettingsStore.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/UserSettingsStore.js b/src/UserSettingsStore.js index bbc436a1f6..fef84468ec 100644 --- a/src/UserSettingsStore.js +++ b/src/UserSettingsStore.js @@ -27,7 +27,6 @@ export default { LABS_FEATURES: [ { name: "-", - _tName: "Matrix Apps", // Translated! id: 'matrix_apps', default: false, }, @@ -35,9 +34,7 @@ export default { // horrible but it works. The locality makes this somewhat more palatable. doTranslations: function() { - this.LABS_FEATURES.forEach((f) => { - f.name = _t(f._tName); - }); + this.LABS_FEATURES[0].name = _t("Matrix Apps"); }, loadProfileInfo: function() { From 77348e6201edaa5b4302ce150e0be008d0908566 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Tue, 4 Jul 2017 15:20:00 +0100 Subject: [PATCH 3/9] Remove spurious, unused code --- src/components/views/rooms/MessageComposer.js | 18 ------------------ .../views/rooms/MessageComposerInput.js | 7 ------- 2 files changed, 25 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 7ca08a5a05..69ccc3d2f8 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -227,21 +227,6 @@ export default class MessageComposer extends React.Component { this.setState({inputState}); } - onUpArrow() { - return this.refs.autocomplete.onUpArrow(); - } - - onDownArrow() { - return this.refs.autocomplete.onDownArrow(); - } - - _tryComplete(): boolean { - if (this.refs.autocomplete) { - return this.refs.autocomplete.onCompletionClicked(); - } - return false; - } - _onAutocompleteConfirm(range, completion) { if (this.messageComposerInput) { this.messageComposerInput.setDisplayedCompletion(range, completion); @@ -366,10 +351,7 @@ export default class MessageComposer extends React.Component { room={this.props.room} placeholder={placeholderText} tryComplete={this._tryComplete} - onUpArrow={this.onUpArrow} - onDownArrow={this.onDownArrow} onFilesPasted={this.uploadFiles} - tabComplete={this.props.tabComplete} // used for old messagecomposerinput/tabcomplete onContentChanged={this.onInputContentChanged} onInputStateChanged={this.onInputStateChanged} />, formattingButton, diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 818c108211..27fd6d4190 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -887,14 +887,7 @@ MessageComposerInput.propTypes = { // called with current plaintext content (as a string) whenever it changes onContentChanged: React.PropTypes.func, - onUpArrow: React.PropTypes.func, - - onDownArrow: React.PropTypes.func, - onFilesPasted: React.PropTypes.func, - // attempts to confirm currently selected completion, returns whether actually confirmed - tryComplete: React.PropTypes.func, - onInputStateChanged: React.PropTypes.func, }; From 7a8f524f4a017862396b28865d757e6e9a79b4ec Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 7 Jul 2017 15:30:31 +0100 Subject: [PATCH 4/9] Remove two possible sources for the "AutoComplete stays visible bug which is now https://github.com/vector-im/riot-web/issues/4537 <- there. This does two things: - Track which query was the most recent one requesting completion and only process completions for that one. (In this case the empty string "" doesn't have any completions but we still track it so that previous calls with non-empty queries would not race and cause completions to be shown when we actuall don't want any.) - Make the "do we want to show the AutoComplete box?" logic a bit more sane --- src/components/views/rooms/Autocomplete.js | 29 +++++++++++-------- .../views/rooms/MessageComposerInput.js | 3 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/components/views/rooms/Autocomplete.js b/src/components/views/rooms/Autocomplete.js index 807e93cc0b..026be0da62 100644 --- a/src/components/views/rooms/Autocomplete.js +++ b/src/components/views/rooms/Autocomplete.js @@ -50,6 +50,7 @@ export default class Autocomplete extends React.Component { } complete(query, selection) { + this.queryRequested = query; if (this.debounceCompletionsRequest) { clearTimeout(this.debounceCompletionsRequest); } @@ -74,16 +75,25 @@ export default class Autocomplete extends React.Component { const deferred = Q.defer(); this.debounceCompletionsRequest = setTimeout(() => { - getCompletions( - query, selection, this.state.forceComplete, - ).then((completions) => { - this.processCompletions(completions); + this.processQuery(query, selection).then(() => { deferred.resolve(); }); }, autocompleteDelay); return deferred.promise; } + processQuery(query, selection) { + return getCompletions( + query, selection, this.state.forceComplete, + ).then((completions) => { + // Only ever process the completions for the most recent query being processed + if (query !== this.queryRequested) { + return; + } + this.processCompletions(completions); + }); + } + processCompletions(completions) { const completionList = flatMap(completions, (provider) => provider.completions); @@ -105,14 +115,9 @@ export default class Autocomplete extends React.Component { } let hide = this.state.hide; - // These are lists of booleans that indicate whether whether the corresponding provider had a matching pattern - const oldMatches = this.state.completions.map((completion) => !!completion.command.command), - newMatches = completions.map((completion) => !!completion.command.command); - - // So, essentially, we re-show autocomplete if any provider finds a new pattern or stops finding an old one - if (!isEqual(oldMatches, newMatches)) { - hide = false; - } + // If `completion.command.command` is truthy, then a provider has matched with the query + const anyMatches = completions.some((completion) => !!completion.command.command); + hide = !anyMatches; this.setState({ completions, diff --git a/src/components/views/rooms/MessageComposerInput.js b/src/components/views/rooms/MessageComposerInput.js index 3465b2ad14..488a8229a6 100644 --- a/src/components/views/rooms/MessageComposerInput.js +++ b/src/components/views/rooms/MessageComposerInput.js @@ -514,6 +514,7 @@ export default class MessageComposerInput extends React.Component { const currentBlockType = RichUtils.getCurrentBlockType(this.state.editorState); // If we're in any of these three types of blocks, shift enter should insert soft newlines // And just enter should end the block + // XXX: Empirically enter does not end these blocks if(['blockquote', 'unordered-list-item', 'ordered-list-item'].includes(currentBlockType)) { return false; } @@ -629,8 +630,6 @@ export default class MessageComposerInput extends React.Component { editorState: this.createEditorState(), }); - this.autocomplete.hide(); - return true; } From f2d243443b76560cc52c3354333d22bde3f2a9d0 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 7 Jul 2017 17:44:25 +0100 Subject: [PATCH 5/9] Suppress more errors from spurious postMessage calls on the demo instance --- src/ScalarMessaging.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ScalarMessaging.js b/src/ScalarMessaging.js index 1104458f22..e7767cb3cd 100644 --- a/src/ScalarMessaging.js +++ b/src/ScalarMessaging.js @@ -481,7 +481,7 @@ const onMessage = function(event) { // All strings start with the empty string, so for sanity return if the length // of the event origin is 0. let url = SdkConfig.get().integrations_ui_url; - if (event.origin.length === 0 || !url.startsWith(event.origin)) { + if (event.origin.length === 0 || !url.startsWith(event.origin) || !event.data.action) { return; // don't log this - debugging APIs like to spam postMessage which floods the log otherwise } From 1e713557bb7e9183af20fb001f05d276353df85f Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 7 Jul 2017 18:34:40 +0100 Subject: [PATCH 6/9] PR feedback --- src/components/structures/GroupView.js | 6 ++- src/components/structures/MyGroups.js | 9 ++-- .../views/dialogs/CreateGroupDialog.js | 51 ++++++++++--------- src/i18n/strings/en_EN.json | 5 +- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/components/structures/GroupView.js b/src/components/structures/GroupView.js index 88c73b75a8..3f321d453d 100644 --- a/src/components/structures/GroupView.js +++ b/src/components/structures/GroupView.js @@ -79,7 +79,7 @@ export default React.createClass({ } let nameNode; - if (summary.profile.name) { + if (summary.profile && summary.profile.name) { nameNode =
{summary.profile.name} @@ -92,6 +92,8 @@ export default React.createClass({
; } + const groupAvatarUrl = summary.profile ? summary.profile.avatar_url : null; + return (
@@ -99,7 +101,7 @@ export default React.createClass({
diff --git a/src/components/structures/MyGroups.js b/src/components/structures/MyGroups.js index 49a2367db8..3eb694acce 100644 --- a/src/components/structures/MyGroups.js +++ b/src/components/structures/MyGroups.js @@ -61,9 +61,6 @@ export default withMatrixClient(React.createClass({ this._fetch(); }, - componentWillUnmount: function() { - }, - _onCreateGroupClick: function() { const CreateGroupDialog = sdk.getComponent("dialogs.CreateGroupDialog"); Modal.createDialog(CreateGroupDialog); @@ -73,7 +70,7 @@ export default withMatrixClient(React.createClass({ this.props.matrixClient.getJoinedGroups().done((result) => { this.setState({groups: result.groups, error: null}); }, (err) => { - this.setState({result: null, error: err}); + this.setState({groups: null, error: err}); }); }, @@ -93,12 +90,12 @@ export default withMatrixClient(React.createClass({ ); }); content =
-
{_t('You are a member of these groups')}:
+
{_t('You are a member of these groups:')}
{groupNodes}
; } else if (this.state.error) { content =
- Error whilst fetching joined groups + {_t('Error whilst fetching joined groups')}
; } else { content = ; diff --git a/src/components/views/dialogs/CreateGroupDialog.js b/src/components/views/dialogs/CreateGroupDialog.js index c436d938df..23194f20a5 100644 --- a/src/components/views/dialogs/CreateGroupDialog.js +++ b/src/components/views/dialogs/CreateGroupDialog.js @@ -130,10 +130,10 @@ export default React.createClass({ render: function() { const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog'); - const Loader = sdk.getComponent("elements.Spinner"); + const Spinner = sdk.getComponent('elements.Spinner'); if (this.state.creating) { - return ; + return ; } let createErrorNode; @@ -154,29 +154,32 @@ export default React.createClass({ >
-
- +
+
+ +
+
+ +
-
- -
-
-
- -
-
- +
+
+ +
+
+ +
{this.state.groupIdError} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 6647126c28..dbef0fc7fe 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -948,9 +948,10 @@ "Group IDs must be of the form +localpart:%(domain)s": "Group IDs must be of the form +localpart:%(domain)s", "It is currently only possible to create groups on your own home server: use a group ID ending with %(domain)s": "It is currently only possible to create groups on your own home server: use a group ID ending with %(domain)s", "Room creation failed": "Room creation failed", - "You are a member of these groups": "You are a member of these groups", + "You are a member of these groups:": "You are a member of these groups:", "Create a group to represent your community! Define a set of rooms and your own custom homepage to mark out your space in the Matrix universe.": "Create a group to represent your community! Define a set of rooms and your own custom homepage to mark out your space in the Matrix universe.", "Join an existing group": "Join an existing group", "To join an exisitng group you'll have to know its group identifier; this will look something like +example:matrix.org.": "To join an exisitng group you'll have to know its group identifier; this will look something like +example:matrix.org.", - "Autocomplete Delay (ms):": "Autocomplete Delay (ms):" + "Autocomplete Delay (ms):": "Autocomplete Delay (ms):", + "Error whilst fetching joined groups": "Error whilst fetching joined groups" } From 9a272d496596a6512c324d6e3674896e650949c3 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 7 Jul 2017 19:02:51 +0100 Subject: [PATCH 7/9] Only allow completion of emoji in certain circumstances Which are: - the emoji to complete is at the start of the query - there is a whitespace character before the emoji - there is an emoji before the emoji (so that several emoji can be input in-a-row) Fixes https://github.com/vector-im/riot-web/issues/4498 (although it seems to be fixed through some other fix) --- src/autocomplete/EmojiProvider.js | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/autocomplete/EmojiProvider.js b/src/autocomplete/EmojiProvider.js index 35e9cc7b68..f70ff7f200 100644 --- a/src/autocomplete/EmojiProvider.js +++ b/src/autocomplete/EmojiProvider.js @@ -18,7 +18,7 @@ limitations under the License. import React from 'react'; import { _t } from '../languageHandler'; import AutocompleteProvider from './AutocompleteProvider'; -import {emojioneList, shortnameToImage, shortnameToUnicode, asciiRegexp} from 'emojione'; +import {emojioneList, shortnameToImage, shortnameToUnicode, asciiRegexp, unicodeRegexp} from 'emojione'; import FuzzyMatcher from './FuzzyMatcher'; import sdk from '../index'; import {PillCompletion} from './Components'; @@ -41,7 +41,15 @@ const CATEGORY_ORDER = [ ]; // Match for ":wink:" or ascii-style ";-)" provided by emojione -const EMOJI_REGEX = new RegExp('(' + asciiRegexp + '|:\\w*:?)$', 'g'); +// (^|\s|(emojiUnicode)) to make sure we're either at the start of the string or there's a +// whitespace character or an emoji before the emoji. The reason for unicodeRegexp is +// that we need to support inputting multiple emoji with no space between them. +const EMOJI_REGEX = new RegExp('(?:^|\\s|' + unicodeRegexp + ')(' + asciiRegexp + '|:\\w*:?)$', 'g'); + +// We also need to match the non-zero-length prefixes to remove them from the final match, +// and update the range so that we don't replace the whitespace or the previous emoji. +const MATCH_PREFIX_REGEX = new RegExp('(\\s|' + unicodeRegexp + ')'); + const EMOJI_SHORTNAMES = Object.keys(EmojiData).map((key) => EmojiData[key]).sort( (a, b) => { if (a.category === b.category) { @@ -73,9 +81,18 @@ export default class EmojiProvider extends AutocompleteProvider { const EmojiText = sdk.getComponent('views.elements.EmojiText'); let completions = []; - let {command, range} = this.getCurrentCommand(query, selection); + const {command, range} = this.getCurrentCommand(query, selection); if (command) { - completions = this.matcher.match(command[0]).map(result => { + let matchedString = command[0]; + + // Remove prefix of any length (single whitespace or unicode emoji) + const prefixMatch = MATCH_PREFIX_REGEX.exec(matchedString); + if (prefixMatch) { + matchedString = matchedString.slice(prefixMatch[0].length); + range.start += prefixMatch[0].length; + } + + completions = this.matcher.match(matchedString).map((result) => { const {shortname} = result; const unicode = shortnameToUnicode(shortname); return { From 62ee0f4e0235c40e8db4c850687296baf20eac9a Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 7 Jul 2017 19:43:30 +0100 Subject: [PATCH 8/9] Fix accepting invites Accepting an invite would cause a room to arrive via /sync only for it to throw an error in the auto complete code and cause the client to go wibbly (infinite spinner or preview bar). The logs that lead to the debugging of this are https://github.com/matrix-org/riot-web-rageshakes/issues/239 Hopefully the error being throw isn't totally unrelated but looking at the sync handling for inviteRooms in sync.js, new rooms are stored and _then_ the Room event is emitted. The Room event could trigger setUserListFromRoom, which is where the bug was. So the room should have been stored regardless of this bug and the client should have been recoverable by swapping away and viewing the room again. --- src/autocomplete/UserProvider.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/autocomplete/UserProvider.js b/src/autocomplete/UserProvider.js index 26ec15e124..0025a3c5e9 100644 --- a/src/autocomplete/UserProvider.js +++ b/src/autocomplete/UserProvider.js @@ -91,8 +91,8 @@ export default class UserProvider extends AutocompleteProvider { if (member.userId !== currentUserId) return true; }); - this.users = _sortBy(this.users, (completion) => - 1E20 - lastSpoken[completion.user.userId] || 1E20, + this.users = _sortBy(this.users, (member) => + 1E20 - lastSpoken[member.userId] || 1E20, ); this.matcher.setObjects(this.users); From 86e717f30db4fe2f2f396656b29b0f4e74eb9792 Mon Sep 17 00:00:00 2001 From: David Baker Date: Sun, 9 Jul 2017 12:34:50 +0100 Subject: [PATCH 9/9] Fix indenting Also autocomplete delay was duplicated --- src/i18n/strings/en_EN.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index dbef0fc7fe..fc4257cbc1 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -952,6 +952,5 @@ "Create a group to represent your community! Define a set of rooms and your own custom homepage to mark out your space in the Matrix universe.": "Create a group to represent your community! Define a set of rooms and your own custom homepage to mark out your space in the Matrix universe.", "Join an existing group": "Join an existing group", "To join an exisitng group you'll have to know its group identifier; this will look something like +example:matrix.org.": "To join an exisitng group you'll have to know its group identifier; this will look something like +example:matrix.org.", - "Autocomplete Delay (ms):": "Autocomplete Delay (ms):", - "Error whilst fetching joined groups": "Error whilst fetching joined groups" + "Error whilst fetching joined groups": "Error whilst fetching joined groups" }