Selectable behavior of Back button #6895
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -755,6 +755,28 @@ public boolean onBackPressed() { | |||||||
| Log.d(TAG, "onBackPressed() called"); | ||||||||
| } | ||||||||
|
|
||||||||
| // User can choose a behavior of the back button. | ||||||||
| // We go down and check if it's OK to stop at some point based on the user's | ||||||||
| // preference. | ||||||||
| // For example, if the user always wants to minimize into the mini player | ||||||||
| // when the backstack is empty, we have to ensure that: | ||||||||
| // - nothing is in the backstack | ||||||||
| // - nothing is in the PlayQueue's history | ||||||||
| // - the player is in non-fullscreen mode. | ||||||||
| // Otherwise it takes priority and the minimize action will not happen right now | ||||||||
| final int behavior = PlayerHelper.getBackBehavior(activity); | ||||||||
|
|
||||||||
| if (behavior == PlayerHelper.BackBehavior.BACK_BEHAVIOR_CLOSE_FULLSCREEN) { | ||||||||
| if (!isPlayerAvailable() || player.videoPlayerSelected()) { | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); | ||||||||
| requireView().postDelayed(this::rotateScreenOnPhoneIfInLandscape, 500); | ||||||||
|
There was a problem hiding this comment. This looks suspicious, how did you calculate the 500ms delay to make sure the rotation happens correctly? There was a problem hiding this comment. Oh, forgot to made something about that. This delay was just brute-forced. If it will not work for someone I'll try to find a better way. |
||||||||
| } else { | ||||||||
| // Since it's not a video playing we can't close the player | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); | ||||||||
| } | ||||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| // If we are in fullscreen mode just exit from it via first back press | ||||||||
|
There was a problem hiding this comment. (obv break it in multiple lines)
Suggested change
|
||||||||
| if (isPlayerAvailable() && player.isFullscreen()) { | ||||||||
| if (!DeviceUtils.isTablet(activity)) { | ||||||||
|
|
@@ -765,6 +787,21 @@ public boolean onBackPressed() { | |||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| if (behavior == PlayerHelper.BackBehavior.BACK_BEHAVIOR_CLOSE) { | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| if (!isPlayerAvailable() || player.videoPlayerSelected()) { | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); | ||||||||
| } else { | ||||||||
| // Since it's not a video playing we can't close the player | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); | ||||||||
| } | ||||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| if (behavior == PlayerHelper.BackBehavior.BACK_BEHAVIOR_HIDE) { | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); | ||||||||
| return true; | ||||||||
| } | ||||||||
|
|
||||||||
| // If we have something in history of played items we replay it here | ||||||||
| if (isPlayerAvailable() | ||||||||
| && player.getPlayQueue() != null | ||||||||
|
|
@@ -773,10 +810,30 @@ public boolean onBackPressed() { | |||||||
| return true; // no code here, as previous() was used in the if | ||||||||
| } | ||||||||
|
|
||||||||
| // If we passed previous check at the top `player.getPlayQueue().previous()` | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| // then we can be sure that queue have no more streams in the history. | ||||||||
| // If the user chose BACK_BEHAVIOR_PREV_PLAYLIST behavior | ||||||||
| // we have to minimize the player into the mini player here | ||||||||
| if (behavior == PlayerHelper.BackBehavior.BACK_BEHAVIOR_PREV_PLAYLIST) { | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_COLLAPSED); | ||||||||
| return true; | ||||||||
|
Comment on lines
+818
to
+819
There was a problem hiding this comment. I still do not understand why this code would cause the previous item from the playlist to be played. Also, how do you check if a play queue item belongs to a playlist, and if its previous item also belongs to a playlist? And where is the code that queues the previous item from the playlist? Or maybe I didn't understand what you mean by playlist, do you mean play queue (though still the code would not explain that, either)? There was a problem hiding this comment. Yes, I mean PlayQueue that has a history of played items. It's a List that holds previously played items from this queue. Here https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java#L772 Maybe would be better to rename |
||||||||
| } | ||||||||
avently marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
|
||||||||
| // That means that we are on the start of the stack | ||||||||
| if (stack.size() <= 1) { | ||||||||
| restoreDefaultOrientation(); | ||||||||
|
|
||||||||
| if (behavior == PlayerHelper.BackBehavior.BACK_BEHAVIOR_HIDE_OR_CLOSE) { | ||||||||
|
There was a problem hiding this comment. Why is "BACK_BEHAVIOR_HIDE_OR_CLOSE" here and not just after "BACK_BEHAVIOR_HIDE"? Shouldn't the two act the same way, with the only difference that "BACK_BEHAVIOR_HIDE_OR_CLOSE" should close the player if nothing is playing? There was a problem hiding this comment. The idea is to have a backstack working (like in BACK_BEHAVIOR_PREV_ANY) but to allow to close mini player completely. With BACK_BEHAVIOR_PREV_ANY the mini player will be at the bottom even when video is paused. With this option you're able to hide it in this case. That's why it is the last check and it is inside |
||||||||
| if (!isPlayerAvailable() || player.getPlayQueue() == null | ||||||||
| || (player.videoPlayerSelected() | ||||||||
| && !player.getPlayQueue().equals(playQueue))) { | ||||||||
| bottomSheetBehavior.setState(BottomSheetBehavior.STATE_HIDDEN); | ||||||||
| return true; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| // let MainActivity handle the onBack (e.g. to minimize the mini player) | ||||||||
| return false; | ||||||||
| } | ||||||||
|
|
||||||||
| // Remove top | ||||||||
|
|
@@ -1092,6 +1149,15 @@ private void toggleFullscreenIfInFullscreenMode() { | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private void rotateScreenOnPhoneIfInLandscape() { | ||||||||
| if (!DeviceUtils.isTablet(activity) | ||||||||
| && !DeviceUtils.isTv(activity) | ||||||||
| && DeviceUtils.isLandscape(requireContext()) | ||||||||
| && globalScreenOrientationLocked(activity)) { | ||||||||
| onScreenRotationButtonClicked(); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| private void openBackgroundPlayer(final boolean append) { | ||||||||
| final AudioStream audioStream = currentInfo.getAudioStreams() | ||||||||
| .get(ListHelper.getDefaultAudioFormat(activity, currentInfo.getAudioStreams())); | ||||||||
|
|
@@ -2180,6 +2246,10 @@ private void showExternalPlaybackDialog() { | |||||||
| * Remove unneeded information while waiting for a next task | ||||||||
| * */ | ||||||||
| private void cleanUp() { | ||||||||
| // On every clean up operation we want to be in non-fullscreen mode, | ||||||||
| // since it returns UI into it's initial look when nothing is played | ||||||||
| toggleFullscreenIfInFullscreenMode(); | ||||||||
avently marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
|
|
||||||||
| // New beginning | ||||||||
| stack.clear(); | ||||||||
| if (currentWorker != null) { | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.