Skip to content

Infinite Scroll: in auto scroll mode or on page load, don't run if there are no entries to load.#3553

Merged
gravityrail merged 2 commits intomasterfrom
update/infinite-scroll-last-batch
Jul 24, 2016
Merged

Infinite Scroll: in auto scroll mode or on page load, don't run if there are no entries to load.#3553
gravityrail merged 2 commits intomasterfrom
update/infinite-scroll-last-batch

Conversation

@eliorivero
Copy link
Copy Markdown
Contributor

Closes #3549.
Improvements to scroll behavior for better UX.

Changes proposed in this Pull Request:

  • in auto scroll mode, if there are no more posts to fetch, don't attempt to fetch posts one more time. This avoids the spinner quick show/hide, which is a bit confusing, since up to this point, after the spinner is shown and hidden, new posts load.
  • when there are fewer posts than the posts to load on each scroll, disable IS. Example: if there are only 3 published posts and IS is set to display 7 on each time, we don't need to run IS looking for posts since we know there aren't others to display. Like before, this avoid the spinner quick show/hide.
    For this, the ::is_last_batch method was rewritten to more reliably reflect if it's the last batch. There were cases where it failed, for example: when there were 15 posts and IS displayed 3 posts per page, the last ::is_last_batch compared 3 (posts returned from db) > 3 (posts per page), which was false, that is, not the last batch, so IS thought there were most posts to fetch. It ran one more time returning nothing, which is a confusing UX.
  • This commit also addresses an issue of a missing dependency: when there were 3 posts and IS displayed 3 posts per page, it didn't enqueued the main assets in ::action_template_redirect but the theme compatibility still required them. They are now registered separately from their enqueuing, which solves the dependency issue.

@eliorivero eliorivero added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Infinite Scroll [Status] In Progress labels Mar 15, 2016
@eliorivero eliorivero added this to the 3.10 milestone Mar 15, 2016
@eliorivero eliorivero force-pushed the update/infinite-scroll-last-batch branch 2 times, most recently from a574b5a to ff6c86c Compare March 15, 2016 20:53
@eliorivero eliorivero force-pushed the update/infinite-scroll-last-batch branch from ff6c86c to fc61443 Compare April 4, 2016 18:35
@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Apr 4, 2016
@samhotchkiss samhotchkiss modified the milestones: 3.10, 4.0.1 Apr 5, 2016
@jeherve jeherve modified the milestones: 4.0.1, 4.0.2, 4.0.3 Apr 21, 2016
@jeherve jeherve modified the milestones: 4.0.3, 4.0.4 May 11, 2016
@samhotchkiss samhotchkiss modified the milestones: 4.1, 4.0.4 May 27, 2016
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Jun 17, 2016

Looks like this one is in need of a rebase.

@jeherve jeherve modified the milestones: 4.2, 4.1 Jun 17, 2016
@eliorivero eliorivero force-pushed the update/infinite-scroll-last-batch branch from fc61443 to 61fb075 Compare July 19, 2016 18:57
@eliorivero
Copy link
Copy Markdown
Contributor Author

Rebased.

It's not necessary to check for responses of empty type the next time, since we disabled scroller by then.
…l at least once.

The ::is_last_batch method was rewritten to more reliable reflect if it's the last batch. There were cases where it failed, for example: when there were 15 posts and IS displayed 3 posts per page, the last ::is_last_batch compared 3 posts returned from db > 3 posts per page, which was false, and so IS thought there were most posts to fetch. It ran one more time but returned nothing, which is a confusing UX.
This commit also addresses an issue of a missing dependency: when there were 3 posts and IS displayed 3 posts per page, it didn't enqueued the assets in ::action_template_redirect but the theme compatibility still required it. They are now registered regardless of whether they're enqueued or not, which solves the dependency issue.
@gravityrail
Copy link
Copy Markdown
Contributor

Nice! :shipit:

@gravityrail gravityrail merged commit eabf696 into master Jul 24, 2016
@gravityrail gravityrail added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Jul 24, 2016
@gravityrail gravityrail deleted the update/infinite-scroll-last-batch branch July 24, 2016 20:01
@gravityrail
Copy link
Copy Markdown
Contributor

Merged to 4.2

gravityrail added a commit that referenced this pull request Jul 24, 2016
…batch

Infinite Scroll: in auto scroll mode or on page load, don't run if there are no entries to load.
jeherve added a commit that referenced this pull request Jul 25, 2016
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Infinite Scroll Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InfiniteScroll on scroll should use the lastbatch response

5 participants