robrix icon indicating copy to clipboard operation
robrix copied to clipboard

render roomlist under people's header

Open alanpoon opened this issue 1 year ago • 2 comments

https://github.com/project-robius/robrix/issues/139

  • Created a new sigqueue for room_update: PENDING_IS_DIRECT_ROOM_UPDATES
  • Added is_direct boolean to differentiate between rooms belong to which header
  • Added new property for RoomsList widget: room_type

Currently, for some reasons, the program will hang when 2 portallists in RoomsList widget being drawn. (Fixed hang issue)

alanpoon avatar Oct 07 '24 05:10 alanpoon

Fixed Crash Screenshot 2024-10-07 172839

alanpoon avatar Oct 07 '24 09:10 alanpoon

Look nice! : ) It seems that you impl it in the drawing layer

aaravlu avatar Oct 07 '24 14:10 aaravlu

Thanks. As I mentioned previously in a different issue/PR, I don't believe this approach is going in the correct direction, unfortunately. Here are a few problems with this:

  • Storage of data vs. presentation of data are separate concerns, but this approach conflates those two things.
    • That is, the backend module here should not have to care about whether a room is direct or not, or whether the UI is displaying them separately. The backend should just provide all information to the frontend UI main thread, who can decide what to do with that info and how to display it.
  • Using two separate queues for sending updates from the background to the same widget is not only inefficient, it's overly complex and invites mistakes. Making all future devs deal with two redundant queues like this is a recipe for unmaintainability, as someone will inevitably forget to poll one queue or push to the proper queue.
  • A user may not actually care about separating rooms into different kinds, e.g., direct messages vs regular public rooms, etc, so they shouldn't pay the cost of that overhead if they don't want to. This PR forces that display choice in the actual backend, which is a strange choice.
  • We should not be storing application data-related globals in Cx (just my opinion, but I don't think that's a good use case for them).

There are a bunch more points to be made here, but I think that suffices. The good news is that I think we have different widgets available now that will make this easier, and we should use those. I can't recall who had discussed that, but Julian may know.

To reiterate, we do not need to restructure the backend data or communication paths to support this feature. We merely need to draw the rooms differently in the UI main thread only.

kevinaboos avatar Oct 30 '24 07:10 kevinaboos

Closing per my above comment. Feel free to re-open later, but also note that I am restructuring how rooms are stored in the list of all rooms as per #185

kevinaboos avatar Oct 30 '24 23:10 kevinaboos

See my latest PR #224 for more info on how I intended this design to work, which includes precise examples of the "storage vs. presentation" argument I made above.

kevinaboos avatar Oct 31 '24 16:10 kevinaboos