[Story performance] Not load all page attachment images when loading story

[mszylkowski]

Description

When loading a story with page attachments, all the images inside all page attachments (independently of distance) get loaded.

Reproduction Steps

Open https://story.merdeka.com/2378675/volume-14

Check the network tab:

It contains all the images from all the page attachments

Relevant Logs

No response

Browser(s) Affected

Chrome

OS(s) Affected

Mac OS, others

Device(s) Affected

No response

AMP Version Affected

latest

[gmajoulet]

Content should not be built nor loaded until the page attachment is open by a user.

Our code claims ownership over the components here:

amphtml/extensions/amp-story/1.0/amp-story-draggable-drawer.js

Lines 189 to 201 in a62b451

	const walker = this.win.document.createTreeWalker(
	this.element,
	NodeFilter.SHOW_ELEMENT,
	null /** filter */,
	false /** entityReferenceExpansion */
	);
	while (walker.nextNode()) {
	const el = dev().assertElement(walker.currentNode);
	if (isAmpElement(el)) {
	this.ampComponents_.push(el);
	Services.ownersForDoc(this.element).setOwner(el, this.element);
	}
	}

And only get built in the open method here:

amphtml/extensions/amp-story/1.0/amp-story-draggable-drawer.js

Lines 589 to 591 in a62b451

	const owners = Services.ownersForDoc(this.element);
	owners.scheduleLayout(this.element, this.ampComponents_);
	owners.scheduleResume(this.element, this.ampComponents_);

we also have a few toggle(container, bool) to ensure that the content is hidden - thus not built - until it's needed.

[gmajoulet]

cc @ampproject/wg-stories

[processprocess]

This is working as expected. The attachment builds when the page builds.
https://user-images.githubusercontent.com/3860311/128731327-36ea1513-c4b7-49de-9d7a-7f4bb945ce98.mp4

And only get built in the open method

We can't build this in open because the user can swipe up to preview the attachment before open is called.

If we want to do this for LCP reasons, I wonder how we can defer loading until the page content is loaded.

[mszylkowski]

I think it should only get built (the content, not necessarily the component buildCallback - eg by keeping it display:none -) either when the page for the attachment is loaded, or when the user swipes up (starts the interaction). If users start to interact with the story, the LCP stops being recorded (which is good), while loading the page attachment after the page is loaded will ensure the active page takes precedence. Story pages have CustomEvent.LOADED events dispatched that we can listen to. How does that sound?

[processprocess]

I want to confirm that this is a problem and not an edge case we're fixing for the example story.

Would implementing this significantly improve user experience and LCP?

[mszylkowski]

With the publisher https://story.merdeka.com/ seems like most of the stories contain around 50 images and get loaded, but I've checked other stories with page attachments and it wasn't consistent whether the images were loaded from the beginning or not. At least with that publisher (I lab tested with that story) there was an 18% gain in PX from not loading the page attachments from the beginning. Sth like this won't improve all stories, only the ones with heavy page attachments (we can use this info to prioritize this fix).

[gmajoulet]

Regardless of LCP issues, it's an overall performance / UX / waste of user resources issue. We're using way too much bandwidth and CPU before it's actually needed. Potentially prerendering the content of page attachments even.

Your point is valid @processprocess, but in this case I'd argue we should bias our implementation towards performance, and only build/layout/load resources when the page attachment opens. It's ok if it's empty as we swipe up, till we release it, that's how it was designed back then before this regression happens.