From 0dde891d4cc941aa16f8a8f61954e2e0f4e5837b Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Jul 2016 11:25:45 +0100 Subject: [PATCH 1/8] Order tab complete by most recently spoke Fixes https://github.com/vector-im/vector-web/issues/1741 --- src/TabCompleteEntries.js | 20 +++++++++++++++++++- src/components/structures/RoomView.js | 2 +- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/TabCompleteEntries.js b/src/TabCompleteEntries.js index a23050063f..488aaa57a6 100644 --- a/src/TabCompleteEntries.js +++ b/src/TabCompleteEntries.js @@ -113,8 +113,26 @@ class MemberEntry extends Entry { } } -MemberEntry.fromMemberList = function(members) { +MemberEntry.fromMemberList = function(room, members) { + // build up a dict of when, in the history we have cached, + // each member last spoke + const lastSpoke = {}; + const timelineEvents = room.getLiveTimeline().getEvents(); + for (var i = timelineEvents.length - 1; i >= 0; --i) { + const ev = timelineEvents[i]; + lastSpoke[ev.sender.userId] = ev.getTs(); + } + return members.sort(function(a, b) { + const lastSpokeA = lastSpoke[a.userId] || 0; + const lastSpokeB = lastSpoke[b.userId] || 0; + + if (lastSpokeA != lastSpokeB) { + // B - A here because the highest value + // is most recent + return lastSpokeB - lastSpokeA; + } + var userA = a.user; var userB = b.user; if (userA && !userB) { diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index cfd359ea01..64a29f9ffc 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -533,7 +533,7 @@ module.exports = React.createClass({ UserProvider.getInstance().setUserList(members); this.tabComplete.setCompletionList( - MemberEntry.fromMemberList(members).concat( + MemberEntry.fromMemberList(this.state.room, members).concat( CommandEntry.fromCommands(SlashCommands.getCommandList()) ) ); From 2ce521fe387428f105013da133c3bf57d291e03c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Jul 2016 11:40:17 +0100 Subject: [PATCH 2/8] Fix null error in TabComplete .sende ris sometimes null: use getSender() which isn't and returns the userId which is what we actually want --- src/TabCompleteEntries.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/TabCompleteEntries.js b/src/TabCompleteEntries.js index 488aaa57a6..3cfe07c7c0 100644 --- a/src/TabCompleteEntries.js +++ b/src/TabCompleteEntries.js @@ -120,7 +120,7 @@ MemberEntry.fromMemberList = function(room, members) { const timelineEvents = room.getLiveTimeline().getEvents(); for (var i = timelineEvents.length - 1; i >= 0; --i) { const ev = timelineEvents[i]; - lastSpoke[ev.sender.userId] = ev.getTs(); + lastSpoke[ev.getSender()] = ev.getTs(); } return members.sort(function(a, b) { From f1d72296b76e87d28753281ef6d1a8d481ef8a4c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Jul 2016 14:06:31 +0100 Subject: [PATCH 3/8] Fix last-spoke order Turns out this timeline is the other way around, so loop through the other way --- src/TabCompleteEntries.js | 3 +-- src/components/structures/RoomView.js | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/TabCompleteEntries.js b/src/TabCompleteEntries.js index 3cfe07c7c0..419b3d7942 100644 --- a/src/TabCompleteEntries.js +++ b/src/TabCompleteEntries.js @@ -118,8 +118,7 @@ MemberEntry.fromMemberList = function(room, members) { // each member last spoke const lastSpoke = {}; const timelineEvents = room.getLiveTimeline().getEvents(); - for (var i = timelineEvents.length - 1; i >= 0; --i) { - const ev = timelineEvents[i]; + for (const ev of room.getLiveTimeline().getEvents()) { lastSpoke[ev.getSender()] = ev.getTs(); } diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 64a29f9ffc..71edbf162d 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -360,6 +360,10 @@ module.exports = React.createClass({ }); } } + + // update ther tab complete list as it depends on who most recently spoke, + // and that has probably just changed + this._updateTabCompleteList(); }, // called when state.room is first initialised (either at initial load, From d5bed78a54926b591422c60fdd3ff3b849c95689 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Jul 2016 16:10:27 +0100 Subject: [PATCH 4/8] Rejig tab complete to make it faster Now do a lot less when people speak. Also move more of the tab completion logic into TabComplete.js and out of RoomView. --- src/TabComplete.js | 77 +++++++++++++++++--- src/TabCompleteEntries.js | 40 +--------- src/components/structures/RoomStatusBar.js | 16 ++-- src/components/structures/RoomView.js | 66 ++++++++--------- src/components/views/rooms/TabCompleteBar.js | 8 +- 5 files changed, 113 insertions(+), 94 deletions(-) diff --git a/src/TabComplete.js b/src/TabComplete.js index 7da8bde76b..5b7be7c286 100644 --- a/src/TabComplete.js +++ b/src/TabComplete.js @@ -13,7 +13,10 @@ 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. */ -var Entry = require("./TabCompleteEntries").Entry; + +import { Entry, MemberEntry, CommandEntry } from './TabCompleteEntries'; +import SlashCommands from './SlashCommands'; +import MatrixClientPeg from './MatrixClientPeg'; const DELAY_TIME_MS = 1000; const KEY_TAB = 9; @@ -45,23 +48,34 @@ class TabComplete { this.isFirstWord = false; // true if you tab-complete on the first word this.enterTabCompleteTimerId = null; this.inPassiveMode = false; + this.memberTabOrder = {}; + this.memberOrderSeq = 0; } /** - * @param {Entry[]} completeList + * Call this when a a UI element representing a tab complete entry has been clicked + * @param {entry} The entry that was clicked */ - setCompletionList(completeList) { - this.list = completeList; + onEntryClick(entry) { if (this.opts.onClickCompletes) { - // assign onClick listeners for each entry to complete the text - this.list.forEach((l) => { - l.onClick = () => { - this.completeTo(l); - } - }); + this.completeTo(entry); } } + loadEntries(room) { + this._makeEntries(room); + this._initSorting(room); + this._sortEntries(); + } + + onMemberSpoke(member) { + if (this.memberTabOrder[member.userId] === undefined) { + this.list.push(new MemberEntry(member)); + } + this.memberTabOrder[member.userId] = this.memberOrderSeq++; + this._sortEntries(); + } + /** * @param {DOMElement} */ @@ -307,6 +321,49 @@ class TabComplete { this.opts.onStateChange(this.completing); } } + + _sortEntries() { + // largest comes first + const KIND_ORDER = { + command: 1, + member: 2, + }; + + this.list.sort((a, b) => { + const kindOrderDifference = KIND_ORDER[b.kind] - KIND_ORDER[a.kind]; + if (kindOrderDifference != 0) { + return kindOrderDifference; + } + + if (a.kind == 'member') { + return this.memberTabOrder[b.member.userId] - this.memberTabOrder[a.member.userId]; + } + + // anything else we have no ordering for + return 0; + }); + } + + _makeEntries(room) { + const myUserId = MatrixClientPeg.get().credentials.userId; + + const members = room.getJoinedMembers().filter(function(member) { + if (member.userId !== myUserId) return true; + }); + + this.list = MemberEntry.fromMemberList(members).concat( + CommandEntry.fromCommands(SlashCommands.getCommandList()) + ); + } + + _initSorting(room) { + this.memberTabOrder = {}; + this.memberOrderSeq = 0; + + for (const ev of room.getLiveTimeline().getEvents()) { + this.memberTabOrder[ev.getSender()] = this.memberOrderSeq++; + } + } }; module.exports = TabComplete; diff --git a/src/TabCompleteEntries.js b/src/TabCompleteEntries.js index 419b3d7942..4a28103210 100644 --- a/src/TabCompleteEntries.js +++ b/src/TabCompleteEntries.js @@ -69,6 +69,7 @@ class Entry { class CommandEntry extends Entry { constructor(cmd, cmdWithArgs) { super(cmdWithArgs); + this.kind = 'command'; this.cmd = cmd; } @@ -95,6 +96,7 @@ class MemberEntry extends Entry { constructor(member) { super(member.name || member.userId); this.member = member; + this.kind = 'member'; } getImageJsx() { @@ -113,42 +115,8 @@ class MemberEntry extends Entry { } } -MemberEntry.fromMemberList = function(room, members) { - // build up a dict of when, in the history we have cached, - // each member last spoke - const lastSpoke = {}; - const timelineEvents = room.getLiveTimeline().getEvents(); - for (const ev of room.getLiveTimeline().getEvents()) { - lastSpoke[ev.getSender()] = ev.getTs(); - } - - return members.sort(function(a, b) { - const lastSpokeA = lastSpoke[a.userId] || 0; - const lastSpokeB = lastSpoke[b.userId] || 0; - - if (lastSpokeA != lastSpokeB) { - // B - A here because the highest value - // is most recent - return lastSpokeB - lastSpokeA; - } - - var userA = a.user; - var userB = b.user; - if (userA && !userB) { - return -1; // a comes first - } - else if (!userA && userB) { - return 1; // b comes first - } - else if (!userA && !userB) { - return 0; // don't care - } - else { // both User objects exist - var lastActiveAgoA = userA.lastActiveAgo || Number.MAX_SAFE_INTEGER; - var lastActiveAgoB = userB.lastActiveAgo || Number.MAX_SAFE_INTEGER; - return lastActiveAgoA - lastActiveAgoB; - } - }).map(function(m) { +MemberEntry.fromMemberList = function(members) { + return members.map(function(m) { return new MemberEntry(m); }); } diff --git a/src/components/structures/RoomStatusBar.js b/src/components/structures/RoomStatusBar.js index 92f50dcb02..4309b1e849 100644 --- a/src/components/structures/RoomStatusBar.js +++ b/src/components/structures/RoomStatusBar.js @@ -26,9 +26,9 @@ module.exports = React.createClass({ propTypes: { // the room this statusbar is representing. room: React.PropTypes.object.isRequired, - - // a list of TabCompleteEntries.Entry objects - tabCompleteEntries: React.PropTypes.array, + + // a TabComplete object + tabComplete: React.PropTypes.object, // the number of messages which have arrived since we've been scrolled up numUnreadMessages: React.PropTypes.number, @@ -208,11 +208,11 @@ module.exports = React.createClass({ ); } - if (this.props.tabCompleteEntries) { + if (this.props.tabComplete.isTabCompleting()) { return (
- + ); - }, + }, }); diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 71edbf162d..f7f7ceb12c 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -31,10 +31,7 @@ var Modal = require("../../Modal"); var sdk = require('../../index'); var CallHandler = require('../../CallHandler'); var TabComplete = require("../../TabComplete"); -var MemberEntry = require("../../TabCompleteEntries").MemberEntry; -var CommandEntry = require("../../TabCompleteEntries").CommandEntry; var Resend = require("../../Resend"); -var SlashCommands = require("../../SlashCommands"); var dis = require("../../dispatcher"); var Tinter = require("../../Tinter"); var rate_limited_func = require('../../ratelimitedfunc'); @@ -136,12 +133,6 @@ module.exports = React.createClass({ }, componentWillMount: function() { - this.dispatcherRef = dis.register(this.onAction); - MatrixClientPeg.get().on("Room", this.onRoom); - MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); - MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); - MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); - this.tabComplete = new TabComplete({ allowLooping: false, autoEnterTabComplete: true, @@ -151,6 +142,12 @@ module.exports = React.createClass({ } }); + this.dispatcherRef = dis.register(this.onAction); + MatrixClientPeg.get().on("Room", this.onRoom); + MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); + MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); + MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); + if (this.props.roomAddress[0] == '#') { // we always look up the alias from the directory server: // we want the room that the given alias is pointing to @@ -205,8 +202,13 @@ module.exports = React.createClass({ MatrixClientPeg.get().credentials.userId, 'join' ); - // update the tab complete list now we have a room - this._updateTabCompleteList(); + this.tabComplete.loadEntries(this.state.room); + + const myUserId = MatrixClientPeg.get().credentials.userId; + const members = this.state.room.getJoinedMembers().filter(function(member) { + if (member.userId !== myUserId) return true; + }); + UserProvider.getInstance().setUserList(members); } if (!user_is_in_room && this.state.roomId) { @@ -363,7 +365,15 @@ module.exports = React.createClass({ // update ther tab complete list as it depends on who most recently spoke, // and that has probably just changed - this._updateTabCompleteList(); + if (ev.sender) { + this.tabComplete.onMemberSpoke(ev.sender); + + const myUserId = MatrixClientPeg.get().credentials.userId; + const members = this.state.room.getJoinedMembers().filter(function(member) { + if (member.userId !== myUserId) return true; + }); + UserProvider.getInstance().setUserList(members); + } }, // called when state.room is first initialised (either at initial load, @@ -441,7 +451,13 @@ module.exports = React.createClass({ } // a member state changed in this room, refresh the tab complete list - this._updateTabCompleteList(); + this.tabComplete.loadEntries(this.state.room); + + const myUserId = MatrixClientPeg.get().credentials.userId; + const members = this.state.room.getJoinedMembers().filter(function(member) { + if (member.userId !== myUserId) return true; + }); + UserProvider.getInstance().setUserList(members); // if we are now a member of the room, where we were not before, that // means we have finished joining a room we were previously peeking @@ -506,8 +522,6 @@ module.exports = React.createClass({ window.addEventListener('resize', this.onResize); this.onResize(); - this._updateTabCompleteList(); - // XXX: EVIL HACK to autofocus inviting on empty rooms. // We use the setTimeout to avoid racing with focus_composer. if (this.state.room && @@ -525,24 +539,6 @@ module.exports = React.createClass({ } }, - _updateTabCompleteList: function() { - var cli = MatrixClientPeg.get(); - - if (!this.state.room) { - return; - } - var members = this.state.room.getJoinedMembers().filter(function(member) { - if (member.userId !== cli.credentials.userId) return true; - }); - - UserProvider.getInstance().setUserList(members); - this.tabComplete.setCompletionList( - MemberEntry.fromMemberList(this.state.room, members).concat( - CommandEntry.fromCommands(SlashCommands.getCommandList()) - ) - ); - }, - componentDidUpdate: function() { if (this.refs.roomView) { var roomView = ReactDOM.findDOMNode(this.refs.roomView); @@ -1380,12 +1376,10 @@ module.exports = React.createClass({ statusBar = } else if (!this.state.searchResults) { var RoomStatusBar = sdk.getComponent('structures.RoomStatusBar'); - var tabEntries = this.tabComplete.isTabCompleting() ? - this.tabComplete.peek(6) : null; statusBar = - {this.props.entries.map(function(entry, i) { + {this.props.tabComplete.peek(6).map((entry, i) => { return (
+ className={ "mx_TabCompleteBar_item " + (entry instanceof CommandEntry ? "mx_TabCompleteBar_command" : "") } + onClick={this.props.tabComplete.onEntryClick.bind(this.props.tabComplete, entry)} > {entry.getImageJsx()} {entry.getText()} From 7d712d06a1086ed0588f04b7c6e281becf9e683d Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Jul 2016 16:14:05 +0100 Subject: [PATCH 5/8] Move code to make diff less confusing --- src/components/structures/RoomView.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index f7f7ceb12c..9c41a993e3 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -133,6 +133,12 @@ module.exports = React.createClass({ }, componentWillMount: function() { + this.dispatcherRef = dis.register(this.onAction); + MatrixClientPeg.get().on("Room", this.onRoom); + MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); + MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); + MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); + this.tabComplete = new TabComplete({ allowLooping: false, autoEnterTabComplete: true, @@ -142,12 +148,6 @@ module.exports = React.createClass({ } }); - this.dispatcherRef = dis.register(this.onAction); - MatrixClientPeg.get().on("Room", this.onRoom); - MatrixClientPeg.get().on("Room.timeline", this.onRoomTimeline); - MatrixClientPeg.get().on("Room.accountData", this.onRoomAccountData); - MatrixClientPeg.get().on("RoomState.members", this.onRoomStateMember); - if (this.props.roomAddress[0] == '#') { // we always look up the alias from the directory server: // we want the room that the given alias is pointing to From 327015ba0f271bed0dac88d5a8ff7759c4847344 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Jul 2016 17:03:53 +0100 Subject: [PATCH 6/8] Tidy up autocomplete updating ..into a function --- src/components/structures/RoomView.js | 30 +++++++++++---------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 9c41a993e3..75614471a0 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -202,13 +202,8 @@ module.exports = React.createClass({ MatrixClientPeg.get().credentials.userId, 'join' ); + this._updateAutoComplete(); this.tabComplete.loadEntries(this.state.room); - - const myUserId = MatrixClientPeg.get().credentials.userId; - const members = this.state.room.getJoinedMembers().filter(function(member) { - if (member.userId !== myUserId) return true; - }); - UserProvider.getInstance().setUserList(members); } if (!user_is_in_room && this.state.roomId) { @@ -367,12 +362,8 @@ module.exports = React.createClass({ // and that has probably just changed if (ev.sender) { this.tabComplete.onMemberSpoke(ev.sender); - - const myUserId = MatrixClientPeg.get().credentials.userId; - const members = this.state.room.getJoinedMembers().filter(function(member) { - if (member.userId !== myUserId) return true; - }); - UserProvider.getInstance().setUserList(members); + // nb. we don't need to update the new autocomplete here since + // its results are currently ordered purely by search score. } }, @@ -452,12 +443,7 @@ module.exports = React.createClass({ // a member state changed in this room, refresh the tab complete list this.tabComplete.loadEntries(this.state.room); - - const myUserId = MatrixClientPeg.get().credentials.userId; - const members = this.state.room.getJoinedMembers().filter(function(member) { - if (member.userId !== myUserId) return true; - }); - UserProvider.getInstance().setUserList(members); + this._updateAutoComplete(); // if we are now a member of the room, where we were not before, that // means we have finished joining a room we were previously peeking @@ -1263,6 +1249,14 @@ module.exports = React.createClass({ } }, + _updateAutoComplete: function() { + const myUserId = MatrixClientPeg.get().credentials.userId; + const members = this.state.room.getJoinedMembers().filter(function(member) { + if (member.userId !== myUserId) return true; + }); + UserProvider.getInstance().setUserList(members); + }, + render: function() { var RoomHeader = sdk.getComponent('rooms.RoomHeader'); var MessageComposer = sdk.getComponent('rooms.MessageComposer'); From ccf8e269cde30f331804e66d51f8c1daf127b068 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Jul 2016 17:15:51 +0100 Subject: [PATCH 7/8] Comments & required props --- src/TabComplete.js | 5 +++++ src/components/structures/RoomStatusBar.js | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/TabComplete.js b/src/TabComplete.js index 5b7be7c286..0ec0b77802 100644 --- a/src/TabComplete.js +++ b/src/TabComplete.js @@ -48,7 +48,12 @@ class TabComplete { this.isFirstWord = false; // true if you tab-complete on the first word this.enterTabCompleteTimerId = null; this.inPassiveMode = false; + + // Map tracking ordering of the room members. + // userId: integer, highest comes first. this.memberTabOrder = {}; + + // monotonically increasing counter used for tracking ordering of members this.memberOrderSeq = 0; } diff --git a/src/components/structures/RoomStatusBar.js b/src/components/structures/RoomStatusBar.js index 4309b1e849..9a0d3dbbdd 100644 --- a/src/components/structures/RoomStatusBar.js +++ b/src/components/structures/RoomStatusBar.js @@ -28,7 +28,7 @@ module.exports = React.createClass({ room: React.PropTypes.object.isRequired, // a TabComplete object - tabComplete: React.PropTypes.object, + tabComplete: React.PropTypes.object.isRequired, // the number of messages which have arrived since we've been scrolled up numUnreadMessages: React.PropTypes.number, From 5c566cae5c01578aa0c164a52903368c7c4460b6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Jul 2016 18:10:56 +0100 Subject: [PATCH 8/8] typo --- src/components/structures/RoomView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 75614471a0..abcccc48b8 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -358,7 +358,7 @@ module.exports = React.createClass({ } } - // update ther tab complete list as it depends on who most recently spoke, + // update the tab complete list as it depends on who most recently spoke, // and that has probably just changed if (ev.sender) { this.tabComplete.onMemberSpoke(ev.sender);