Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix 'Related' section lazy-load not working in some scenarios #4586

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

infinite-persistence
Copy link
Contributor

Issue

  1. Sometimes, the Related Section will load unnecessarily when a File Page is entered. This happens more to Image claims, although I see it happening on Videos. I think it's due to my slow connection causing the upper components to not fully inflate, so the Related Section will briefly appear on screen.
  2. In the Autoplay case, if the WaitUntilOnPage has already opened the gates previously, the next video's Related Section will be loaded regardless of scroll position.

Changes

See the commit messages for the walkthrough.

Comments

Using the debounce method doesn't fix the first issue 100%, but it covers most cases with a decent connection speed. In the future, I'm thinking of using a dummy placeholder component with typical Video height while waiting for Images to load.

injectedItem={!isAuthenticated && IS_WEB && <Ads type="video" small />}
empty={__('No related content found')}
/>
<WaitUntilOnPage lastUpdateDate={this.lastReset}>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this prop be added to the WaitUntilOnPage wrapper around CommentList?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, missed it. For some reason, I kept thinking that one is the Channel Comments.

There are cases where `WaitUntilOnPage` will incorrectly render, such as at the beginning if the upper components hasn't expanded to full size, so `WaitUntilOnPage` would be briefly visible.

Added a 300ms debounce to fix this, which would also improve scrolling performance a bit by doing less. Hopefully 300ms is enough for the upper components to inflate to full size.
…stead.

This allows `RecommendedContent` to render the Card but with an empty list, so that the area isn't totally blank while waiting for `WaitUntilOnPage` to debounce.
## Issue
In the `Autoplay` case, if the `WaitUntilOnPage` has already opened the gates previously, the next video's Related will be loaded regardless of scroll position.

## Changes
Add a `lastUpdateDate` prop to `WaitUntilOnPage` to allow the parent to reset the gating state.

I don't really like the `lastUpdateDate` prop since it's purpose is not intuitive. Is there a standard way to do a "one-time trigger" from the parent?
@neb-b neb-b merged commit 93d26a0 into master Jul 29, 2020
@neb-b neb-b deleted the ip-related-gating branch July 29, 2020 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants