Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Add WordPress guidance for iframes lazy-loading #3698

Merged
merged 7 commits into from
Sep 14, 2020

Conversation

JayHoltslander
Copy link
Contributor

@JayHoltslander JayHoltslander commented Aug 13, 2020

Add paragraph with bulk update solution for Wordpress users

next step: @kaycebasques to verify that we support this approach (pinged relevant people on 8/25)

Add paragraph with bulk update solution for Wordpress users
@JayHoltslander JayHoltslander requested a review from a team as a code owner August 13, 2020 05:17
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Contributor has not signed the CLA label Aug 13, 2020
@JayHoltslander
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes Contributor has signed the CLA and removed cla: no Contributor has not signed the CLA labels Aug 13, 2020
@netlify
Copy link

netlify bot commented Aug 13, 2020

Deploy preview for web-dev-staging ready!

Built with commit 7353107

https://deploy-preview-3698--web-dev-staging.netlify.app

@jpmedley jpmedley self-assigned this Aug 13, 2020
@jpmedley jpmedley requested a review from addyosmani August 13, 2020 16:27
@robdodson
Copy link
Contributor

@addyosmani are you ok reviewing this change?

@robdodson
Copy link
Contributor

See #3705 for an explanation on why this is failing. I think rebasing or pulling from master will fix it.

Copy link
Contributor

@kaycebasques kaycebasques left a comment

Choose a reason for hiding this comment

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

This is a wonderful little addition, thanks @JayHoltslander (you seem to have written technical docs before)

I'm going to get someone in to verify that we recommend this WP practice

src/site/content/en/blog/iframe-lazy-loading/index.md Outdated Show resolved Hide resolved
@kaycebasques kaycebasques changed the title Update index.md Add WordPress guidance for iframes lazy-loading Aug 14, 2020
JayHoltslander and others added 3 commits August 14, 2020 22:33
Co-authored-by: Kayce Basques <kayce@google.com>
Co-authored-by: Kayce Basques <kayce@google.com>
Co-authored-by: Kayce Basques <kayce@google.com>
Copy link
Contributor Author

@JayHoltslander JayHoltslander left a comment

Choose a reason for hiding this comment

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

Accept suggestions

Copy link
Contributor

@kaycebasques kaycebasques left a comment

Choose a reason for hiding this comment

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

Someone pointed out to me that we're not formatting WordPress correctly

(Tip: Use the Add suggestion to batch feature to batch-accept all of these small edits as one commit)

src/site/content/en/blog/iframe-lazy-loading/index.md Outdated Show resolved Hide resolved
src/site/content/en/blog/iframe-lazy-loading/index.md Outdated Show resolved Hide resolved
src/site/content/en/blog/iframe-lazy-loading/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. I added a reference to the WP core ticket where this is being worked on and updated the code snippet to cover for once that is built into core, to avoid this snippet from conflicting.

@addyosmani
Copy link
Member

Similarly, change mostly LGTM.

Kayce Basques and others added 2 commits September 14, 2020 12:05
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@kaycebasques kaycebasques merged commit 96d8c6e into GoogleChrome:master Sep 14, 2020
@kaycebasques kaycebasques added content update for issues that do not require new content (only for updates to existing content) eng - performance issues related to speed labels Sep 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Contributor has signed the CLA content update for issues that do not require new content (only for updates to existing content) eng - performance issues related to speed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants