Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

onLoadMoreComplete(null) call from noMoreLoad() cause infinite loop #291

Closed
o-kyrych opened this issue Feb 17, 2017 · 7 comments
Closed

Comments

@o-kyrych
Copy link

o-kyrych commented Feb 17, 2017

According to documentation of onLoadMoreComplete(@nullable List newItems)

When noMoreLoad OR onError OR onCancel, pass empty list or null to hide the progressItem.

If null or empty list is passed in onLoadMoreComplete() the last if clause is executed and noMoreLoad is called again.

v5.0.0-rc1

public void onLoadMoreComplete(@Nullable List<T> newItems, @IntRange(from = -1) long delay) {
		// 1. Calculate new items count
		int newItemsSize = newItems == null ? 0 : newItems.size();
		int totalItemCount = newItemsSize + getMainItemCount();
		// 2. Add any new items
		if (newItemsSize > 0) {
			if (DEBUG)
				Log.v(TAG, "onLoadMore     performing adding " + newItemsSize + " new items on Page=" + getEndlessCurrentPage());
			//TODO: 0 + headers for Endless Top Scrolling
			addItems(getGlobalPositionOf(mProgressItem), newItems);
		}
		// 3. Check if features are enabled and the limits have been reached
		if (mEndlessPageSize > 0 && newItemsSize < mEndlessPageSize || // Is feature enabled and Not enough items?
				mEndlessTargetCount > 0 && totalItemCount >= mEndlessTargetCount) { // Is feature enabled and Max limit has been reached?
			// Disable the EndlessScroll feature
			setEndlessProgressItem(null);
		}
		// 4. Remove the progressItem if needed
		if (delay > 0 && (newItemsSize == 0 || !isEndlessScrollEnabled())) {
			if (DEBUG)
				Log.v(TAG, "onLoadMore     enqueued removing progressItem (" + delay + "ms)");
			mHandler.sendEmptyMessageDelayed(LOAD_MORE_COMPLETE, delay);
		} else if (isEndlessScrollEnabled()) {
			hideProgressItem();
		}
		// 5. Reset the loading status
		endlessLoading = false;
		// 6. Eventually notify noMoreLoad
		if (newItemsSize == 0 || !isEndlessScrollEnabled()) {
			noMoreLoad(newItemsSize);
		}
	}
@davideas
Copy link
Owner

davideas commented Feb 17, 2017

@KyrychenkoOleksandr, the function adapter.onLoadMoreComplete(newItems); has to be called only inside onLoadMore() callback.
The callback noMoreLoad() is a termination of your business use case, see demoApp.

@o-kyrych
Copy link
Author

@davideas Thank you for your responce.
So

  • does it mean that you have a mistake in javadocs?
  • how progressItem can be hidden after termination of my business use case?

@davideas
Copy link
Owner

  • "noMoreLoad" in the javadoc i can fix it, was written before I added the real callback in the newer version.
  • progressItem is automatically hidden/deleted, also endless features is automatically disabled if limits are set and met.

@o-kyrych
Copy link
Author

o-kyrych commented Feb 17, 2017

@davideas
progressItem is not automatically hidden
inside void onLoadMoreComplete(@Nullable List<T> newItems, @IntRange(from = -1) long delay)

// 3. Check if features are enabled and the limits have been reached
		if (mEndlessPageSize > 0 && newItemsSize < mEndlessPageSize || // Is feature enabled and Not enough items?
				mEndlessTargetCount > 0 && totalItemCount >= mEndlessTargetCount) { // Is feature enabled and Max limit has been reached?
			// Disable the EndlessScroll feature
			setEndlessProgressItem(null);
		}

setEndlessProgressItem(null); is called. inside

public FlexibleAdapter setEndlessProgressItem(@Nullable T progressItem) {
		endlessScrollEnabled = progressItem != null;
		if (progressItem != null) {
			setEndlessScrollThreshold(mEndlessScrollThreshold);
			mProgressItem = progressItem;
			if (DEBUG) {
				Log.i(TAG, "Set progressItem=" + getClassName(progressItem));
				Log.i(TAG, "Enabled EndlessScrolling");
			}
		} else if (DEBUG) {
			Log.i(TAG, "Disabled EndlessScrolling");
		}
		return this;
	}

endlessScrollEnabled is set to false.
than inside void onLoadMoreComplete(@Nullable List<T> newItems, @IntRange(from = -1) long delay)

// 4. Remove the progressItem if needed
		if (delay > 0 && (newItemsSize == 0 || !isEndlessScrollEnabled())) {
			if (DEBUG)
				Log.v(TAG, "onLoadMore     enqueued removing progressItem (" + delay + "ms)");
			mHandler.sendEmptyMessageDelayed(LOAD_MORE_COMPLETE, delay);
		} else if (isEndlessScrollEnabled()) {
			hideProgressItem();
		}

hideProgressItem(); can't be called because isEndlessScrollEnabled() returns false.

@davideas
Copy link
Owner

For the automatic endless, progressItem and listener must be set with setEndlessScrollListener().
I tested it, everything should work.

@eemihauk
Copy link

eemihauk commented Feb 23, 2017

@davideas
The progressItem is not automatically hidden if there is no delay given to the onLoadMoreComplete() method.

In the demo app the automatic hiding seems to work because the onLoadMoreComplete() method has a delay of 5000L given to it in the FragmentEndlessScrolling example.

Here is the snippet from the onLoadMoreComplete() method of the FlexibleAdapter (from 5.0.0-rc1) that seems to cause the problem:

// 3. Check if features are enabled and the limits have been reached
if (mEndlessPageSize > 0 && newItemsSize < mEndlessPageSize || // Is feature enabled and Not enough items?
		mEndlessTargetCount > 0 && totalItemCount >= mEndlessTargetCount) { // Is feature enabled and Max limit has been reached?
	// Disable the EndlessScroll feature
	setEndlessProgressItem(null);
}
// 4. Remove the progressItem if needed
if (delay > 0 && (newItemsSize == 0 || !isEndlessScrollEnabled())) {
	if (DEBUG)
		Log.v(TAG, "onLoadMore     enqueued removing progressItem (" + delay + "ms)");
	mHandler.sendEmptyMessageDelayed(LOAD_MORE_COMPLETE, delay);
} else if (isEndlessScrollEnabled()) {
	hideProgressItem();
}

If endless scrolling is disabled and there is no delay, we do not end up calling hideProgressItem() directly or indirectly through the LOAD_MORE_COMPLETE message. This results in the progressItem to hang after the last item added to the adapter.

@davideas
Copy link
Owner

@eemihauk, removing the else condition should then work also when no items and no delay.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants