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

Improve Room Feature #12

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

schlagmichdoch
Copy link

@schlagmichdoch schlagmichdoch commented Oct 3, 2022

This PR combines #9 , #10 and #11 .

It implements #8 by introducing temporary keyRooms with room keys.

What is new:

  • RoomIds are now created by using crypto.randomUUID()
  • Room keys are 6-digit long and expire after 10 min automatically.
  • UI is implemented to create a temporary room key immediatly when a room is created and via button.
  • Rooms cannot be created with user specified room key. If the user input is 6-digit number and no fitting keyRoom exists a message is shown that the room key is invalid
  • Rooms can be joined by inserting a room key manually, by using a temporary room link with parameter ?room_key=123456 or by using a permanent room link with parameter ?room_id=2f79f900-68d6-43a5-8ca0-a6fd8c0a7100
  • QRCode is shown to connect quickly from smartphones via the temporary link
  • Additionally, this PR replaces location.reload() with an event to reconnect to the websocket;

To add a qr-code, I cherry-picked a commit from @SuperSandro2000 that I found simple and lightweight enough.

UI

Mobile (Darkmode)

localhost_8080_(iPhone 12 Pro)

Desktop (Lightmode)

localhost_8080_(Desktop 1080p)

schlagmichdoch and others added 18 commits October 1, 2022 12:36
…ly method on PersistentStorage.set(key, value)
@schlagmichdoch schlagmichdoch changed the title WIP: Improve Room Feature Improve Room Feature Oct 5, 2022
@yjmp14
Copy link
Owner

yjmp14 commented Nov 16, 2022

Have you tested the code? The message and file transfer functions don't seem to work.

@schlagmichdoch
Copy link
Author

schlagmichdoch commented Nov 17, 2022

Have you tested the code? The message and file transfer functions don't seem to work.

I overlooked two occurences when refactoring .ip to .roomId on server/index.js

Now everything should be working fine again.

Other changes:

  • there is no delay anymore between drawing of peersui and circles
  • cache_name version number is increased by one to force an update of cached scripts
  • server is not failing anymore if roomkey parameter is not included on wss connection request (as it is currently the case)
  • merged master into branch

@schlagmichdoch
Copy link
Author

@yjmp14 what do you think?

@yjmp14
Copy link
Owner

yjmp14 commented Nov 22, 2022

@yjmp14 what do you think?

Sorry, I'm busy with other things these few days, and I'm still thinking about how to reduce the complexity of the UI and interaction.

@schlagmichdoch
Copy link
Author

Do you think the UI is getting too complicated?

I‘d suggest splitting the roomDialog into two:

  • Top Part: Big Button ‚Create new room‘
  • Bottom Part: Input Field and Button ‚Join Room‘ which is grayed out and only becomes clickable when 6 digits are entered.

Also the implementation of this PR has the 6-digit code as a one time key (nonce), which gets invalidated as soon as someone is joining. Should we change this?

@yjmp14
Copy link
Owner

yjmp14 commented Nov 22, 2022

What I'm thinking about:

  • Is it necessary to show the long uuid room id to user?
  • Is it necessary to keep two types of sharing links, a temporary one and a permanent one?
  • Under what circumstances do users need to copy a link? Since the success rate of file/message transfer using this program cannot be guaranteed, it is only suitable for face-to-face use, or even only for transferring between two devices of the same person. So, do users really need a sharing link?

And I don't think it's necessary to make the 6-digit code valid for only one time, we can consider shortening the valid time to, say, 1 minute or 3 minutes.

@schlagmichdoch
Copy link
Author

  • Is it necessary to show the long uuid room id to user?

No, I also thought about that. I think hiding complexity from the user is a good approach.

  • Is it necessary to keep two types of sharing links, a temporary one and a permanent one?

The reason are two different use cases:

  • A temporary URL is used for the QR Code as an alternative to typing in the 6-digit-key. It does not include the room ID itself and is more secure as a result
  • A permanent URL is used to enter any room again even after every peer left. The usecase is to bookmark the URL on all devices and find the own devices instantly even if not in the same network. (See [feature-request] add long, secure room id to access room via bookmarked url #8 )
  • Under what circumstances do users need to copy a link?
  • Permanent link
    • bookmark to find own devices faster
  • Temporary link
    • Mainly for the QR-Code
    • As an edge case I could think of sending someone the link while in a video call to send sensitive files via peer to peer connection
    • Send someone a link through a messanger that does not support file sharing
    • We could remove the "Copy Temporary Link" in favour of a "Close" Button that does not cancel the room key but I'd definitely leave the temporary link for the QR-Code

the success rate of file/message transfer using this program cannot be guaranteed

Why not? There are notifications indicating the completion of message and file transfer.

And I don't think it's necessary to make the 6-digit code valid for only one time, we can consider shortening the valid time to, say, 1 minute or 3 minutes.

I'm not sure about that. I guess 10 minutes are fine as the expiration time. When we make the room key valid for more than one time, the user experience should still indicate to the sending user that a device has joined successfully and close the RoomDialog as it is the case now. Maybe we could then add the time as a pill at the bottom:


Enter the room key on other devices to join this room



                    You are: 6119
      Room Key: `203904` - expires in 04:45 🛈

Tapping on the bottom line opens up the roomDialog again with the possibility to cancel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants