-
Notifications
You must be signed in to change notification settings - Fork 136
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
Containerized branch based on 303-proxy_http_ffmpeg_streams #305
base: master
Are you sure you want to change the base?
Conversation
…n files for container deployments
In the master, I included my css changes to splash.html and the full screen on button press when the splash page first loads |
…area, also updated the docker readme
…area, updated the Dockerfile to pull from this branch, and updated the docker readme
Fixed a few broken/incorrect links
#now-playing { | ||
padding: 10px 20px 10px 20px; | ||
border-bottom-left-radius: 20px; | ||
} | ||
#qr-code { | ||
max-width: 25%; | ||
border-top-right-radius: 20px; /* Rounded corner */ | ||
text-align: center; /* Center QR code */ | ||
padding: 20px; | ||
} | ||
#up-next { | ||
max-width: 75%; | ||
border-top-left-radius: 20px; /* Rounded corner */ | ||
padding: 10px 20px 10px 20px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look and I'm not crazy about these boxes. They cover up much more screen real estate than the text alone, much of it being blank space. If the goal is to make the text more readable, I feel that adding a black or semi-transparent stroke effect to them would achieve the same thing and minimize how much of the video this is all covering up. https://developer.mozilla.org/en-US/docs/Web/CSS/-webkit-text-stroke
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, stroke is not great because it's an internal, not external stroke so it makes the text thinner. But an 8-axis shadow should work:
https://stackoverflow.com/a/47511171/2909171
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.... I really like the look, to me it makes the interface very polished looking, and on most my screens it's rare they cover up important words.
I think what I might try to do is configure this so by default it's off, but ad a check box in the admin area so it can be turned on and off dynamically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do like the general look, I think it's just the excessive blank space that bugs me. Maybe if the singer name goes on the same line as the song title, it would be cleaner. Then give it a max-width so it will wrap if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, I don't think it's trivial to add admin settings checkboxes that will affect client side frontend. You'd probably have to dip into flask
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought, maybe the project in general should allow css overrides for custom "skinning". There's no way any interface will make everyone happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#qr-code { | ||
max-width: 25%; | ||
border-top-right-radius: 20px; /* Rounded corner */ | ||
text-align: center; /* Center QR code */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer for this to remain left-aligned so it's less likely to cover karaoke video text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can do, not a biggie. It looked a little awkward when I had a long url there... I have one instance spun up on my personal server, and I know it would look odd to have the QR code left aligned.... buuuut even centered, the long URL + the QR doesn't look the greatest graphically either.
Been pondering a way to make it look more visually appealing... or maybe make the url text disappear altogether once a certain character count is reached.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree its not great either way, maybe lowering the text size of the url would make it look more symmetrical. Or it might be worth adding a command line option to hide the url, but show the QR code, for minimalists. If that were the case a corner-aligned QR code would look pretty clean. But that's another ticket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…n a headless Ubuntu VPS
… connected to while troubleshooting
…o the splash screen... and possibly other guest connections
I'm trying to run this on my machine but i keep getting an error: PiKaraoke | ____ _ _ __ _ |
My first pull request. Not sure if I'm doing this right, but between GPT and KB's, things are happening. Resolves #293
Future commits with a more detailed readme on the docker deployment scenarios and options coming soon.