Skip to content

Lazy Images: Skip images with data-skip-lazy attribute#14539

Merged
kraftbj merged 1 commit intomasterfrom
update/lazy-images-skip-lazy-data-attr
Feb 13, 2020
Merged

Lazy Images: Skip images with data-skip-lazy attribute#14539
kraftbj merged 1 commit intomasterfrom
update/lazy-images-skip-lazy-data-attr

Conversation

@ebinnion
Copy link
Copy Markdown
Contributor

@ebinnion ebinnion commented Jan 31, 2020 •
edited
Loading

Per a request from a hosting partner and a community member, add the ability to skip lazy loading of images that have the data-skip-lazy attribute.

Changes proposed in this Pull Request:

  • Do not lazy load images that have data-skip-lazy attribute

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • This is a new feature for an existing module.

Testing instructions:

  • Checkout branch
  • Ensure Lazy Images is active
  • Create a post with an image
  • Modify image's HTML to include data-skip-lazy="1"
  • Publish post then load post
  • Check source of image, and ensure that it does not have the jetpack-lazy-image--handled class nor any other lazy attributes.

For example, there are two images in the source here. The top on was not lazy loaded but the bottom one was.

Screen Shot 2020-01-31 at 11 00 59 AM

Proposed changelog entry for your changes:

  • Update lazy images module to skip images with the data-skip-lazy attribute

@ebinnion ebinnion requested a review from a team January 31, 2020 16:21
@ebinnion ebinnion self-assigned this Jan 31, 2020
@ebinnion ebinnion changed the title Lazy Images: Skip images with data-skip-lazy attrib Lazy Images: Skip images with data-skip-lazy attribute Jan 31, 2020
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Jan 31, 2020 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: February 11, 2020.
Scheduled code freeze: February 4, 2020

Generated by 🚫 dangerJS against f4d0f59

@ebinnion ebinnion force-pushed the update/lazy-images-skip-lazy-data-attr branch from 9840d86 to f4d0f59 Compare January 31, 2020 16:26
@ebinnion ebinnion added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Jan 31, 2020
@ebinnion ebinnion added this to the 8.2 milestone Jan 31, 2020
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Jan 31, 2020

I'd have 2 questions about this:

  • Isn't the jetpack_lazy_images_skip_image_with_atttributes filter meant to do just that today?
  • We're already past the code freeze and in the Beta period for Jetpack 8.2; would it be acceptable to ship this in Jetpack 8.3 instead?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Needs Review This PR is ready for review. labels Jan 31, 2020
@ebinnion
Copy link
Copy Markdown
Contributor Author

ebinnion commented Jan 31, 2020 •
edited
Loading

Isn't the jetpack_lazy_images_skip_image_with_atttributes filter meant to do just that today?

That's a good question. Yes, users and third-party developers could use that filter to exclude images based on an attribute. But, the purpose of adding this attribute is to standardize how users and third-party theme/developers can exclude images from being lazily loaded.

Other plugins use some combination of the skip-lazy class or the data-skip-lazy attribute. So, the goal is for all plugins to support both.

Of note, there's a core marketing push for this around this as well.

would it be acceptable to ship this in Jetpack 8.3 instead?

Yes sir.

@ebinnion ebinnion modified the milestones: 8.2, 8.3 Jan 31, 2020
@jeherve jeherve added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jan 31, 2020
dereksmart
dereksmart approved these changes Feb 12, 2020
View reviewed changes
tests/php/modules/lazy-images/test_class.lazy-images.php
}

/**
* @dataProvider get_dont_process_images_with_skip_lazy_data_attribute_data
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart Feb 12, 2020

Choose a reason for hiding this comment

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

TIL how to use @dataProvider! Thanks for adding the tests.

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 12, 2020
@kraftbj kraftbj merged commit f583dfd into master Feb 13, 2020
@kraftbj kraftbj deleted the update/lazy-images-skip-lazy-data-attr branch February 13, 2020 05:32
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 13, 2020
jeherve added a commit that referenced this pull request Feb 25, 2020
@jeherve
Changelog: add #14539
cd9d295
jeherve added a commit that referenced this pull request Feb 25, 2020
@jeherve
Changelog: 8.3 running changelog (#14642)
8797bea 
* 8.3 release: changelog

* Changelog: add #14516

* Changelog: add #14574

* Bring in changes from 8.2.1 and 8.2.2

* Update stable version

* Bring in 8.2.3 changes

* Changelog: add #14714

* Changelog: add #14639

* Changelog: add #14678

* Changelog: add #14673

* Changelog: add #14687

* Changelog: add #14704

* Changelog: add #14702

* Changelog: add #14541

* Changelog: add #14657

* Changelog: add #14622

* Changelog: add #14582

* Changelog: add #14638

* Changelog: add #14633

* Changelog: add #14571

* Changelog: add #14592

* Changelog: add #14539

* Changelog: add #14514

* Changelog: add #14643

* Changelog: add #14494

* Changelog: add #13739

* Changelog: add #14707

* Changelog: add #14736

* Changelog: add #14706

* Changelog: add #14730

* Changelog: add #14685

* Changelog: add #14727

* Changelog: add #14711

* Changelog: add #14742

* Changelog: add #14746

* Changelog: add #14725

* Changelog: add #13999

* Changelog: add #14740

* Changelog: add #14759

* Changelog: add #14703

* Changelog: add #14753

* Changelog: add #14754

* Changelog: add #14645

* Cahngelog: add #14599
nicokaiser added a commit to nicokaiser/lazy-images-without-jetpack that referenced this pull request Mar 31, 2020
@nicokaiser
backport: Lazy Images: Skip images with data-skip-lazy attribute (Aut…
56ae989 
…omattic/jetpack#14539)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dereksmart dereksmart dereksmart approved these changes

Assignees

@ebinnion ebinnion

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Lazy Images

Projects

None yet

Milestone

8.3

Development

Successfully merging this pull request may close these issues.

6 participants

@ebinnion @jetpackbot @jeherve @dereksmart @kraftbj @matticbot