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

fix(YouTube): Do not hide player controls when using double tap to skip forward #4487

Merged

Conversation

LisoUseInAIKyrios
Copy link
Contributor

@LisoUseInAIKyrios LisoUseInAIKyrios commented Feb 21, 2025

Follow up fix to #4469

The prior fix could cause the player controls to permanently disappear if the screen is tapped while a fade out animation is running.

@MarcaDian
Copy link
Contributor

What if use an invisible placeholder in the xml of each button? I will try this option now.

@LisoUseInAIKyrios
Copy link
Contributor Author

I tried using animation listeners and set the buttons to GONE after the animation finishes, but it has issues with the buttons flickering and fading in/out inappropriately when tapping rapidly.

@MarcaDian
Copy link
Contributor

MarcaDian@d20c487

I haven't checked this commit yet, I'm running out of time.
I previously wanted to implement this as a second solution.
That is, the animations remains as it was, but during the animation of the button's disappearance, there will be a placeholder under it, which prevents the chapter title from climbing onto the button.

@LisoUseInAIKyrios
Copy link
Contributor Author

If a button is turned off in the settings while a video is open, then the placeholder is not set hidden and a blank space is kept.

Needs a minor adjustment to fix that otherwise this seems to work.

@MarcaDian
Copy link
Contributor

MarcaDian commented Feb 21, 2025

MarcaDian@ae9ee2d ? I think that fixes it

The disabled buttons disappear after tapping the screen after restoring from the miniplayer.

@MarcaDian
Copy link
Contributor

MarcaDian commented Feb 21, 2025

Only one small bug remains, if you tap on the screen and immediately swipe to the miniplayer, and then expand it, the buttons keep fading animation.
This is so ridiculous that it probably doesn't need fixing.
But it is possible to cause cleaning after changing the state of the player.
Is it necessary?

Screenrecorder-2025-02-22-00-02-36-656.mp4

@MarcaDian
Copy link
Contributor

Only one small bug remains, if you tap on the screen and immediately swipe to the miniplayer, and then expand it, the buttons keep fading animation.

Probably fix by MarcaDian@9094c8b
More tests are needed, but for now everything works as it should for me

@LisoUseInAIKyrios
Copy link
Contributor Author

It seems to be working now. I cannot reproduce any issues when entering/exiting fullscreen, so I'm unsure if MarcaDian@9094c8b is needed.

@MarcaDian
Copy link
Contributor

entering/exiting fullscreen

Miniplayer?

@LisoUseInAIKyrios
Copy link
Contributor Author

Actually, maximizing from miniplayer can incorrectly show the buttons for a moment.

Will incorporate later today MarcaDian@9094c8b

…' into fix_fade

# Conflicts:
#	extensions/youtube/src/main/java/app/revanced/extension/youtube/videoplayer/CopyVideoUrlButton.java
#	extensions/youtube/src/main/java/app/revanced/extension/youtube/videoplayer/CopyVideoUrlTimestampButton.java
#	extensions/youtube/src/main/java/app/revanced/extension/youtube/videoplayer/PlaybackSpeedDialogButton.java
#	extensions/youtube/src/main/java/app/revanced/extension/youtube/videoplayer/PlayerControlButton.java
@LisoUseInAIKyrios
Copy link
Contributor Author

I could only reproduce the miniplayer button visibility issue once, but I don't remember if the fullscreen button was also visible (if fullscreen was visible then it was not a bug and was the correct behavior).

I will try a bit more and see if the issue can be reproduced with what's currently here.

@MarcaDian
Copy link
Contributor

if the fullscreen button was also visible

Only the RV buttons were visible to me, see video above

@LisoUseInAIKyrios
Copy link
Contributor Author

Did you try with this PR? If you can reproduce then I'll integrate MarcaDian@9094c8b

@MarcaDian
Copy link
Contributor

MarcaDian commented Feb 22, 2025

Did you try with this PR?

Just tried it now.

Before
Screenrecorder-2025-02-22-00-02-36-656.gif

After
Screenrecorder-2025-02-22-10-49-27-377.gif

@LisoUseInAIKyrios
Copy link
Contributor Author

@MarcaDian any issues remaining? I think this is ready.

@MarcaDian
Copy link
Contributor

I used it for about an hour, I did not notice any bugs. Probably ready

@LisoUseInAIKyrios LisoUseInAIKyrios merged commit 63fe870 into ReVanced:dev Feb 22, 2025
1 check passed
@LisoUseInAIKyrios LisoUseInAIKyrios deleted the fix/controls_fade_out branch February 22, 2025 15:44
github-actions bot pushed a commit that referenced this pull request Feb 22, 2025
# [5.13.0-dev.9](v5.13.0-dev.8...v5.13.0-dev.9) (2025-02-22)

### Bug Fixes

* **YouTube:** Do not hide player controls when using double tap to skip forward ([#4487](#4487)) ([63fe870](63fe870))
@MarcaDian
Copy link
Contributor

😮‍💨 it feels like it will never end

Now
VID_20250224_192736.gif

5.12.0
VID_20250224_192831.gif

@LisoUseInAIKyrios
Copy link
Contributor Author

Try adding this snippet back again:

ec89649

protected void setVisibilityImmediate(boolean visible) {
        if (visible) {
            Utils.runOnMainThread(() -> private_setVisibility(true, false));
        } else {
            private_setVisibility(false, false);
        }

@MarcaDian
Copy link
Contributor

That's the first thing I thought, I'll have to try it later. 👌

@MarcaDian
Copy link
Contributor

Try adding this snippet back again:

This solved the problem.

@LisoUseInAIKyrios
Copy link
Contributor Author

Make a PR with the change

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