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 condition to keep video paused if leader leaves/changes #53

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

Conversation

mattman107
Copy link

Hello let me preface this PR with this is my first time using Haxe, and I am by no means a web developer. So please look this over and let me know if I messed anything up or if there is anything problematic with my logic here.

What brought my desire for this feature to be a thing is that my friends and I sometimes pause videos and leave them in the queue for a couple of days. I realized that if all users disconnected from the webpage, and one person rejoined the video would unpause. This kind of sucked because we would then have to re-add the video to the queue or the person who rejoined would have to manually pause it.

I have added a new boolean variable in the default-config.json and referenced it in the server Main.hx file to control when a video should resume. I set it to false by default so videos won't resume in these scenarios (of course if you would prefer it be true we can go with that).

@mattman107
Copy link
Author

I'm also thinking about giving a bit more granularity for these. So maybe different variables for the different message types (because right now I only have it setup for one variable to control them all).

@RblSb
Copy link
Owner

RblSb commented Apr 21, 2024

Thank you for your contribution.

This feature will require additional indication that the video is paused by the server, just as a pause is currently displayed on clients when there is a leader.

Even now the presence of a pause is not obvious enough, and it might be worth making some kind of overlay on top of the video with a panel in the corner with the author of the current pause. Unfortunately, I’m also not very good in html/css, and such tasks are very painful.

@mattman107
Copy link
Author

Thank you for the input.

Let me see what I can do. I would consider myself a novice in all of these technologies (html/css/js), but maybe I can whip something up for displaying what is keeping a video paused.

I was also planning on taking a crack at #52, so maybe this would be a good warm up.

@RblSb
Copy link
Owner

RblSb commented Apr 21, 2024

If it becomes too difficult, I will help finish this PR later.

The minimum necessary indication would be control through the setPauseIndicator function, which changes the icon under the player, and also when a user connects write a server message in the chat if the video is paused by server and that the pause needs to be removed (I don’t know how it is removed currently with your implementation, just by video play button?)

@mattman107
Copy link
Author

mattman107 commented Apr 26, 2024

Hello back again with some progress. This isn't ready by any means but I figured I would show what I think it should kinda look like.

I really liked the way the client list looks so I tried emulating how that works but including my jank stuff for when a user pauses. Major issues that are still present:

  • The paused by list never disappears
  • The paused by name always uses the name of the client who's browser one is viewing from
  • I wanted that slick animation to play that the client list has when it opens and closes but couldn't figure it out
  • If the client who paused leaves it doesn't switch to saying that it is paused by the server

@RblSb
Copy link
Owner

RblSb commented Apr 28, 2024

This is not bad, but it duplicates the users panel and current userlist would be enough to display a pause if the server was already displayed in it as item.

But I'm thinking of trying to make a fixed pop-up panel above the chat input field with the text like:
Paused by server
(click to unpause)
It could also be reused by showing a button to skip ad blocks in YouTube videos and for some other simple actions.

@mattman107
Copy link
Author

Got it. When I get some time I will revise.

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.

2 participants