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

Extract view click listeners from Player #8011

Merged
merged 7 commits into from
Mar 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 53 additions & 78 deletions app/src/main/java/org/schabi/newpipe/player/Player.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@
import org.schabi.newpipe.player.helper.AudioReactor;
import org.schabi.newpipe.player.helper.LoadController;
import org.schabi.newpipe.player.helper.MediaSessionManager;
import org.schabi.newpipe.player.helper.PlaybackParameterDialog;
import org.schabi.newpipe.player.helper.PlayerDataSource;
import org.schabi.newpipe.player.helper.PlayerHelper;
import org.schabi.newpipe.player.listeners.view.PlaybackSpeedClickListener;
import org.schabi.newpipe.player.listeners.view.QualityClickListener;
import org.schabi.newpipe.player.playback.CustomTrackSelector;
import org.schabi.newpipe.player.playback.MediaSourceManager;
import org.schabi.newpipe.player.playback.PlaybackListener;
Expand Down Expand Up @@ -530,9 +531,12 @@ private void initPlayer(final boolean playOnReady) {
}

private void initListeners() {
binding.qualityTextView.setOnClickListener(
new QualityClickListener(this, qualityPopupMenu));
binding.playbackSpeed.setOnClickListener(
new PlaybackSpeedClickListener(this, playbackSpeedPopupMenu));

binding.playbackSeekBar.setOnSeekBarChangeListener(this);
binding.playbackSpeed.setOnClickListener(this);
binding.qualityTextView.setOnClickListener(this);
binding.captionTextView.setOnClickListener(this);
binding.resizeTextView.setOnClickListener(this);
binding.playbackLiveSync.setOnClickListener(this);
Expand All @@ -541,11 +545,15 @@ private void initListeners() {
gestureDetector = new GestureDetectorCompat(context, playerGestureListener);
binding.getRoot().setOnTouchListener(playerGestureListener);

binding.queueButton.setOnClickListener(this);
binding.segmentsButton.setOnClickListener(this);
binding.repeatButton.setOnClickListener(this);
binding.shuffleButton.setOnClickListener(this);
binding.addToPlaylistButton.setOnClickListener(this);
binding.queueButton.setOnClickListener(v -> onQueueClicked());
binding.segmentsButton.setOnClickListener(v -> onSegmentsClicked());
binding.repeatButton.setOnClickListener(v -> onRepeatClicked());
binding.shuffleButton.setOnClickListener(v -> onShuffleClicked());
binding.addToPlaylistButton.setOnClickListener(v -> {
if (getParentActivity() != null) {
onAddToPlaylistClicked(getParentActivity().getSupportFragmentManager());
}
});

binding.playPauseButton.setOnClickListener(this);
binding.playPreviousButton.setOnClickListener(this);
Expand Down Expand Up @@ -1926,7 +1934,7 @@ public void hideControls(final long duration, final long delay) {
}, delay);
}

private void showHideShadow(final boolean show, final long duration) {
public void showHideShadow(final boolean show, final long duration) {
animate(binding.playbackControlsShadow, show, duration, AnimationType.ALPHA, 0, null);
animate(binding.playerTopShadow, show, duration, AnimationType.ALPHA, 0, null);
animate(binding.playerBottomShadow, show, duration, AnimationType.ALPHA, 0, null);
Expand Down Expand Up @@ -3607,37 +3615,6 @@ public void onDismiss(@Nullable final PopupMenu menu) {
}
}

private void onQualitySelectorClicked() {
if (DEBUG) {
Log.d(TAG, "onQualitySelectorClicked() called");
}
qualityPopupMenu.show();
isSomePopupMenuVisible = true;

final VideoStream videoStream = getSelectedVideoStream();
if (videoStream != null) {
final String qualityText = MediaFormat.getNameById(videoStream.getFormatId()) + " "
+ videoStream.resolution;
binding.qualityTextView.setText(qualityText);
}

saveWasPlaying();
}

private void onPlaybackSpeedClicked() {
if (DEBUG) {
Log.d(TAG, "onPlaybackSpeedClicked() called");
}
if (videoPlayerSelected()) {
PlaybackParameterDialog.newInstance(getPlaybackSpeed(), getPlaybackPitch(),
getPlaybackSkipSilence(), this::setPlaybackParameters)
.show(getParentActivity().getSupportFragmentManager(), null);
} else {
playbackSpeedPopupMenu.show();
isSomePopupMenuVisible = true;
}
}

private void onCaptionClicked() {
if (DEBUG) {
Log.d(TAG, "onCaptionClicked() called");
Expand Down Expand Up @@ -3742,11 +3719,7 @@ public void onClick(final View v) {
if (DEBUG) {
Log.d(TAG, "onClick() called with: v = [" + v + "]");
}
if (v.getId() == binding.qualityTextView.getId()) {
onQualitySelectorClicked();
} else if (v.getId() == binding.playbackSpeed.getId()) {
Comment on lines -3745 to -3747
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

onPlaybackSpeedClicked();
} else if (v.getId() == binding.resizeTextView.getId()) {
if (v.getId() == binding.resizeTextView.getId()) {
onResizeClicked();
} else if (v.getId() == binding.captionTextView.getId()) {
onCaptionClicked();
Expand All @@ -3758,23 +3731,6 @@ public void onClick(final View v) {
playPrevious();
} else if (v.getId() == binding.playNextButton.getId()) {
playNext();
} else if (v.getId() == binding.queueButton.getId()) {
onQueueClicked();
return;
} else if (v.getId() == binding.segmentsButton.getId()) {
onSegmentsClicked();
return;
} else if (v.getId() == binding.repeatButton.getId()) {
onRepeatClicked();
return;
} else if (v.getId() == binding.shuffleButton.getId()) {
onShuffleClicked();
return;
} else if (v.getId() == binding.addToPlaylistButton.getId()) {
if (getParentActivity() != null) {
onAddToPlaylistClicked(getParentActivity().getSupportFragmentManager());
}
return;
} else if (v.getId() == binding.moreOptionsButton.getId()) {
onMoreOptionsClicked();
} else if (v.getId() == binding.share.getId()) {
Expand Down Expand Up @@ -3803,23 +3759,33 @@ public void onClick(final View v) {
context.sendBroadcast(new Intent(VideoDetailFragment.ACTION_HIDE_MAIN_PLAYER));
}

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);
}
}
});
manageControlsAfterOnClick(v);
}

/**
* Manages the controls after a click occurred on the player UI.
* @param v – The view that was clicked
*/
public void manageControlsAfterOnClick(@NonNull final View v) {
if (currentState == STATE_COMPLETED) {
return;
}

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);
}
}
});
}

