1573206 - DiscoveryStream lazy load card html

Status: VERIFIED FIXED

(Firefox :: New Tab Page, enhancement, P1)

Description

[Scott [:thecount] Downe]

If we use intersection Observer on some of the html cards, the below the fold cards don't need to render until the user scrolls.

I saw some benefits from doing this in the profiler, and also did some screen recordings and got pretty good results: https://docs.google.com/document/d/1VsvhJ3eLcRhpOYLb7AR9S58QIjD7jK3ZvpPOH5nOzoQ/edit

Comment 1

[Scott [:thecount] Downe]

[Tracking Requested - why for this release]: I think the code is simple enough and the wins big enough to consider this for uplift.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Doesn't sound like this is significant enough to warrant tracking for 69, but I'm still happy to consider an uplift request when the time comes if the patch isn't risky.

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/5249

Comment 4

[Scott [:thecount] Downe]

Attachment: Uplift 1573206 - A lazy render change

Comment 5

[Scott [:thecount] Downe]

Comment on attachment 9085298 [details]
Uplift 1573206 - A lazy render change

Beta/Release Uplift Approval Request

• User impact if declined: Performance increase on newtab.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: To test:

1. Go to newtab.
2. Inspect a card, and look for these: <div class="ds-card">
3. Scroll down a bit.

Expected: Initially, any cards above the fold should be <div class="ds-card"> and any below the fold should be <div class="ds-card placeholder"> and be empty (so they render fast) . As you scroll, they should update with their content.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Once verified in nightly, it's low risk.

The patch is pretty small, has tests, and the method used to do this is similar to another method we use before rendering images and has been working great.

• String changes made/needed: none

Comment 6

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/cf52300e04c9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9085298 [details]
Uplift 1573206 - A lazy render change

New Tab Page perf improvement. Approved for 69.0b15.

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/12f2b490f4c4

Comment 9

[Cristian Fogel [:cfogel]]

Confirmed lack of patch with 69.0b14.
Verified enhancement with 70.0a1 (2019-08-18) on Windows 10, macOS 10.13, Ubutnu 16.04.

Leaving qe+ flag up for the the beta15 build.

Comment 10

[Cristian Fogel [:cfogel]]

Looking at 69.0b15 the structure is with lists ( <li class="card-outer"> [...] </li> ) instead of the ds-card + div(s) one.

Comment 11

[Cristian Fogel [:cfogel]]

Marking as verified after changing the browser.newtabpage.activity-stream.discoverystream.enabled pref to true for beta.
The placeholder class and structure are in order.