Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Use correct 1-1 room avatar after users leave #593

Merged
merged 5 commits into from
Apr 11, 2018

Conversation

lukebarnard1
Copy link
Contributor

The correct 1-1 avatar is used with rooms in which there are only two users with either "join" or "invite" as their membership (importantly, not "leave" or otherwise).

(This is important when a user moves accounts and re-joins previously left 1-1 chats)

The correct 1-1 avatar is used with rooms in which there are only two users with either "join" or "invite" as their membership (importantly, not "leave" or otherwise).

(This is important when a user moves accounts and re-joins previously left 1-1 chats)
@ara4n ara4n self-assigned this Dec 19, 2016
@ara4n
Copy link
Member

ara4n commented Dec 19, 2016

will this not have the side-effect that if i'm in a 1:1 with someone and they leave, the icon will change to just be me, rather than showing them? This will be quite unintuitive, given the history is with them...

@ara4n ara4n assigned lukebarnard1 and unassigned ara4n and lukebarnard1 Dec 19, 2016
@ara4n
Copy link
Member

ara4n commented Dec 22, 2016

lgtm, assuming this matches the logic for calculating the name. (it feels wrong that these are maintained entirely separately........)

@lukebarnard1
Copy link
Contributor Author

lukebarnard1 commented Dec 22, 2016

It would be nice to push this sort of stuff back up to the js-sdk somehow.

I'm now thinking that leftUserIds should probably only contain users that actually left... If there's one person in the room and one person got kicked, they should probably not be the room avatar?... The spec says (and this makes a lot of sense) that you should only " consider m.room.member events for users other than the logged-in user, with membership: join or membership: invite. i.e. other members should be totally ignored.

TL;DR this PR currently ignores the room name spec. If we want to consider users that have left, we need to update the spec really.

@lukebarnard1 lukebarnard1 removed their assignment Mar 1, 2017
@lukebarnard1
Copy link
Contributor Author

Closing as this is an obscure edge-case of swapping accounts

@lukebarnard1 lukebarnard1 reopened this Feb 6, 2018
@lukebarnard1
Copy link
Contributor Author

FTR @dbkr looked this over and gave the go-ahead.

@lukebarnard1
Copy link
Contributor Author

I'm going to merge this, knowingly excluding it from Riot 14 so that we can necessary QA on it.

@lukebarnard1 lukebarnard1 merged commit d877384 into develop Apr 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants