Skip to content

Commit

Permalink
Reverted to loading behavior of TeamNewPipe#7638 and improved it
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
litetex committed Jan 16, 2022
1 parent 5e0384b commit 3aab187
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 80 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,74 @@ public void selected(final CommentsInfoItem selectedItem) {
});

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(),
Expand Down Expand Up @@ -407,66 +467,7 @@ public void onCreateOptionsMenu(@NonNull final Menu menu,
// Load and handle
//////////////////////////////////////////////////////////////////////////*/

/**
* If more items are loadable and the itemList is not scrollable -> load more data.
* <br/>
* Should be called once the initial items inside {@link #startLoading(boolean)}
* has been loaded and added to the {@link #itemsList}.
* <br/>
* 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()}.
* <br/>
* Execute it once the data was loaded and added to the {@link #itemsList}.
* <br/>
* Might be <code>null</code>.
*/
protected abstract void loadMoreItems(@Nullable Runnable initialDataLoadCallback);

protected void loadMoreItems() {
loadMoreItems(null);
}
protected abstract void loadMoreItems();

protected abstract boolean hasMoreItems();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import android.view.View;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import org.schabi.newpipe.error.ErrorInfo;
import org.schabi.newpipe.error.UserAction;
Expand Down Expand Up @@ -146,7 +145,6 @@ public void startLoading(final boolean forceLoad) {
currentInfo = result;
currentNextPage = result.getNextPage();
handleResult(result);
ifMoreItemsLoadableLoadUntilScrollable();
}, throwable ->
showError(new ErrorInfo(throwable, errorUserAction,
"Start loading: " + url, serviceId)));
Expand All @@ -162,7 +160,7 @@ public void startLoading(final boolean forceLoad) {
protected abstract Single<ListExtractor.InfoItemsPage> loadMoreItemsLogic();

@Override
protected void loadMoreItems(@Nullable final Runnable initialDataLoadCallback) {
protected void loadMoreItems() {
isLoading.set(true);

if (currentWorker != null) {
Expand All @@ -178,9 +176,6 @@ protected void loadMoreItems(@Nullable final Runnable initialDataLoadCallback) {
.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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -885,15 +885,12 @@ public void startLoading(final boolean forceLoad) {
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.doOnEvent((searchResult, throwable) -> 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;
}
Expand All @@ -911,12 +908,7 @@ protected void loadMoreItems(@Nullable final Runnable initialDataLoadCallback) {
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.doOnEvent((nextItemsResult, throwable) -> isLoading.set(false))
.subscribe(itemsPage -> {
handleNextItems(itemsPage);
if (initialDataLoadCallback != null) {
initialDataLoadCallback.run();
}
}, this::onItemError);
.subscribe(this::handleNextItems, this::onItemError);
}

@Override
Expand Down

0 comments on commit 3aab187

Please sign in to comment.