Extract view click listeners from Player

[XiangRongLin]

What is it?

• Codebase improvement (dev facing)

Description of the changes in your PR

• Extract the listeners for views out of the player into their own classes
• It continues to be tightly coupled, but when they are all extracted it should give a better picture of what is needed and what not. Thus allowing to clean them up a bit.
• Only did 2 as a sample and ask for feedback before doing all.

Fixes the following issue(s)

• relates Rewrite the player (and all its components) #7939

Due diligence

• I read the contribution guidelines.

[litetex]

Some documentation would be good in that class.

[XiangRongLin]

Problem is I don't know what the listener does. The focus of the PR is extracting the code out of the player. Adding documentation would require more effort, which I don't want to put into it.

[litetex]

I would rename it to PlaybackSpeedClickListener

[litetex]

this if is useless

[litetex]

Same statements as above also apply for this class

[litetex]

Are the ( ) required?

[litetex]

Would rename the class to QualityTextClickListener

[Fs00]

In my personal opinion those listeners would look much better as lambdas/method references, like:

binding.qualityTextView.setOnClickListener(this::onQualitySelectorClicked);

What I don't like much about listeners in separate classes, other than the pre-Java 8 kind of verbosity, is that it's being introduced a layer of "public-internal" APIs (like setSomePopupMenuVisible, getBinding) that exposes internal details of the Player to consumers.
Also, this kind of extraction doesn't seem too beneficial to me, since it adds more indirection and verbosity without really making it easier to understand of how the different parts of Player work.

[Fs00]

Also, I've just noticed now that those listeners can't even be extracted from here since there is the following logic at the end of the onClick method that runs regardless of the item that gets clicked:

NewPipe/app/src/main/java/org/schabi/newpipe/player/Player.java

Lines 3738 to 3753 in 47f9ed0

	if (currentState != STATE_COMPLETED) {
	controlsVisibilityHandler.removeCallbacksAndMessages(null);
	showHideShadow(true, DEFAULT_CONTROLS_DURATION);
	animate(binding.playbackControlRoot, true, DEFAULT_CONTROLS_DURATION,
	AnimationType.ALPHA, 0, () -> {
	if (currentState == STATE_PLAYING && !isSomePopupMenuVisible) {
	if (v.getId() == binding.playPauseButton.getId()
	// Hide controls in fullscreen immediately
	|| (v.getId() == binding.screenRotationButton.getId()
	&& isFullscreen)) {
	hideControls(0, 0);
	} else {
	hideControls(DEFAULT_CONTROLS_DURATION, DEFAULT_CONTROLS_HIDE_TIME);
	}
	}
	});

There are only a few branches in onClick that do an early return and can therefore be safely extracted.

[Stypox]

Maybe that part could be extracted in a player function? Something like maintainControlsShown or something like that (I think there is a particular verb for that in the english language but I cannot remember it rn)

[litetex]

Fixed problems myself.

@Stypox You may check my implementation of manageControlsAfterOnClick 😄

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[Stypox]

I tested and it works, thank you :-)