From ff7cfe471531976e7b385818ab13bdaa55bb2181 Mon Sep 17 00:00:00 2001 From: litetex <40789489+litetex@users.noreply.github.com> Date: Sun, 16 Jan 2022 19:05:51 +0100 Subject: [PATCH] Reverted to loading behavior of #7638 and improved it The previous/reverted behavior caused unwanted data transmission: * Removed loading via handleResults/loadMoreItems-callback because the RecyclerView is apparently not immediately updated in the UI when the data is set which causes one load of data to much. --- .../fragments/list/BaseListFragment.java | 127 +++++++++--------- .../fragments/list/BaseListInfoFragment.java | 7 +- .../fragments/list/search/SearchFragment.java | 14 +- 3 files changed, 68 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java index d1685c5af..279d6d563 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListFragment.java @@ -312,14 +312,74 @@ public abstract class BaseListFragment extends BaseStateFragment }); itemsList.clearOnScrollListeners(); - itemsList.addOnScrollListener(new OnScrollBelowItemsListener() { + + /* + * Add initial scroll listener - which tries to load more items when not enough + * are in the view (not scrollable) and more are available. + * + * Note: This method only works because "This callback will also be called if visible + * item range changes after a layout calculation. In that case, dx and dy will be 0." + * - which might be unexpected because no actual scrolling occurs... + * + * This listener will be replaced by DefaultItemListOnScrolledDownListener when + * * the view was actually scrolled + * * the view is scrollable + * * No more items can be loaded + */ + itemsList.addOnScrollListener(new DefaultItemListOnScrolledDownListener() { @Override - public void onScrolledDown(final RecyclerView recyclerView) { - onScrollToBottom(); + public void onScrolled(final RecyclerView recyclerView, final int dx, final int dy) { + super.onScrolled(recyclerView, dx, dy); + + if (dy != 0) { + log("Vertical scroll occurred"); + + useNormalScrollListener(); + return; + } + if (isLoading.get()) { + log("Still loading data -> Skipping"); + return; + } + if (!hasMoreItems()) { + log("No more items to load"); + + useNormalScrollListener(); + return; + } + if (itemsList.canScrollVertically(1) + || itemsList.canScrollVertically(-1)) { + log("View is scrollable"); + + useNormalScrollListener(); + return; + } + + log("Loading more data"); + loadMoreItems(); + } + + private void useNormalScrollListener() { + log("Unregistering and using normal listener"); + itemsList.removeOnScrollListener(this); + itemsList.addOnScrollListener(new DefaultItemListOnScrolledDownListener()); + } + + private void log(final String msg) { + if (DEBUG) { + Log.d(TAG, "itemListInitScrollListener - " + msg); + } } }); } + class DefaultItemListOnScrolledDownListener extends OnScrollBelowItemsListener { + @Override + public void onScrolledDown(final RecyclerView recyclerView) { + onScrollToBottom(); + } + } + private void onStreamSelected(final StreamInfoItem selectedItem) { onItemSelected(selectedItem); NavigationHelper.openVideoDetailFragment(requireContext(), getFM(), @@ -407,66 +467,7 @@ public abstract class BaseListFragment extends BaseStateFragment // Load and handle //////////////////////////////////////////////////////////////////////////*/ - /** - * If more items are loadable and the itemList is not scrollable -> load more data. - *
- * Should be called once the initial items inside {@link #startLoading(boolean)} - * has been loaded and added to the {@link #itemsList}. - *
- * Otherwise the loading indicator is always shown but no data can be loaded - * because the view is not scrollable; see also #1974. - */ - protected void ifMoreItemsLoadableLoadUntilScrollable() { - ifMoreItemsLoadableLoadUntilScrollable(0); - } - - /** - * If more items are loadable and the itemList is not scrollable -> load more data. - * - * @param recursiveCallCount Amount of recursive calls that occurred - * @see #ifMoreItemsLoadableLoadUntilScrollable() - */ - protected void ifMoreItemsLoadableLoadUntilScrollable(final int recursiveCallCount) { - // Try to prevent malfunction / stackoverflow - if (recursiveCallCount > 100) { - Log.w(TAG, "loadEnoughInitialData - Too many recursive calls - Aborting"); - return; - } - if (!hasMoreItems()) { - if (DEBUG) { - Log.d(TAG, "loadEnoughInitialData - OK: No more items to load"); - } - return; - } - if (itemsList.canScrollVertically(1) - || itemsList.canScrollVertically(-1)) { - if (DEBUG) { - Log.d(TAG, "loadEnoughInitialData - OK: itemList is scrollable"); - } - return; - } - if (DEBUG) { - Log.d(TAG, "loadEnoughInitialData - View is not scrollable " - + "but it could load more items -> Loading more"); - } - loadMoreItems(() -> - ifMoreItemsLoadableLoadUntilScrollable(recursiveCallCount + 1)); - } - - /** - * Loads more items. - * @param initialDataLoadCallback - * Callback used in {@link #ifMoreItemsLoadableLoadUntilScrollable()}. - *
- * Execute it once the data was loaded and added to the {@link #itemsList}. - *
- * Might be null. - */ - protected abstract void loadMoreItems(@Nullable Runnable initialDataLoadCallback); - - protected void loadMoreItems() { - loadMoreItems(null); - } + protected abstract void loadMoreItems(); protected abstract boolean hasMoreItems(); diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java index 87f031c12..ebd586e35 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java @@ -6,7 +6,6 @@ import android.util.Log; import android.view.View; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import org.schabi.newpipe.error.ErrorInfo; import org.schabi.newpipe.error.UserAction; @@ -146,7 +145,6 @@ public abstract class BaseListInfoFragment currentInfo = result; currentNextPage = result.getNextPage(); handleResult(result); - ifMoreItemsLoadableLoadUntilScrollable(); }, throwable -> showError(new ErrorInfo(throwable, errorUserAction, "Start loading: " + url, serviceId))); @@ -162,7 +160,7 @@ public abstract class BaseListInfoFragment protected abstract Single loadMoreItemsLogic(); @Override - protected void loadMoreItems(@Nullable final Runnable initialDataLoadCallback) { + protected void loadMoreItems() { isLoading.set(true); if (currentWorker != null) { @@ -178,9 +176,6 @@ public abstract class BaseListInfoFragment .subscribe(infoItemsPage -> { isLoading.set(false); handleNextItems(infoItemsPage); - if (initialDataLoadCallback != null) { - initialDataLoadCallback.run(); - } }, (@NonNull Throwable throwable) -> dynamicallyShowErrorPanelOrSnackbar(new ErrorInfo(throwable, errorUserAction, "Loading more items: " + url, serviceId))); diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java index 35bb2c349..055c27733 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java @@ -868,15 +868,12 @@ public class SearchFragment extends BaseListFragment isLoading.set(false)) - .subscribe(result -> { - handleResult(result); - ifMoreItemsLoadableLoadUntilScrollable(); - }, this::onItemError); + .subscribe(this::handleResult, this::onItemError); } @Override - protected void loadMoreItems(@Nullable final Runnable initialDataLoadCallback) { + protected void loadMoreItems() { if (!Page.isValid(nextPage)) { return; } @@ -894,12 +891,7 @@ public class SearchFragment extends BaseListFragment isLoading.set(false)) - .subscribe(itemsPage -> { - handleNextItems(itemsPage); - if (initialDataLoadCallback != null) { - initialDataLoadCallback.run(); - } - }, this::onItemError); + .subscribe(this::handleNextItems, this::onItemError); } @Override