Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that there's a good UX when you launch the app having meanwhile been kicked from rooms #308

Closed
matrixbot opened this issue Nov 2, 2015 · 18 comments
Assignees

Comments

@matrixbot
Copy link

Created by @ matthew:matrix.org.

@ara4n
Copy link
Member

ara4n commented Nov 15, 2015

This is critical as it's been used as an excuse in the past for not kicking idle users out of HQ and other super-busy rooms.

@ara4n ara4n modified the milestone: Ragnarok Nov 29, 2015
@kegsay kegsay self-assigned this Dec 7, 2015
@kegsay
Copy link
Contributor

kegsay commented Dec 7, 2015

So the UX currently is that the room does not show up in Vector. Doesn't appear anywhere on the room list. We presumably want this to appear in the room list so we can say "yo you've been kicked from this room.". My questions are:

  • Which section should this appear (presumably not archived but then what is the criteria to determine archived vs recent? Inspect the m.room.member event f.e room you're left in and if it's a kick then put it back in recents?)
  • Presumably you can "forget" the room like you can with any archived room?
  • Any other UX requirements?

Assigning back for feedback.

@kegsay kegsay assigned ara4n and unassigned kegsay Dec 7, 2015
@ara4n
Copy link
Member

ara4n commented Dec 11, 2015

The UX should be that we show it wherever it was before in the roomlist, but with a "you've been kicked" in the plinth at the top of the page that we also use for accepting/rejecting invites (or when peeking in on a public room). The room would otherwise behave as if they were trying to join it for the first time (or as if they were looking at an archived room; the behaviour is the same) - letting them peek at the current activity if it's public; giving them the option to join it (in the plinth at the top). I guess we should also have an option to archive it as a special case for this situation.

@ara4n ara4n assigned kegsay and unassigned ara4n Dec 11, 2015
@ara4n ara4n modified the milestones: Ragnarok, v0 Dec 13, 2015
@kegsay
Copy link
Contributor

kegsay commented Dec 14, 2015

This is blocked on providing archived rooms support in Vector. We need this in order for the room you've been kicked from to be sent to the client.

@kegsay
Copy link
Contributor

kegsay commented Dec 18, 2015

We now have an archived room implementation that is reasonably performant but it doesn't help for this use case. We lazily get the archived room list, so we don't know if there are rooms you've been kicked from that are present in the archived room list.

At this point, adding on some additional server-side keys would be most useful in order to do this. The easiest way would be to treat "rooms you've been kicked or banned from" differently to "rooms you've actually consciously left" and then filter them differently in v2 /sync. The JS SDK would say "give me kicked/banned rooms but not ones I've actually left, I'll do that separately thanks".

  • Simple and should work without any invasive client changes beyond tweaking filter keys.
  • Will make room data for banned/kicked rooms come down that first initial sync, which is probably what we want, you could always filter it out.

In the long run, we should have a sanity check over the idea of splatting data from left/kicked/banned rooms in via /sync, it feels very wrong semantically to do this because there are no incremental deltas which is the whole point to the /sync protocol. Rooms you're no longer a part of are just an unchanging blob of JSON, do we really want that blob to come down every time there is an initial /sync? In this case, yes we do. In other cases, no we don't, hence the quick fix proposal to distinguish between the two.

@ara4n
Copy link
Member

ara4n commented Dec 18, 2015

okay. let's add something special to /sync to distinguish left from kicked/banned rooms so we can fix this properly. assigning to @NegativeMjark for an experimental fix, which may or may not make it into the actual matrix spec.

@ara4n ara4n assigned NegativeMjark and unassigned kegsay Dec 18, 2015
@ara4n
Copy link
Member

ara4n commented Jan 11, 2016

reassigning to @erikjohnston in his current synapse wrangler role

@ara4n ara4n assigned erikjohnston and unassigned NegativeMjark Jan 11, 2016
@ara4n ara4n added P2 and removed P1 labels Jan 13, 2016
@erikjohnston
Copy link
Member

matrix-org/synapse#625

@erikjohnston
Copy link
Member

Done.

@ara4n
Copy link
Member

ara4n commented Mar 4, 2016

Looks like this will need a tweak to RoomList.getRoomLists() to ensure that rooms that you've been kicked from are shown in the Recents sublist rather than Historical. Bans are already handled correctly.

For the best UX we'll also need to implement #375 so that users have a way to move kick/banned rooms out of their non-archived sublists.

@ara4n ara4n removed the X-Blocked label Mar 4, 2016
ara4n added a commit to matrix-org/matrix-react-sdk that referenced this issue Mar 18, 2016
@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

I landed this on matrix-org/matrix-react-sdk@bfbb7a6 but it doesn't work, as synapse on matrix.org at least only passes me the kicked rooms when I expand the historical section - not on every /sync :(

@erikjohnston are you sure your fix to synapse works?

@ara4n ara4n assigned erikjohnston and unassigned ara4n Mar 18, 2016
@erikjohnston
Copy link
Member

Hmm, do you have an example of this? The synapse code looks like it should do what I expect, so might be useful to have something concrete to chase.

@erikjohnston
Copy link
Member

To clarify, is this for initial sync or incremental sync?

@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

this is on initial sync. however, i think vdh has broken vector/develop in other ways which are confusing this...

@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

so hold off for now; sorry for red herring :/

@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

(improved fix at matrix-org/matrix-react-sdk@0250192 ftr)

@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

yeah, it's vdh's fault - sorry :(

ara4n added a commit to matrix-org/matrix-react-sdk that referenced this issue Mar 18, 2016
@ara4n
Copy link
Member

ara4n commented Mar 18, 2016

both vector & synapse seem happy modulo current develop breakage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants