Refactor Room History = Fewer Future Bugs

This MR tries to improve maintainability of Fractal by redesigning the code path that displays model data in the message history UI.

Introduction

Currently updating the message history UI with new messages, edits, temporary messages etc. is done in a variety of places. This makes it hard to reason about the program, what the effect of changes might be and introduces a lot of special-casing in all of these places. This MR introduces a more functional approach with better separation between the model and view. Thus, it makes it easier to reason about the data flow in the program and by that hopefully reduces the amount of bugs and maintainance needed.

I started with this MR because I was working on reactions (#530 (closed)) and didn’t want to complicate the structure of the UI code even further (and by that drawing the anger of all other developers to me 😄). I hope that this design cleanup will make it easier for others and for me to implement new features.

Changes

The main focus of this MR regards the update of the MessageHistory view. I changed its code, so that there exists only a single update method, which handles all changes required by an updated model. Everything else are adjustments to cater to this change.

The new data flow looks like this:

• The update of the message history is initiated by an action, usually from inside AppOp, after the model is populated with new data.
• As the first step, we take the room model with all its messages and generate a list of Elements containing MessageContent structs (this is the viewmodel). It is kind of a blueprint for the final widget layout.
• We then compare the old Element list from the last update and the new one and act on all the changes that occur between them (performed by the apply_diff function).
• For each change encountered we insert/remove(/alter) a widget. As this does only affect changes, most widgets and their view-specific state are left untouched 🚀.
• After this, the message history is now representing the updated model.

I tried to make the individual commits in this MR readable, so that the changes can be more easily comprehended. The commits in this MR are organized by having the first few commits setting up the “new” data path and the last commits wire everything up and remove the remaining now-dead code.

The following commits are pretty self-containing and I can make them into separate MRs – if you prefer so:

• Add apply_diff helper function
• Make {model.Message, MessageContent, Element} order by date…
• Move constructor methods to MessageContent
• Make RoomHistory constructor always succeed

Background

The main driver for this change is the separation of the model and view. I think, much of Fractal’s code can be described in terms of the following concepts:

• Model: Holds all state of the program (business logic). An example is the model.Message struct. AppOp is the place where the model resides in Fractal.
• Actions: Can be triggered to perform changes to the model.
• Viewmodel: Is caclulated as a pure function from the model. The viewmodel suits the needs of a concrete view. An example is the MessageContent struct.
• View: The UI. Depends only on the current viewmodel to determine its state. (An exception is view specific state like e.g. scroll position.)

Example: The redaction of a message with this framework could work in this (simplified) way: First, an action to redact a message is triggered by the user, which causes the model to mark the message as redacted. The model update in turn causes the calculation of the viewmodel. The viewmodel will have the redacted message filtered out. This shorter list of messages gets then passed to the view, which adjusts its state to represent the new viewmodel – the redacted message is gone from the UI.

If you did wonder, this tries to work similar to the likes of Elm and React (quite functional, having a good distinction between model and view, and updating only parts of the view that require it).

Draft Notes

Currently this MR is a draft and there are quite some unfinished/untested bits. I wanted to get this out as early as possible to get feedback from everyone as this is a quite big and non-trivial change.

Open tasks:

• Wait for !626 (merged), !648 (merged), and rebase
• Update/replace widgets when Elements’ state changes
• Handle temp messages
• Unit tests
• Reintroduce and/or test
  • Message loading on scroll (#543 (closed) remains)
  • Message lazy loading
  • Sending messages
  • Edits
  • Redactions
  • Video player
  • Media attachments
  • Dividers
  • Headers

Known Bugs:

• Scroll position starts always at the top
• New message divider at top even if no new messages
• New message notification even if no new messages
• Scrollback is not progressing after first load while in a room
• Using Ctrl + PgDown after switching rooms freezes the app
• Edits are not always recognized
• GLib error message: Source ID XXX was not found when attempting to remove it (Not caused by this MR)

Hopefully, you can see the same value in this effort as I do. I’m glad to answer any questions you might have and am happy to hear your feedback on this MR! :)