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

IT/1959 Loop/Repeat Parity #1990

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

kjonathan024
Copy link
Contributor

Improved the sync of the repeat and loop buttons to be visually and functionally linked when the user turns both buttons on. Before, I noticed that when one was set to on and the other was then added, the visual indicators of if the video would loop or not was indiscernable. I fixed this by detecting if the other button was active and then setting their opacities to be the same based on the current setting of the video (if looping or not). I did this through the onClick functions for each button respectfully, as well as adding an if statement check when the button is first added to ensure it is visually and functionally in sync.

@ImprovedTube
Copy link
Member

ImprovedTube commented Feb 7, 2024

hi @kjonathan024! thank you!
'.improvedtube-player-button' must be the group of all our buttons below the player? (not only the loop button)


querySelector by class can be "slow. (Might only be worth considering because it is enabled by default and can run billions of times). Generally one could use getElementById. But we can also just call it on click only, and then we can specifically check our settings in that are loaded already, to know if the other button is enabled ( if (this.storage.player_repeat_button === true) { and if ( this.storage.below_player_loop !== false) { respectively. - Or both buttons could share the same .css class, and change the class' opacity on click.


just added to #1504 :

  • separate the 'always repeat'-feature from the buttons (in our function & GUI), so that it works alone. ImprovedTube.alwaysRepeat = function () { if(this.storage.player_always_repeat === true) .

@kjonathan024
Copy link
Contributor Author

hi @ImprovedTube ! Thank you for your comments, I have made appropriate changes based on your recommendations.

@ImprovedTube ImprovedTube merged commit 282f7d6 into code-charity:master Feb 8, 2024
ImprovedTube added a commit that referenced this pull request Feb 8, 2024
ImprovedTube added a commit that referenced this pull request Feb 8, 2024
ImprovedTube added a commit that referenced this pull request Feb 8, 2024
ImprovedTube added a commit that referenced this pull request Feb 8, 2024
@ImprovedTube
Copy link
Member

hi! thanks @kjonathan024, hope you like the 4 commits above and they don't break anything.

just added to #1504 :

  • separate the always repeat switch from the buttons (in our function & GUI), so that it works alone. ImprovedTube.alwaysRepeat = function () { if(this.storage.player_always_repeat === true)
  • allow repeating by category only like music or education (implemented at our speed-watching feature only yet. but many of our features might consider such data in future)
  • remember looping (once or unlimited) for each video
  • allow sticky-on with rightclick (faster than finding always repeat button in our list of features) compare <video> Player buttons to be added</video> #1445

ImprovedTube added a commit that referenced this pull request Feb 8, 2024
@kjonathan024 kjonathan024 deleted the improvement/1959 branch February 8, 2024 14:37
@kjonathan024
Copy link
Contributor Author

kjonathan024 commented Feb 8, 2024

@ImprovedTube Thank you for your guidance and support! Glad to be able to contribute to your project. From the testing that I did, it seems that I remedied any small bugs that happened because of the change.

@JourneyOver
Copy link

JourneyOver commented Feb 18, 2024

I think this pr/commit broke something in 4.641 (my extension just updated to 4.641 (from version 4.615) as of an hour ago and that is when I started to notice the following issue) as now any videos seem to just loop when it gets to the end of the video even though repeat/loop video is turned off in the settings...

I turned on the option to show the loop button under the video and any videos I go to seem to have the icon light up
Screenshot 2024-02-18 031557 and the video loops back to the beginning again even though I have nothing for looping/repeating videos turned on at all in the settings.. even if I click the icon and turn it off if I refresh the page or go to another video it automatically seems to turn itself back on again.

Edit: Downgraded back to 4.615 (moving all the files from 4.615 to 4.641 and replacing everything) and the issue stops... idk if it is this pr exactly but since this PR was to do with loop/repeat I am in my mind thinking it has something to at least do with it.

@deadpixel134
Copy link

I just encountered the same issue.
Turning off this extension makes the problem go away.

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.

4 participants