From b41787c3357c8d7546b19510fe54c4582d4e1a9c Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 23 Feb 2017 09:03:20 +0000 Subject: [PATCH 1/3] Initial work on improving invite dialog --- .../views/dialogs/ChatInviteDialog.js | 94 +++++++++++-------- 1 file changed, 57 insertions(+), 37 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index ca3b07aa00..ebfca2c48b 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -167,45 +167,55 @@ module.exports = React.createClass({ const query = ev.target.value; let queryList = []; - // Only do search if there is something to search - if (query.length > 0 && query != '@') { - // filter the known users list - queryList = this._userList.filter((user) => { - return this._matches(query, user); - }).map((user) => { - // Return objects, structure of which is defined - // by InviteAddressType - return { - addressType: 'mx', - address: user.userId, - displayName: user.displayName, - avatarMxc: user.avatarUrl, - isKnown: true, - } - }); + if (query.length < 2) { + return; + } - // If the query isn't a user we know about, but is a - // valid address, add an entry for that - if (queryList.length == 0) { - const addrType = getAddressType(query); - if (addrType !== null) { - queryList[0] = { - addressType: addrType, - address: query, - isKnown: false, - }; - if (this._cancelThreepidLookup) this._cancelThreepidLookup(); - if (addrType == 'email') { - this._lookupThreepid(addrType, query).done(); + if (this.queryChangedDebouncer) { + clearTimeout(this.queryChangedDebouncer); + } + this.queryChangedDebouncer = setTimeout(() => { + // Only do search if there is something to search + if (query.length > 0 && query != '@') { + // filter the known users list + queryList = this._userList.filter((user) => { + return this._matches(query, user); + }).sort((userA, userB) => { + return this._sortedMatches(query, userA, userB); + }).map((user) => { + // Return objects, structure of which is defined + // by InviteAddressType + return { + addressType: 'mx', + address: user.userId, + displayName: user.displayName, + avatarMxc: user.avatarUrl, + isKnown: true, + } + }); + + // If the query isn't a user we know about, but is a + // valid address, add an entry for that + if (queryList.length == 0) { + const addrType = getAddressType(query); + if (addrType !== null) { + queryList[0] = { + addressType: addrType, + address: query, + isKnown: false, + }; + if (this._cancelThreepidLookup) this._cancelThreepidLookup(); + if (addrType == 'email') { + this._lookupThreepid(addrType, query).done(); + } } } } - } - - this.setState({ - queryList: queryList, - error: false, - }); + this.setState({ + queryList: queryList, + error: false, + }); + }, 200); }, onDismissed: function(index) { @@ -349,8 +359,8 @@ module.exports = React.createClass({ return false; } - // direct prefix matches - if (name.indexOf(query) === 0 || uid.indexOf(query) === 0) { + // positional matches + if (name.indexOf(query) !== -1 || uid.indexOf(query) !== -1) { return true; } @@ -374,6 +384,16 @@ module.exports = React.createClass({ return false; }, + _sortedMatches: function(query, userA, userB) { + if (userA.displayName.startsWith(query) || userA.userId.startsWith(query)) { + return -1; + } + if (userA.displayName.length === query.length) { + return -1; + } + return 0; + }, + _isOnInviteList: function(uid) { for (let i = 0; i < this.state.inviteList.length; i++) { if ( From 439bde309ef28476b53c755c8d78b15adf1ecd35 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 23 Feb 2017 12:12:25 +0000 Subject: [PATCH 2/3] General ChatInviteDialog optimisations - Use avatar initial instead of "R" or "?" - Use Fuse.js to do case-insensitive fuzzy search. This allows for better sorting of results as well as search based on weighted keys (so userId has a high weight when the input starts with "@"). - Added debounce of 200ms to prevent analysis on every key stroke. Fuse seems to degrade performance vs. simple, non-fuzzy, unsorted matching, but the debounce should prevent too much computation. - Move the selection to the top when the query is changed. There's no point in staying mid-way through the items at that point. --- .../views/dialogs/ChatInviteDialog.js | 91 +++++++------------ .../views/elements/AddressSelector.js | 18 +++- src/components/views/elements/AddressTile.js | 19 ++-- 3 files changed, 58 insertions(+), 70 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index ebfca2c48b..0371f67302 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -26,6 +26,7 @@ import dis from '../../../dispatcher'; import Modal from '../../../Modal'; import AccessibleButton from '../elements/AccessibleButton'; import q from 'q'; +import Fuse from 'fuse.js'; const TRUNCATE_QUERY_LIST = 40; @@ -85,6 +86,21 @@ module.exports = React.createClass({ // Set the cursor at the end of the text input this.refs.textinput.value = this.props.value; } + // Create a Fuse instance for fuzzy searching this._userList + if (!this._fuse) { + this._fuse = new Fuse( + // Use an empty list at first that will later be populated + // (see this._updateUserList) + [], + { + shouldSort: true, + location: 0, // The index of the query in the test string + distance: 5, // The distance away from location the query can be + // 0.0 = exact match, 1.0 = match anything + threshold: 0.3, + } + ); + } this._updateUserList(); }, @@ -177,12 +193,15 @@ module.exports = React.createClass({ this.queryChangedDebouncer = setTimeout(() => { // Only do search if there is something to search if (query.length > 0 && query != '@') { - // filter the known users list - queryList = this._userList.filter((user) => { - return this._matches(query, user); - }).sort((userA, userB) => { - return this._sortedMatches(query, userA, userB); - }).map((user) => { + // Weighted keys prefer to match userIds when first char is @ + this._fuse.options.keys = [{ + name: 'displayName', + weight: query[0] === '@' ? 0.1 : 0.9, + },{ + name: 'userId', + weight: query[0] === '@' ? 0.9 : 0.1, + }]; + queryList = this._fuse.search(query).map((user) => { // Return objects, structure of which is defined // by InviteAddressType return { @@ -214,6 +233,8 @@ module.exports = React.createClass({ this.setState({ queryList: queryList, error: false, + }, () => { + this.addressSelector.moveSelectionTop(); }); }, 200); }, @@ -341,59 +362,15 @@ module.exports = React.createClass({ _updateUserList: new rate_limited_func(function() { // Get all the users this._userList = MatrixClientPeg.get().getUsers(); + // Remove current user + const meIx = this._userList.findIndex((u) => { + return u.userId === MatrixClientPeg.get().credentials.userId; + }); + this._userList.splice(meIx, 1); + + this._fuse.set(this._userList); }, 500), - // This is the search algorithm for matching users - _matches: function(query, user) { - var name = user.displayName.toLowerCase(); - var uid = user.userId.toLowerCase(); - query = query.toLowerCase(); - - // don't match any that are already on the invite list - if (this._isOnInviteList(uid)) { - return false; - } - - // ignore current user - if (uid === MatrixClientPeg.get().credentials.userId) { - return false; - } - - // positional matches - if (name.indexOf(query) !== -1 || uid.indexOf(query) !== -1) { - return true; - } - - // strip @ on uid and try matching again - if (uid.length > 1 && uid[0] === "@" && uid.substring(1).indexOf(query) === 0) { - return true; - } - - // Try to find the query following a "word boundary", except that - // this does avoids using \b because it only considers letters from - // the roman alphabet to be word characters. - // Instead, we look for the query following either: - // * The start of the string - // * Whitespace, or - // * A fixed number of punctuation characters - const expr = new RegExp("(?:^|[\\s\\(\)'\",\.-_@\?;:{}\\[\\]\\#~`\\*\\&\\$])" + escapeRegExp(query)); - if (expr.test(name)) { - return true; - } - - return false; - }, - - _sortedMatches: function(query, userA, userB) { - if (userA.displayName.startsWith(query) || userA.userId.startsWith(query)) { - return -1; - } - if (userA.displayName.length === query.length) { - return -1; - } - return 0; - }, - _isOnInviteList: function(uid) { for (let i = 0; i < this.state.inviteList.length; i++) { if ( diff --git a/src/components/views/elements/AddressSelector.js b/src/components/views/elements/AddressSelector.js index 9f37fa90ff..6bad15f7d0 100644 --- a/src/components/views/elements/AddressSelector.js +++ b/src/components/views/elements/AddressSelector.js @@ -61,6 +61,15 @@ export default React.createClass({ } }, + moveSelectionTop: function() { + if (this.state.selected > 0) { + this.setState({ + selected: 0, + hover: false, + }); + } + }, + moveSelectionUp: function() { if (this.state.selected > 0) { this.setState({ @@ -124,7 +133,14 @@ export default React.createClass({ // Saving the addressListElement so we can use it to work out, in the componentDidUpdate // method, how far to scroll when using the arrow keys addressList.push( -
{ this.addressListElement = ref; }} > +
{ this.addressListElement = ref; }} + >
); diff --git a/src/components/views/elements/AddressTile.js b/src/components/views/elements/AddressTile.js index 18492d8ae6..9961a1a428 100644 --- a/src/components/views/elements/AddressTile.js +++ b/src/components/views/elements/AddressTile.js @@ -64,19 +64,14 @@ export default React.createClass({ const address = this.props.address; const name = address.displayName || address.address; - let imgUrl; - if (address.avatarMxc) { - imgUrl = MatrixClientPeg.get().mxcUrlToHttp( - address.avatarMxc, 25, 25, 'crop' - ); - } + let imgUrls = []; - if (address.addressType === "mx") { - if (!imgUrl) imgUrl = 'img/icon-mx-user.svg'; + if (address.addressType === "mx" && address.avatarMxc) { + imgUrls.push(MatrixClientPeg.get().mxcUrlToHttp( + address.avatarMxc, 25, 25, 'crop' + )); } else if (address.addressType === 'email') { - if (!imgUrl) imgUrl = 'img/icon-email-user.svg'; - } else { - if (!imgUrl) imgUrl = "img/avatar-error.svg"; + imgUrls.push('img/icon-email-user.svg'); } // Removing networks for now as they're not really supported @@ -168,7 +163,7 @@ export default React.createClass({ return (
- +
{ info } { dismiss } From b822bc66eefe06a13298f2853a3665461736e2d0 Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 3 Mar 2017 10:22:02 +0000 Subject: [PATCH 3/3] Remove redundant null check --- .../views/dialogs/ChatInviteDialog.js | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/components/views/dialogs/ChatInviteDialog.js b/src/components/views/dialogs/ChatInviteDialog.js index 0371f67302..42394259fb 100644 --- a/src/components/views/dialogs/ChatInviteDialog.js +++ b/src/components/views/dialogs/ChatInviteDialog.js @@ -87,20 +87,18 @@ module.exports = React.createClass({ this.refs.textinput.value = this.props.value; } // Create a Fuse instance for fuzzy searching this._userList - if (!this._fuse) { - this._fuse = new Fuse( - // Use an empty list at first that will later be populated - // (see this._updateUserList) - [], - { - shouldSort: true, - location: 0, // The index of the query in the test string - distance: 5, // The distance away from location the query can be - // 0.0 = exact match, 1.0 = match anything - threshold: 0.3, - } - ); - } + this._fuse = new Fuse( + // Use an empty list at first that will later be populated + // (see this._updateUserList) + [], + { + shouldSort: true, + location: 0, // The index of the query in the test string + distance: 5, // The distance away from location the query can be + // 0.0 = exact match, 1.0 = match anything + threshold: 0.3, + } + ); this._updateUserList(); },