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.

Sign up for GitHub

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

Jump to bottom

Peeking improvements #5

Closed
wants to merge 9 commits into from
Closed

Peeking improvements #5

wants to merge 9 commits into from

Conversation

ashfame
Copy link
Member

@ashfame ashfame commented Feb 17, 2023 •
edited
Loading

Draft PR (only meant for review and not merge)

  • Delete data when navigating away from the room
  • Handle matrix.org edge case when it chooses to block returning events for world_readable room
  • Fix bug with not being able to render regular rooms when navigating away from unknown room (the url changes but UI doesn't change)
  • Join functionality

@ashfame ashfame self-assigned this Feb 17, 2023
@ashfame
Copy link
Member Author

ashfame commented Feb 17, 2023

Bug fix with navigation is also fixed, just need to do some more testing with timeline template being rendered fine, which I am currently unable to do so as:

  • Local synapse is borked (spent time fixing it but still need to do more)
  • Matrix.org homeserver is not liking the preview at all today
  • Orbit sandbox (same restrictions as matrix.org)

I will pick this up on Monday, by either fixing my local Synapse or having a world_readable room created on orbit-sandbox with a handful of messages by someone that I can just use with my orbit-sandbox account to test the timeline template, before making the commit.

@ashfame ashfame requested a review from psrpinto February 22, 2023 09:38
@ashfame
possible template fix to fix navigating out of unknown room
554f517
@ashfame
split preview view out of UnknownRoomView into WorldReadableRoomView …
e6c4141 
…and re-render the middle for WorldReadableRoomView to take over
@ashfame
separate WorldReadableRoomViewModel out of UnknownRoomViewModel
7bba357
@ashfame
move WorldReadableRoomView out to its own file and create WorldReadab…
3a700ba 
…leRoomViewModel right in the RoomViewModelObservable

create a new RoomStatus enum value for WorldReadable and utilise a promise to check whether room is previewable which first UnknownRoomViewModel uses to show UI context and then switches to WorldReadableRoomViewModel to render timeline for preview of messages
@ashfame ashfame force-pushed the peeking_improvements branch from ccadd66 to 3a700ba Compare February 23, 2023 14:02
@ashfame
Copy link
Member Author

ashfame commented Feb 23, 2023

Complete Peeking PR changes can be seen on this link https://github.com/vector-im/hydrogen-web/compare/master...Automattic:hydrogen-web:peeking_improvements?expand=1

@ashfame ashfame marked this pull request as ready for review February 23, 2023 14:59
ashfame
ashfame commented Feb 23, 2023
View reviewed changes
src/domain/session/room/WorldReadableRoomViewModel.js

export class WorldReadableRoomViewModel extends RoomViewModel {
constructor(options) {
options.room.isWorldReadable = true;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@psrpinto Do you think this is fine or it warrants the need of creating a WorldReadableRoom like ArchivedRoom just because we need to differentiate between the room instance of a regular room and WorldReadable room in RoomViewModel?

@ashfame
Copy link
Member Author

ashfame commented Feb 23, 2023

Quick update: Join functionality is now functional but I have noticed sometimes the messages don't show up after joining the room. It worked fine in Chrome, atleast in the last few attempts but in Firefox I just get an empty room with no messages. Will continue to look into this more.

@ashfame ashfame force-pushed the peeking_improvements branch from 10415d1 to 8f20f97 Compare February 24, 2023 09:31
@ashfame
Copy link
Member Author

ashfame commented Feb 25, 2023

Squashed commits here and pushed to original PR

@ashfame ashfame closed this Feb 25, 2023
@ashfame ashfame deleted the peeking_improvements branch February 25, 2023 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@psrpinto psrpinto Awaiting requested review from psrpinto

Assignees

@ashfame ashfame

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ashfame