From f62ae930c7b69d802debc3a44e294142bcdb5b87 Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Thu, 8 Feb 2018 19:53:04 -0800 Subject: [PATCH] -Merged bookmark buttons on playlist fragment into one. -Fixed bookmark button flickering on visibility toggling. -Removed toolbar up button control from local fragments, delegating functionality back to main fragment. -Updated extractor to latest. --- app/build.gradle | 2 +- .../list/playlist/PlaylistFragment.java | 72 ++++++++++--------- .../local/bookmark/BaseLocalListFragment.java | 15 ---- .../local/bookmark/BookmarkFragment.java | 9 --- app/src/main/res/menu/menu_playlist.xml | 12 +--- 5 files changed, 43 insertions(+), 67 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 86d6542e0..273616f91 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -55,7 +55,7 @@ dependencies { exclude module: 'support-annotations' } - implementation 'com.github.TeamNewPipe:NewPipeExtractor:7fd21ec08581d' + implementation 'com.github.TeamNewPipe:NewPipeExtractor:4fb49d54b5' testImplementation 'junit:junit:4.12' testImplementation 'org.mockito:mockito-core:1.10.19' diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java index 15c9d4b38..2c0b94c69 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/playlist/PlaylistFragment.java @@ -36,13 +36,16 @@ import org.schabi.newpipe.playlist.SinglePlayQueue; import org.schabi.newpipe.report.UserAction; import org.schabi.newpipe.util.ExtractorHelper; import org.schabi.newpipe.util.NavigationHelper; +import org.schabi.newpipe.util.ThemeHelper; import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; import io.reactivex.Single; import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.disposables.Disposable; +import io.reactivex.disposables.Disposables; import static org.schabi.newpipe.util.AnimationUtils.animateView; @@ -50,6 +53,7 @@ public class PlaylistFragment extends BaseListInfoFragment { private CompositeDisposable disposables; private Subscription bookmarkReactor; + private AtomicBoolean isBookmarkButtonReady; private RemotePlaylistManager remotePlaylistManager; private PlaylistRemoteEntity playlistEntity; @@ -70,7 +74,6 @@ public class PlaylistFragment extends BaseListInfoFragment { private View headerBackgroundButton; private MenuItem playlistBookmarkButton; - private MenuItem playlistUnbookmarkButton; public static PlaylistFragment getInstance(int serviceId, String url, String name) { PlaylistFragment instance = new PlaylistFragment(); @@ -86,6 +89,7 @@ public class PlaylistFragment extends BaseListInfoFragment { public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); disposables = new CompositeDisposable(); + isBookmarkButtonReady = new AtomicBoolean(false); remotePlaylistManager = new RemotePlaylistManager(NewPipeDatabase.getInstance(getContext())); } @@ -176,14 +180,14 @@ public class PlaylistFragment extends BaseListInfoFragment { inflater.inflate(R.menu.menu_playlist, menu); playlistBookmarkButton = menu.findItem(R.id.menu_item_bookmark); - playlistUnbookmarkButton = menu.findItem(R.id.menu_item_unbookmark); - - updateBookmarkButtonsVisibility(); + updateBookmarkButtons(); } @Override public void onDestroyView() { super.onDestroyView(); + if (isBookmarkButtonReady != null) isBookmarkButtonReady.set(false); + if (disposables != null) disposables.clear(); if (bookmarkReactor != null) bookmarkReactor.cancel(); @@ -199,6 +203,7 @@ public class PlaylistFragment extends BaseListInfoFragment { disposables = null; remotePlaylistManager = null; playlistEntity = null; + isBookmarkButtonReady = null; } /*////////////////////////////////////////////////////////////////////////// @@ -225,10 +230,7 @@ public class PlaylistFragment extends BaseListInfoFragment { shareUrl(name, url); break; case R.id.menu_item_bookmark: - bookmarkPlaylist(); - break; - case R.id.menu_item_unbookmark: - unbookmarkPlaylist(); + onBookmarkClicked(); break; default: return super.onOptionsItemSelected(item); @@ -343,7 +345,9 @@ public class PlaylistFragment extends BaseListInfoFragment { @Override public void onNext(List playlist) { playlistEntity = playlist.isEmpty() ? null : playlist.get(0); - updateBookmarkButtonsVisibility(); + + updateBookmarkButtons(); + isBookmarkButtonReady.set(true); if (bookmarkReactor != null) bookmarkReactor.request(1); } @@ -366,35 +370,39 @@ public class PlaylistFragment extends BaseListInfoFragment { headerTitleView.setText(title); } - private void bookmarkPlaylist() { - if (remotePlaylistManager == null || currentInfo == null) return; + private void onBookmarkClicked() { + if (isBookmarkButtonReady == null || !isBookmarkButtonReady.get() || + remotePlaylistManager == null) + return; - playlistBookmarkButton.setVisible(false); - playlistUnbookmarkButton.setVisible(false); + final Disposable action; - final Disposable disposable = remotePlaylistManager.onBookmark(currentInfo) - .observeOn(AndroidSchedulers.mainThread()) - .subscribe(ignored -> {/* Do nothing */}, this::onError); - disposables.add(disposable); + if (currentInfo != null && playlistEntity == null) { + action = remotePlaylistManager.onBookmark(currentInfo) + .observeOn(AndroidSchedulers.mainThread()) + .subscribe(ignored -> {/* Do nothing */}, this::onError); + } else if (playlistEntity != null) { + action = remotePlaylistManager.deletePlaylist(playlistEntity.getUid()) + .observeOn(AndroidSchedulers.mainThread()) + .doFinally(() -> playlistEntity = null) + .subscribe(ignored -> {/* Do nothing */}, this::onError); + } else { + action = Disposables.empty(); + } + + disposables.add(action); } - private void unbookmarkPlaylist() { - if (remotePlaylistManager == null || playlistEntity == null) return; + private void updateBookmarkButtons() { + if (playlistBookmarkButton == null || activity == null) return; - playlistBookmarkButton.setVisible(false); - playlistUnbookmarkButton.setVisible(false); + final int iconAttr = playlistEntity == null ? + R.attr.ic_playlist_add : R.attr.ic_playlist_check; - final Disposable disposable = remotePlaylistManager.deletePlaylist(playlistEntity.getUid()) - .observeOn(AndroidSchedulers.mainThread()) - .doFinally(() -> playlistEntity = null) - .subscribe(ignored -> {/* Do nothing */}, this::onError); - disposables.add(disposable); - } + final int titleRes = playlistEntity == null ? + R.string.bookmark_playlist : R.string.unbookmark_playlist; - private void updateBookmarkButtonsVisibility() { - if (playlistBookmarkButton == null || playlistUnbookmarkButton == null) return; - - playlistBookmarkButton.setVisible(playlistEntity == null); - playlistUnbookmarkButton.setVisible(playlistEntity != null); + playlistBookmarkButton.setIcon(ThemeHelper.resolveResourceIdFromAttr(activity, iconAttr)); + playlistBookmarkButton.setTitle(titleRes); } } diff --git a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BaseLocalListFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BaseLocalListFragment.java index 261f28d7c..d2c4e1b14 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BaseLocalListFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BaseLocalListFragment.java @@ -87,13 +87,6 @@ public abstract class BaseLocalListFragment extends BaseStateFragment // Lifecycle - Menu //////////////////////////////////////////////////////////////////////////*/ - /** Determines if the fragment is part of the main fragment view pager. - * If so, then this method must be overriden to return true - * in order to show the hamburger menu. */ - protected boolean isPartOfFrontPager() { - return false; - } - @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { super.onCreateOptionsMenu(menu, inflater); @@ -104,14 +97,6 @@ public abstract class BaseLocalListFragment extends BaseStateFragment if (supportActionBar == null) return; supportActionBar.setDisplayShowTitleEnabled(true); - - // Show up arrow icon if the fragment is not used as front page or part of the front pager - if (!useAsFrontPage && !isPartOfFrontPager()) { - // If set true, an up arrow icon will be displayed. - // If set false, no icon will be shown. - // If unset, show hamburger menu - supportActionBar.setDisplayHomeAsUpEnabled(true); - } } /*////////////////////////////////////////////////////////////////////////// diff --git a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java index 4166f462b..21aceade8 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/local/bookmark/BookmarkFragment.java @@ -147,15 +147,6 @@ public final class BookmarkFragment }); } - /*////////////////////////////////////////////////////////////////////////// - // Fragment Lifecycle - Menu - //////////////////////////////////////////////////////////////////////////*/ - - @Override - protected boolean isPartOfFrontPager() { - return true; - } - /////////////////////////////////////////////////////////////////////////// // Fragment LifeCycle - Loading /////////////////////////////////////////////////////////////////////////// diff --git a/app/src/main/res/menu/menu_playlist.xml b/app/src/main/res/menu/menu_playlist.xml index e0e7ebe18..4583ff719 100644 --- a/app/src/main/res/menu/menu_playlist.xml +++ b/app/src/main/res/menu/menu_playlist.xml @@ -18,15 +18,7 @@ android:id="@+id/menu_item_bookmark" android:icon="?attr/ic_playlist_add" android:title="@string/bookmark_playlist" - android:visible="false" - app:showAsAction="always" - tools:visible="true"/> - - \ No newline at end of file