@Override
Expand Down Expand Up @@ -4446,6 +4412,10 @@ public boolean isSomePopupMenuVisible() {
return isSomePopupMenuVisible;
}

public void setSomePopupMenuVisible(final boolean somePopupMenuVisible) {
isSomePopupMenuVisible = somePopupMenuVisible;
}

public ImageButton getPlayPauseButton() {
return binding.playPauseButton;
}
Expand Down Expand Up @@ -4527,6 +4497,11 @@ public ExpandableSurfaceView getSurfaceView() {
public PlayQueueAdapter getPlayQueueAdapter() {
return playQueueAdapter;
}

public PlayerBinding getBinding() {
return binding;
}

//endregion


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package org.schabi.newpipe.player.listeners.view

import android.util.Log
import android.view.View
import androidx.appcompat.widget.PopupMenu
import org.schabi.newpipe.MainActivity
import org.schabi.newpipe.player.Player
import org.schabi.newpipe.player.helper.PlaybackParameterDialog

/**
* Click listener for the playbackSpeed textview of the player
*/
class PlaybackSpeedClickListener(
private val player: Player,
private val playbackSpeedPopupMenu: PopupMenu
) : View.OnClickListener {

companion object {
private const val TAG: String = "PlaybSpeedClickListener"
}

override fun onClick(v: View) {
if (MainActivity.DEBUG) {
Log.d(TAG, "onPlaybackSpeedClicked() called")
}

if (player.videoPlayerSelected()) {
PlaybackParameterDialog.newInstance(
player.playbackSpeed.toDouble(),
player.playbackPitch.toDouble(),
player.playbackSkipSilence
) { speed: Float, pitch: Float, skipSilence: Boolean ->
player.setPlaybackParameters(
speed,
pitch,
skipSilence
)
}
.show(player.parentActivity!!.supportFragmentManager, null)
} else {
playbackSpeedPopupMenu.show()
player.isSomePopupMenuVisible = true
}

player.manageControlsAfterOnClick(v)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package org.schabi.newpipe.player.listeners.view

import android.annotation.SuppressLint
import android.util.Log
import android.view.View
import androidx.appcompat.widget.PopupMenu
import org.schabi.newpipe.MainActivity
import org.schabi.newpipe.extractor.MediaFormat
import org.schabi.newpipe.player.Player

/**
* Click listener for the qualityTextView of the player
*/
class QualityClickListener(
private val player: Player,
private val qualityPopupMenu: PopupMenu
) : View.OnClickListener {

companion object {
private const val TAG: String = "QualityClickListener"
}

@SuppressLint("SetTextI18n") // we don't need I18N because of a " "
override fun onClick(v: View) {
if (MainActivity.DEBUG) {
Log.d(TAG, "onQualitySelectorClicked() called")
}

qualityPopupMenu.show()
player.isSomePopupMenuVisible = true

val videoStream = player.selectedVideoStream
if (videoStream != null) {
player.binding.qualityTextView.text =
MediaFormat.getNameById(videoStream.formatId) + " " + videoStream.resolution
}

player.saveWasPlaying()
player.manageControlsAfterOnClick(v)
}
}
Loading