Peeking unknown rooms #3

ashfame · 2023-02-09T12:49:38Z

PR meant for review (not for merge)

psrpinto

This is looking great, looking forward to having this feature in hydrogen! Well done!

I noticed that when viewing large rooms there is a long delay between the UI first rendering and the timeline appearing, so I think we would probably need to show a loading state before the timeline actually renders:

Screen.Recording.2023-02-10.at.15.37.18.mov

src/domain/session/SessionViewModel.js

src/platform/web/ui/session/room/timeline/BaseMessageView.js

src/platform/web/ui/session/SessionView.js

src/platform/web/ui/session/room/PeekableRoomView.js

src/domain/session/room/UnknownRoomViewModel.js

src/platform/web/ui/session/room/PeekableRoomView.js

ashfame · 2023-02-11T14:17:01Z

@psrpinto Thanks for the great suggestions! <3

Could you explain the issue in your screencast? matrix.org doesn't allow previews on their rooms if you are not in the room and hence peeking can't be functional for those rooms. I believe they must have done it to prevent DOS attacks or to improve scaling. How were you able to even trigger the timeline to load for you?

bwindels · 2023-02-13T10:28:26Z

src/domain/session/room/UnknownRoomViewModel.js

+        }
+        try {
+            this._room = await this._session.loadWorldReadableRoom(this.roomIdOrAlias);
+            const timeline = await this._room.openTimeline();


Interesting ... how does this work if you don't already have a fragment of the timeline stored? If I understand things correctly, at this point, the room exists solely in-memory and nothing related has been written to storage, right? So what token do you end up calling /messages with? I might very well be missing something :)

Actually loadWorldReadableRoom() above it does all of that - creates the Room object instance, fetch events from messages endpoint & load it all. You will see that here - https://github.com/Automattic/hydrogen-web/pull/3/files#diff-25d3af8c52953975b54837c62ef3195e1054638339aa9b1a9c1d6bc267cdadc8R962

psrpinto

Could you explain the issue in your screencast?
How were you able to even trigger the timeline to load for you?

In the screen recording I'm using #matrix:matrix.org as an example (!OGEhHVWSdvArJzumhm:matrix.org). I'm not a member of that room but I think I was a member and then left, that's why the last message on the timeline is @psrpinto:matrix.org left the room.

So that might explain why the timeline renders for me, which would mean that we would have to take that scenario in account.

But what should we do? Should we:

Render the UnknownRoomView and show the latest messages
Don't render the UnknownRoomView and let hydrogen take care of it. Hydrogen already shows the timeline if you left the room, I believe.

At the moment we're in the middle of 1. and 2., where we show the UnknownRoomView with not the most recent messages.

matrix.org doesn't allow previews on their rooms if you are not in the room and hence peeking can't be functional for those rooms.

By "doesn't allow" you mean that you can't call the /messages endpoint for the room if you're not a member?

src/matrix/Session.js

src/platform/web/ui/session/room/PeekableRoomView.js

psrpinto · 2023-02-13T14:52:35Z

Hydrogen already shows the timeline if you left the room, I believe.

Actually, on master. if I navigate to !OGEhHVWSdvArJzumhm%3Amatrix.org (#matrix:matrix.org) it shows "You are currently not in !OGEhHVWSdvArJzumhm:matrix.org. Want to join it?", it doesn't render the timeline.

Am I going crazy? I thought I'd seen hydrogen rendering the timeline for rooms you left, with history up to the point you left, and a message in the bottom saying "You left the room".

ashfame · 2023-02-13T15:04:41Z

So that might explain why the timeline renders for me, which would mean that we would have to take that scenario in account.

That would be the timeline you saw for an archive room. Unless we choose to forget the room, UnknownRoomView doesn't come into the picture and by then peeking functionality handles it.

By "doesn't allow" you mean that you can't call the /messages endpoint for the room if you're not a member?

Correct! If you try to peek into matrix hq room, you will see console errors stating previews are disabled.

I tried peeking into !OGEhHVWSdvArJzumhm%3Amatrix.org and it was 403: User @p1:synapse.dev:8008 not in room !OGEhHVWSdvArJzumhm:matrix.org, and room previews are disabled which is shown in console as well.

I think you got confused by the archive room timeline view after leaving the room.

ashfame · 2023-02-14T17:02:28Z

@psrpinto and I discussed this more and it turns out this happens when you try to peek into a room that you earlier were a member in but not anymore. Weirdly enough matrix.org has room previews disabled if you are trying to peek from another homeserver but if you are logged in with an account on matrix.org, it lets you preview but with this edge case of /messages endpoint acting up. I will continue to look into this.

Also while trying to figure this out, I was happy to learn that back scrolling is functional as is :)

…e by fetching messages and storing in the indexed db tables

… in a message

…iew only

…ligned with Matrix spec

Co-authored-by: Paulo Pinto <[email protected]>

…i HTTP request

…nd loading things async and updating view as we do This shows a spinner on UnknownRoomView stating "checking preview capability" and goes away when done and if room is world_readable, then renders the timeline with the last 100 messages

ashfame · 2023-02-16T15:13:33Z

Created PR on upstream - element-hq#1037 so closing this PR

ashfame requested a review from psrpinto February 9, 2023 12:49

ashfame self-assigned this Feb 9, 2023

ashfame mentioned this pull request Feb 9, 2023

Add support for room peeking Automattic/chatrix#141

Closed

4 tasks

psrpinto reviewed Feb 10, 2023

View reviewed changes

ashfame force-pushed the peeking_unknown_rooms branch from 0762625 to 647d8a4 Compare February 11, 2023 12:02

bwindels reviewed Feb 13, 2023

View reviewed changes

psrpinto reviewed Feb 13, 2023

View reviewed changes

src/matrix/Session.js Outdated Show resolved Hide resolved

src/platform/web/ui/session/room/PeekableRoomView.js Outdated Show resolved Hide resolved

ashfame force-pushed the peeking_unknown_rooms branch from 932b34d to 7596057 Compare February 15, 2023 14:41

ashfame added 6 commits February 15, 2023 18:44

define PeekableRoomView and make UnknownRoomViewModel handle it

d468abd

setup timelineViewModel on unknownRoomViewModel to render the timelin…

8b56a0d

…e by fetching messages and storing in the indexed db tables

move room alias resolution to canPeekInRoom() instead

9fa2f71

use css to hide the right panel triggered when clicked on user avatar…

c76e45d

… in a message

remove PeekableRoomView and bring the code for it within UnknownRoomV…

5bc5ca0

…iew only

change code to refer peekable room as WorldReadable room to be more a…

4439fb3

…ligned with Matrix spec

ashfame force-pushed the peeking_unknown_rooms branch 5 times, most recently from 579a1c5 to 098067f Compare February 16, 2023 15:05

ashfame and others added 3 commits February 16, 2023 19:10

use string's startsWith() instead

220fe38

Co-authored-by: Paulo Pinto <[email protected]>

ensure a boolean is returned despite of failure/error in HomeserverAp…

2fb9cb6

…i HTTP request

ashfame force-pushed the peeking_unknown_rooms branch from 098067f to 654cdf0 Compare February 16, 2023 15:10

ashfame closed this Feb 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Peeking unknown rooms #3

Peeking unknown rooms #3

ashfame commented Feb 9, 2023

psrpinto left a comment

ashfame commented Feb 11, 2023

bwindels Feb 13, 2023 •

edited

Loading

ashfame Feb 13, 2023

psrpinto left a comment

psrpinto commented Feb 13, 2023

ashfame commented Feb 13, 2023

ashfame commented Feb 14, 2023

ashfame commented Feb 16, 2023

Peeking unknown rooms #3

Peeking unknown rooms #3

Conversation

ashfame commented Feb 9, 2023

psrpinto left a comment

Choose a reason for hiding this comment

ashfame commented Feb 11, 2023

bwindels Feb 13, 2023 • edited Loading

Choose a reason for hiding this comment

ashfame Feb 13, 2023

Choose a reason for hiding this comment

psrpinto left a comment

Choose a reason for hiding this comment

psrpinto commented Feb 13, 2023

ashfame commented Feb 13, 2023

ashfame commented Feb 14, 2023

ashfame commented Feb 16, 2023

bwindels Feb 13, 2023 •

edited

Loading