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

Add QR Code to player #57

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add QR Code to player #57

wants to merge 12 commits into from

Conversation

EffakT
Copy link

@EffakT EffakT commented Apr 17, 2023

This adds a QR code to the bottom left of the player, using the document.baseURI as the URL for the QR code.
Resolves #32

  • QR showing on page
  • QR code enabled/disabled
  • Position(s) (maybe alternate sides every song / at an interval to prevent issues with static elements on OLEDs)
  • Size
  • Translucency
  • Optionally Include room password (so room password would be autofilled)
  • URL (maybe? automatically using the base URI as you did might be fine)

@bhj
Copy link
Owner

bhj commented Apr 19, 2023

Hey, welcome and thanks for this!

The main reason this feature hasn't been implemented (yet) is that I'd like it to be a bit more flexible, which depends on there being per-room settings, which don't exist yet. Once they do, I think these ideally should be configurable per-room:

  • QR code enabled/disabled
  • Position(s) (maybe alternate sides every song to prevent issues with static elements on OLEDs)
  • Size
  • Translucency
  • Optionally Include room password (so room password would be autofilled)
  • URL (maybe? automatically using the base URI as you did might be fine)

It's a lot, I know, but users will (rightly) be picky about things on the screen all the time. This PR is a great reference for the core functionality though. Thanks again!

@EffakT
Copy link
Author

EffakT commented Apr 19, 2023

Sure, I'll look into adding settings. This was just a really "get a QR code on there" sort of thing for now.
Instead of alternating the location each song, maybe at an interval? in-case the screen is on, with nothing in the queue.

I'll update the PR with a checklist of requirements.

@EffakT
Copy link
Author

EffakT commented Apr 25, 2023

@bhj With regards to passing through the room password, What are your thoughts on using a nonce or short-term password for this? Would provide better security rather than passing the room password through to the client.

It could work in a way that after the QR code is used it regenerates a new password & QR code, leaving the previous password(s) available for a period of time (in-case multiple people scan the same code).

Basic steps would be:

  1. Generate a password & QR Code
  2. The client requests the page with the temporary password as a query string
  3. Client asks the server if password is correct (active or within the last few minutes)
  4. Server is notified the password is used, generates a new password and a new QR code.
  5. In the background, server can clean-up expired passwords to keep the database clean.

Let me know if you think this is too complicated, or if you have any other thoughts on how this should work.

@bhj
Copy link
Owner

bhj commented Apr 26, 2023

@EffakT Very nice work! And wow, there are still more class-style components left than I thought 😮‍💨

Let's keep the QR password stuff simple for now (maybe just base64 it?). I guess the question is what this means for the QR payload size, and whether we need to enforce a max length for room passwords.

@bhj
Copy link
Owner

bhj commented May 1, 2024

Just a note to say I haven't forgotten about this; been working on some long-standing things on the server side. Thanks again for the contribution and patience!

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.

[FEATURE REQUEST] - QR Code to load app on mobile devices
2 participants