Multiverse rework

[poljar]

This has grown a bit too much, I probably should have continued to push smaller refactor PRs out but I wasn't really sure where this will go until the end.

So here we are with a huge PR, if we really want to review this a bit more carefully I can split it out.

I probably should also open more PRs after this to document things a bit better.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 42 lines in your changes missing coverage. Please review.

Project coverage is 85.73%. Comparing base (e8fcdf4) to head (aa5a051).
Report is 140 commits behind head on main.

Files with missing lines	Patch %	Lines
...bs/multiverse/src/widgets/room_view/details/mod.rs	0.00%	27 Missing ⚠️
labs/multiverse/src/widgets/room_view/input.rs	0.00%	9 Missing ⚠️
labs/multiverse/src/widgets/mod.rs	0.00%	2 Missing ⚠️
...multiverse/src/widgets/room_view/details/events.rs	0.00%	1 Missing ⚠️
...erse/src/widgets/room_view/details/linked_chunk.rs	0.00%	1 Missing ⚠️
...rse/src/widgets/room_view/details/read_receipts.rs	0.00%	1 Missing ⚠️
labs/multiverse/src/widgets/room_view/timeline.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4861      +/-   ##
==========================================
- Coverage   86.49%   85.73%   -0.77%     
==========================================
  Files         300      306       +6     
  Lines       34785    35113     +328     
==========================================
+ Hits        30087    30103      +16     
- Misses       4698     5010     +312     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bnjbvr]

Thanks for the changes. I've mostly tested it to make sure I could find the features I implemented in the past, and that it wasn't too painful to use. A few workflows are now much more complicated, so requesting changes for those:

• I loved the ability to be able to back-paginate from the linked chunk views, since it directly showed me how the state of the event chunk would change in this case. In my testing of this PR I've noticed it changed and it's not possible to do so anymore. Could we reinstate that, please?
• I'd like direct shortcut access to each view pwease; otherwise, playing with the linked chunk (especially because of the above) requires: (1) going to the room view to do A Thing, (2) get to the detail view, (3) hit the arrows/tab a few times to get to the linked chunk view.
• Thanks for having the details view take the full screen space, btw. At this point I'm not sure it's worth distinguishing the rendered timeline view from the details subviews; what do you think of having the rendered timeline view part of the tab list, so we don't have to hit F8 to first select a room's detailed view? (that is, the tab would always be displayed, with 4 entries, the first one being the rendered timeline view)

I am not a big fan of prioriziting inputting any message in the room's view, over using plain shortcuts without Ctrl / function keys. The goal of multiverse is not to be a matrix CLI client, only a debugging tool; as such, having a field for entering messages is of little value to me. The only thing that matters is to be able to send a prerecorded message, to make sure the send queue worked, and that it would behave nicely when the send queue was disabled / reenabled. I can live with it, though.

Bonus points if you can support navigating a timeline's messages, and maybe use that to allow liking a specific message (instead of the last one, by default) — really this is going to be super helpful for opening threads, in the future. But I can look into it later, of course :)

Nice work overall. The code seems nicely organized, in such a way that I could find the place I'd need to touch if I wanted to add a new detailed view 👍

[poljar]

Thanks for the changes. I've mostly tested it to make sure I could find the features I implemented in the past, and that it wasn't too painful to use. A few workflows are now much more complicated, so requesting changes for those:

I loved the ability to be able to back-paginate from the linked chunk views, since it directly showed me how the state of the event chunk would change in this case. In my testing of this PR I've noticed it changed and it's not possible to do so anymore. Could we reinstate that, please?

Added back in 6d01d0b.

I'd like direct shortcut access to each view pwease; otherwise, playing with the linked chunk (especially because of the above) requires: (1) going to the room view to do A Thing, (2) get to the detail view, (3) hit the arrows/tab a few times to get to the linked chunk view.

Any idea which shortcuts you'd like to use?

Thanks for having the details view take the full screen space, btw. At this point I'm not sure it's worth distinguishing the rendered timeline view from the details subviews; what do you think of having the rendered timeline view part of the tab list, so we don't have to hit F8 to first select a room's detailed view? (that is, the tab would always be displayed, with 4 entries, the first one being the rendered timeline view)

That would then require global shortcuts for switching between the tabs (tab/shift-tab, left/right), additionally I don't imagine that those views will be particularly interesting to the crypto team.

I am not a big fan of prioriziting inputting any message in the room's view, over using plain shortcuts without Ctrl / function keys. The goal of multiverse is not to be a matrix CLI client, only a debugging tool; as such, having a field for entering messages is of little value to me.

I understand that, but the goal of this PR is to make multiverse to other teams and for other purposes usable. More specifically, the crypto team would like to use multiverse for prototyping purposes:

• multiverse: support verifying the multiverse client device #4508
• multiverse: support sending room invites to other users #4509

As such, it's necessary for multiverse to become more of a Matrix client. Furthermore, the input line isn't necessarily only useful to send messages, implementing simple commands like /invite MXID or /discard-room-key require the existence of an input line.

Bonus points if you can support navigating a timeline's messages, and maybe use that to allow liking a specific message (instead of the last one, by default) — really this is going to be super helpful for opening threads, in the future. But I can look into it later, of course :)

@Hywan has some further improvements to the timeline rendering which would be nice to integrate as well, selecting each message individually isn't part of that but I think having the ability to scroll is somewhat a prerequisite for this. So perhaps we'll get to that as well.

[bnjbvr]

Thanks for the rework of the rework! I agree with all your points, some answers below.

Any idea which shortcuts you'd like to use?

I can add them in a followup, if I'd like to; cheers though for adding the back-pagination to the linked-chunk view, that was one of the two blockers for me.

Thanks for having the details view take the full screen space, btw. At this point I'm not sure it's worth distinguishing the rendered timeline view from the details subviews; what do you think of having the rendered timeline view part of the tab list, so we don't have to hit F8 to first select a room's detailed view? (that is, the tab would always be displayed, with 4 entries, the first one being the rendered timeline view)

That would then require global shortcuts for switching between the tabs (tab/shift-tab, left/right), additionally I don't imagine that those views will be particularly interesting to the crypto team.

Those views not being particularly interesting to the crypto team doesn't sound like a compelling argument to me; I'm particularly interested in all of those views, and would really really like to keep direct access to those (I'm actually mostly uninterested in the timeline rendering, most of the time). It also sounds like a simplification in that we'd have a single screen for views, so intuitively I would imagine this is a win overall, in terms of code size, future maintenance, etc. Can we have that please? 🥺

[bnjbvr]

Explicitly requesting changes, because of the last point about getting direct access to the detailed views. Until this is addressed, I strongly object to merging this.

[poljar]

Thanks for the rework of the rework! I agree with all your points, some answers below.

Any idea which shortcuts you'd like to use?

I can add them in a followup, if I'd like to; cheers though for adding the back-pagination to the linked-chunk view, that was one of the two blockers for me.

I'm confused. This what would give you direct access to the individual details tabs, you are blocking the merger of this PR on this yet you don't want me to add any shortcuts.

Thanks for having the details view take the full screen space, btw. At this point I'm not sure it's worth distinguishing the rendered timeline view from the details subviews; what do you think of having the rendered timeline view part of the tab list, so we don't have to hit F8 to first select a room's detailed view? (that is, the tab would always be displayed, with 4 entries, the first one being the rendered timeline view)

We discussed this in the Matrix room, but let me reiterate the the reasons why I think that this is would be suboptimal:

1. It doesn't give you the direct access you are asking for, i.e. you need to hit tab twice to get to the third tab containing the one of the details views.
2. Takes away visual space which could be used for other things, i.e. we could have those tabs always visible but they would be for the threads.
3. Takes away the tab keys in the global shortcuts space.

Point 3 is probably the most important here, as the tab key could be used for plenty of other things:

1. Tab completion for commands on the input line
2. Navigating individual items of the timeline view
3. Navigating reactions on a selected timeline item

To name a few.

Those views not being particularly interesting to the crypto team doesn't sound like a compelling argument to me; I'm particularly interested in all of those views, and would really really like to keep direct access to those (I'm actually mostly uninterested in the timeline rendering, most of the time).

Ok, but nobody wants to take away your direct access. Previously you had direct access to this using letters, i.e. e for the events view. This isn't possible anymore since we added an input line so we have to find some alternative. My proposal was a modifier key in combination to a number, here it is using ALT: 8589a6f.

It also sounds like a simplification in that we'd have a single screen for views, so intuitively I would imagine this is a win overall, in terms of code size, future maintenance, etc. Can we have that please? 🥺

I wouldn't really think that this is a simplification, it's certainly more work to now move the input line and timeline to the RoomDetails struct. Furthermore future expansion of multiverse to show thread or a member list would always have to consider that these details are part of the central and main view of the application.

Now they are compartmentalized, the shortcuts as well as the rendering of the details, and out of the way for further expansion.

[bnjbvr]

I'm confused. This what would give you direct access to the individual details tabs, you are blocking the merger of this PR on this yet you don't want me to add any shortcuts.

We had a chat about this in an internal room in the 24 minutes between my first comment and the second review comment, and that discussion brought up some extra context and requests, so I'll ignore this part, assuming you've read the contents of our discussion (but it's possible you have missed the last two messages, as you didn't answer them).

Point 3 is probably the most important here, as the tab key could be used for plenty of other things:

This is a strawman argument. You are preventing changes that would be useful right now for someone who cares about them, in favor of future potential changes for requirements that are ill-defined and unclear at this point. You're also talking about using the same keybinding for actions where the context matters (am I in the timeline view list? am I in a reactions view list?), while all actions would take place in the timeline view, making it impossible to distinguish context. But again, this is making up future use cases that we are not sure we need, or even want right now.

Why to reserve this keybinding for future use cases where literally any other keybindings would also work? In other words, if I borrow your argument to apply it the other way around: maybe we could use the F1 key later to navigate messages in the timeline, so we shouldn't use it now for the help window, then? Hopefully this argument sounds as devoid of logic to you for F1 as it does to me for tab.

My proposal was a modifier key in combination to a number, here it is using ALT: 8589a6f.

I still find these less intuitive than only tabbing from one tab to the other. What's the mnemonics for reminding oneself that Alt+3 is the linked chunk view? Compared to seeing the linked chunk view as the 4th tab in plain sight, a strong hint that I'll need to hit tab 3 times.

I wouldn't really think that this is a simplification, it's certainly more work to now move the input line and timeline to the RoomDetails struct.

It is actually surprising that the input line be present while I'm looking at the details view, because this suggests I can use it to send messages while I'm focused on a detailed view. However, this is not the case: typing has no effect, when I'm focused in a detailed view; as such, it must be hidden, or it mustn't be present in the first place. That's one more argument to move the input line into the timeline tab, and thus removing the last reason to move the timeline view into a tab. (The fact that it's slightly less convenient to implement doesn't make a strong case, since the tool is intended for being easy to use for debugging purposes, and if it's not the case, then it's failing at its primary goal.)

Please move the timeline view into the "detailed view" tabs, and make it so that it's always displayed.

[bnjbvr]

We had a live chat about this to unblock this PR, where I demo'd my current usage of multiverse: mostly go to a room, go to the linked chunk view, back-paginate, go to the messages view to check that are messages are coming into the right order, get back to the linked chunk view, back-paginate, get back to the timeline view, etc.

This helped @poljar understand my workflow a bit more, and he then suggested an even better solution to the problem: show the detailed views in a horizontal split of the room's view, at the same time as the timeline view. That way, it's possible to both see timeline items, and the details that relate to those. This nicely solves the problem by avoiding the need to switch from one view to the other all the time.

We discussed keybindings too:

• we could have a way to open the detailed view with Alt+ something (D for details?), then single-letter keybindings to navigate to a specific view (I proposed during this chat e for the raw Events, l for the Linked chunk and r for Read receipts),
• and/or also keybindings to split the view directly focused into an interested detailed view: could be Alt + the letter for each view.

With those changes in place, I'd be happy with the state of the tool, and would gladly approve it (I won't be there for the next week, so I'd suggest to not block on my review). Thanks for proposing the idea and to implement it Damir!

[Hywan]

I've reviewed the 53 commits and it makes to me. Thanks for having improved this client. It diverges quite a lot from my reimplementation, but I still believe we can find a way to merge both 😛.

[poljar]

I've reviewed the 53 commits and it makes to me. Thanks for having improved this client. It diverges quite a lot from my reimplementation, but I still believe we can find a way to merge both 😛.

That would be lovely, thanks for the swift ✅

On holiday, Ivan reviewed instead.