🧪 [Story performance] Lazy load share menu on desktop on-demand#37044
Closed
mszylkowski wants to merge 51 commits intoampproject:mainfrom
Closed
🧪 [Story performance] Lazy load share menu on desktop on-demand#37044mszylkowski wants to merge 51 commits intoampproject:mainfrom
mszylkowski wants to merge 51 commits intoampproject:mainfrom
Conversation
…'main' of github.com:ampproject/amphtml into removeShare
…l into removeShare
…l into removeShare
Contributor
Author
|
Changes already merged in other PRs, closing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The share menu adds 3.5kB to amp-story and is not useful for the first paint of the page, so removing it will speed up the download and parsing of the amp-story bundle. The share menu will be fetched when users click on the share icon if there is no native share, and given that the click happens after all assets were downloaded, the share menu will be downloaded faster.
We can remove or simplify the requests service from amp-story with this change, since it's not used for any core amp-story functionality anymore.
Changes:
AmpSocialSharenot live in the same file as theregisterElementdeclaration so that we can import just the class.amp-story-share-menu.jsthat builds the share menu for desktop or mobile when installed, and installsAmpSocialShare.amp-story-1.0.jsso we don't have to download the share menu on mobile browsers.amp-story.-1.0jswhich were causing the styles to show up 3 times on the bundle (now they are only onamp-story-share-menu-0.1.js)