Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for lazy loading #177

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

bhaskarmelkani
Copy link

Adding support for lazy loading, this is already done in this fork of you branch.
I am using the same fork in lot may projects in production environments with and without lazy load.
Thought, to create a PR for it. :)

Dave Nagoda and others added 19 commits August 20, 2015 16:57
If a media query contains comments, the busting process is currently cancelled with a "Cannot read property 'property' of undefined" error.

For example:
@media only screen and (min-device-pixel-ratio: 2) and (min-resolution: 2dppx) {
    /** Just a comment */
    .large-image {
        background-image: url('assets/image1.jpg');
    }
}
Support both versions of including videos using standalone video tag and using video tag in combination with source tags.

Option 1
<video><source src="video.mp4" type="video/mp4" /></video>

Option 2
<video src="video.mp4"></video>
…h-comments-issue

Fix CSS processing for media queries with comments
…en_registering

Use passed in grunt when registering
'cachbust' => 'cachebust'
Support cache busting for meta tags
@benhoIIand
Copy link
Owner

Thanks for the PR. I'll have a look at merging this later. FYI, the next version of the plugin (currently the beta version on npm) will remove the need for adding filters to allow this to happen. You can take a look at what's changed in #147 or by changing to that branch and reading the README

@ArmorDarks
Copy link

I'm slightly confused — what exactly does it do?

@bhaskarmelkani
Copy link
Author

@ArmorDarks A lot of times we use lazyloading of images, some common approaches are like using lazy class or data-original. I added support so that we can cache bust these implementations as well.

@ArmorDarks
Copy link

@bhaskarmelkani Ah, I see. So how exactly does it work? I can't check the code, probably because of rebasing whole PR has been messed up and it's hard to read changes now...

So it just scans data attributes too?

@bhaskarmelkani
Copy link
Author

@ArmorDarks Yeah, but its been very long since I updated it. And it working perfectly fine some of my production systems.
May be I will rebase the PR.

@ArmorDarks
Copy link

May be I will rebase the PR.

I think it would be wise to do so, otherwise it's very hard to review it.

Yeah, but its been very long since I updated it. And it working perfectly fine some of my production systems.

I just wanted to know how exactly it finds which data attributes have to be processed too.

@bhaskarmelkani
Copy link
Author

@ArmorDarks It looks like the whole implementation is changed now, the support for filters is removed.

@ArmorDarks
Copy link

Yeap, as @HollandBen mentioned above.

Though, I think general idea of this PR is great, since it's indeed would be cool if lazyloaded images would be cachebusted too. However, maybe we should add config to state data attributes which should be cachebusted explicitly? Otherwise I have feeling that we might end up in awkward situation when something like data-example-image-name='image.jpg' will be cachebusted by accident too just because of similar name, while you wanted to put inside only example of name.

@bhaskarmelkani
Copy link
Author

Yeah makes sense, maybe I will update the new implementation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants