fix membership list ordering when presence is disabled. #1924
Conversation
ara4n
changed the title
This isn't really pertinent to this particular PR/issue. |
| @@ -270,7 +270,7 @@ module.exports = React.createClass({ | |||
|
|
|||
| // console.log("comparing " + this.memberString(memberA) + " and " + this.memberString(memberB)); | |||
|
|
|||
| if (userA.currentlyActive && userB.currentlyActive) { | |||
| if ((userA.currentlyActive && userB.currentlyActive) || !this._showPresence) { | |||
There was a problem hiding this comment.
// order by last active, with "active now" first.
// ...and then by power
// ...and then alphabetically.
Why don't we just order by power first instead of special casing?
There was a problem hiding this comment.
I think because the desired behaviour is that online users should still come first if presence is enabled.
There was a problem hiding this comment.
The intended behaviour of the list is to order first by online state, then by power (so offline mods don't clutter up the top of the list). Changing that UX is an entirely separate issue and irrelevant to this PR. This PR is just trying to fix the regression to return to the intended behaviour.
The regression here was that we were ordering by invisible(!) and invalid(!) online state if presence is disabled, which results in an incorrect ordering as per element-hq/element-web#6643, hence this fix.
ara4n
changed the title
|
@lukebarnard1 as a team we are failing to prioritise fixing regressions, hence me highlighting this one with @lampholder so he and I can discuss it and use it as a microcosm of the problem, even if that meta-discussion is technically off-topic for this specific PR. I would hope it is clear that the bigger goal of improving the overall quality of our work takes priority over pedantry over pertinence of GH comments. |
To me it was just noisy. I don't see an advantage of mentioning it here. I absolutely agree that we should discuss the overall quality of our work but it doesn't belong on a random PR. |
Fixes element-hq/element-web#6643.
@lampholder we really need to find a way for P1 regressions to not hang around open for 23 days (for a regression introduced 66 days ago), especially when it's a oneliner to fix...