mirrors/element-web
1
0
Fork 0
mirror of https://github.com/element-hq/element-web synced 2024-11-24 10:15:43 +03:00
Code Issues Projects Releases Packages Wiki Activity

Flatten and simplify the memberlist sorting algorithm

Browse source 
The previous algorithm had a bug where it was getting stuck on the power level comparison, leaving the memberlist incorrectly ordered. The new algorithm uses less branching to try and walk through the different cases instead. Additionally, the steps used to determine the order have changed slightly to better represent an active member list.

This commit also includes changes to try and re-sort the member list more often during presence changes. Events are not always emitted, however. This may be a js-sdk bug but appears to happen prior to these changes as well.

Fixes https://github.com/vector-im/riot-web/issues/6953
This commit is contained in:
Travis Ralston 2018-12-20 14:56:18 -07:00
parent ad47144355
commit 63a7b86eac
1 changed files with 55 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

94
src/components/views/rooms/MemberList.js
View file

@ -68,7 +68,9 @@ module.exports = React.createClass({
// We listen for changes to the lastPresenceTs which is essentially // We listen for changes to the lastPresenceTs which is essentially
// listening for all presence events (we display most of not all of // listening for all presence events (we display most of not all of
// the information contained in presence events). // the information contained in presence events).
cli.on("User.lastPresenceTs", this.onUserLastPresenceTs); cli.on("User.lastPresenceTs", this.onUserPresenceChange);
cli.on("User.presence", this.onUserPresenceChange);
cli.on("User.currentlyActive", this.onUserPresenceChange);
// cli.on("Room.timeline", this.onRoomTimeline); // cli.on("Room.timeline", this.onRoomTimeline);
}, },
@ -81,7 +83,9 @@ module.exports = React.createClass({
cli.removeListener("Room.myMembership", this.onMyMembership); cli.removeListener("Room.myMembership", this.onMyMembership);
cli.removeListener("RoomState.events", this.onRoomStateEvent); cli.removeListener("RoomState.events", this.onRoomStateEvent);
cli.removeListener("Room", this.onRoom); cli.removeListener("Room", this.onRoom);
cli.removeListener("User.lastPresenceTs", this.onUserLastPresenceTs); cli.removeListener("User.lastPresenceTs", this.onUserPresenceChange);
cli.removeListener("User.presence", this.onUserPresenceChange);
cli.removeListener("User.currentlyActive", this.onUserPresenceChange);
} }
// cancel any pending calls to the rate_limited_funcs // cancel any pending calls to the rate_limited_funcs
@ -132,12 +136,12 @@ module.exports = React.createClass({
}; };
}, },
onUserLastPresenceTs(event, user) { onUserPresenceChange(event, user) {
// Attach a SINGLE listener for global presence changes then locate the // Attach a SINGLE listener for global presence changes then locate the
// member tile and re-render it. This is more efficient than every tile // member tile and re-render it. This is more efficient than every tile
// evar attaching their own listener. // ever attaching their own listener.
// console.log("explicit presence from " + user.userId);
const tile = this.refs[user.userId]; const tile = this.refs[user.userId];
console.log(`Got presence update for ${user.userId}. hasTile=${!!tile}`);
if (tile) { if (tile) {
this._updateList(); // reorder the membership list this._updateList(); // reorder the membership list
} }
@ -267,7 +271,8 @@ module.exports = React.createClass({
if (!member) { if (!member) {
return "(null)"; return "(null)";
} else { } else {
return "(" + member.name + ", " + member.powerLevel + ", " + member.user.lastActiveAgo + ", " + member.user.currentlyActive + ")"; const u = member.user;
return "(" + member.name + ", " + member.powerLevel + ", " + (u ? u.lastActiveAgo : "<null>") + ", " + (u ? u.getLastActiveTs() : "<null>") + ", " + (u ? u.currentlyActive : "<null>") + ")";
} }
}, },
@ -275,48 +280,59 @@ module.exports = React.createClass({
// returns 0 if a and b are equivalent in ordering // returns 0 if a and b are equivalent in ordering
// returns positive if a comes after b. // returns positive if a comes after b.
memberSort: function(memberA, memberB) { memberSort: function(memberA, memberB) {
// order by last active, with "active now" first. // order by presence, with "active now" first.
// ...and then by power // ...and then by power level
// ...and then alphabetically. // ...and then by last active
// We could tiebreak instead by "last recently spoken in this room" if we wanted to. // ...and then alphabetically.
// We could tiebreak instead by "last recently spoken in this room" if we wanted to.
const userA = memberA.user; console.log(`Comparing userA=${this.memberString(memberA)} userB=${this.memberString(memberB)}`);
const userB = memberB.user;
// if (!userA || !userB) { const userA = memberA.user;
// console.log("comparing " + memberA.name + " user=" + memberA.user + " with " + memberB.name + " user=" + memberB.user); const userB = memberB.user;
// }
if (!userA && !userB) return 0; if (!userA) console.log("!! MISSING USER FOR A-SIDE: " + memberA.name + " !!");
if (userA && !userB) return -1; if (!userB) console.log("!! MISSING USER FOR B-SIDE: " + memberB.name + " !!");
if (!userA && userB) return 1;
// console.log("comparing " + this.memberString(memberA) + " and " + this.memberString(memberB)); if (!userA && !userB) return 0;
if (userA && !userB) return -1;
if (!userA && userB) return 1;
if ((userA.currentlyActive && userB.currentlyActive) || !this._showPresence) { // First by presence
// console.log(memberA.name + " and " + memberB.name + " are both active"); if (this._showPresence) {
if (memberA.powerLevel === memberB.powerLevel) { const convertPresence = (p) => p === 'unavailable' ? 'online' : p;
// console.log(memberA + " and " + memberB + " have same power level"); const presenceIndex = p => {
if (memberA.name && memberB.name) { const order = ['active', 'online', 'offline'];
// console.log("comparing names: " + memberA.name + " and " + memberB.name); const idx = order.indexOf(convertPresence(p));
const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name; return idx === -1 ? order.length : idx; // unknown states at the end
const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name; };
return nameA.localeCompare(nameB);
} else { const idxA = presenceIndex(userA.currentlyActive ? 'active' : userA.presence);
return 0; const idxB = presenceIndex(userB.currentlyActive ? 'active' : userB.presence);
} console.log(`userA_presenceGroup=${idxA} userB_presenceGroup=${idxB}`);
} else { if (idxA !== idxB) {
// console.log("comparing power: " + memberA.powerLevel + " and " + memberB.powerLevel); console.log("Comparing on presence group - returning");
return memberB.powerLevel - memberA.powerLevel; return idxA - idxB;
}
} }
}
if (userA.currentlyActive && !userB.currentlyActive) return -1; // Second by power level
if (!userA.currentlyActive && userB.currentlyActive) return 1; if (memberA.powerLevel !== memberB.powerLevel) {
console.log("Comparing on power level - returning");
return memberB.powerLevel - memberA.powerLevel;
}
// For now, let's just order things by timestamp. It's really annoying // Third by last active
// that a user disappears from sight just because they temporarily go offline if (this._showPresence && userA.getLastActiveTs() !== userB.getLastActiveTs) {
console.log("Comparing on last active timestamp - returning");
return userB.getLastActiveTs() - userA.getLastActiveTs(); return userB.getLastActiveTs() - userA.getLastActiveTs();
}
// Fourth by name (alphabetical)
const nameA = memberA.name[0] === '@' ? memberA.name.substr(1) : memberA.name;
const nameB = memberB.name[0] === '@' ? memberB.name.substr(1) : memberB.name;
console.log(`Comparing userA_name=${nameA} against userB_name=${nameB} - returning`);
return nameA.localeCompare(nameB);
}, },
onSearchQueryChanged: function(ev) { onSearchQueryChanged: function(ev) {