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.

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

Single session #239

Merged
merged 20 commits into from Nov 30, 2023
Merged

Single session #239

merged 20 commits into from Nov 30, 2023

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Nov 30, 2023

Fixes #236 #224 #200

This PR makes it so that the session picker is only shown when there is no session, so that users can click "Sign in". Once a session exists (user has logged-in), it will always automatically enter the existing session on page load. Additionally, when a session exists, "Back" buttons that would lead to the session picker will no longer be rendered.

This effectively makes it so that Chatrix is a single-session client.

TODO

  • If multiple sessions are encountered, log out all but the last, then enter the remaining one (the last one)
  • When session picker would be rendered, automatically enter the previously-existing session, if there is one. If not, show session picker.
  • If it's single-room mode, remove "Back" button in the room
  • If it's not single-room mode, remove "Back" button in Room list.

Screen recordings

Screen.Recording.2023-11-30.at.13.27.22.mov
Screen.Recording.2023-11-30.at.15.49.58.mov

@psrpinto psrpinto self-assigned this Nov 30, 2023
@psrpinto psrpinto linked an issue Nov 30, 2023 that may be closed by this pull request
@akirk
Copy link
Member

akirk commented Nov 30, 2023

This is great, thanks for working on this!
What happens for people who already have more than one session and we upgrade them? Can we take the last session? I heard that for those with multiple sessions the first sessions might have been broken, so we don't want them end up in a place where there would be one that worked but it's outside of reach.

@psrpinto
Copy link
Member Author

psrpinto commented Nov 30, 2023

Good point, I'm not sure what the best solution is here. When encountering multiple sessions, we could:

  1. Log them all out. User would see a blank session picker screen with just the login button
  2. Use the last one (not sure it's possible to know which is the last one)
  3. Use whichever session is first in the array of sessions

@akirk
Copy link
Member

akirk commented Nov 30, 2023

  1. Log them all out but the most recent one.

What about this?

@psrpinto
Copy link
Member Author

Sadly, it doesn't seem to be possible to know which session is the most recent. So I'll go with option 1.?

@akirk
Copy link
Member

akirk commented Nov 30, 2023

Don't they have ids? I'd go with the last in the list and log all the others out.

@psrpinto
Copy link
Member Author

The id is a random alphanumeric string, it's not incremental.

I'd go with the last in the list and log all the others out.

Alright, going with this one.

@psrpinto psrpinto force-pushed the single-session branch 2 times, most recently from 799235f to 3d8467b Compare November 30, 2023 12:57
@psrpinto psrpinto marked this pull request as ready for review November 30, 2023 13:29
@psrpinto
Copy link
Member Author

The screen recording doesn't shown it, but only one of the sessions was kept, the other one was logged out.

@akirk
Copy link
Member

akirk commented Nov 30, 2023

Maybe it's my local setup but when I try to build this I get:

yarn run v1.22.21
$ tsc && yarn run init && vite build --config frontend/vite-app.ts --emptyOutDir && yarn run build:logout && yarn run build:iframe && yarn run build:block
frontend/iframe/viewmodels/RootViewModel.ts:219:44 - error TS2339: Property 'resolveRoomAlias' does not exist on type 'HomeServerApi'.

219         let response = await homeserverApi.resolveRoomAlias(roomIdOrAlias).response();
                                               ~~~~~~~~~~~~~~~~

frontend/iframe/views/SessionView.ts:17:39 - error TS2307: Cannot find module 'hydrogen-web/src/platform/web/ui/session/room/WorldReadableRoomView' or its corresponding type declarations.

17 import { WorldReadableRoomView } from "hydrogen-web/src/platform/web/ui/session/room/WorldReadableRoomView";
                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 2 errors in 2 files.

Errors  Files
     1  frontend/iframe/viewmodels/RootViewModel.ts:219
     1  frontend/iframe/views/SessionView.ts:17
error Command failed with exit code 2.

@psrpinto
Copy link
Member Author

Mmm, that's weird. The build succeeded so must be something about your local setup indeed.

You could try:

rm -rf node_modules vendor
make

Hopefully that will fix it

@akirk
Copy link
Member

akirk commented Nov 30, 2023

Still the same problem. I just wanted to help and test but I suppose I need to leave it to others.

@psrpinto
Copy link
Member Author

It's weird, it should just work, not sure what could be the culprit.

@psrpinto
Copy link
Member Author

I noticed some other places where the session picker is shown (before a session exists), working on fixing those here too.

@psrpinto
Copy link
Member Author

AFAICT, the session picker is now never shown. I added another screen recording that shows the login/logout flow.

@psrpinto
Copy link
Member Author

I force-pushed to remove a useless wip commit that only changed whitespace.

Copy link
Member

@ashfame ashfame left a comment

Choose a reason for hiding this comment

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

LGTM! Testing it to be functional as expected!

@psrpinto
Copy link
Member Author

Thanks for testing @ashfame! Merging.

@psrpinto psrpinto merged commit 2ba3019 into main Nov 30, 2023
2 checks passed
@psrpinto psrpinto deleted the single-session branch November 30, 2023 16:55
@psrpinto psrpinto mentioned this pull request Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment