-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(module:image): support lazy loading for image component #8157
base: master
Are you sure you want to change the base?
feat(module:image): support lazy loading for image component #8157
Conversation
This preview will be available after the AzureCI is passed. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8157 +/- ##
==========================================
- Coverage 91.49% 91.47% -0.03%
==========================================
Files 535 535
Lines 18427 18441 +14
Branches 2815 2819 +4
==========================================
+ Hits 16860 16868 +8
- Misses 1248 1252 +4
- Partials 319 321 +2 ☔ View full report in Codecov by Sentry. |
Should we consider using NgOptimizedImage also uses the same technique |
Unfortunately, this method is ineffective. The reason is that in I implement a manual approach to verify when the newly created img element appears in the viewport. I utilize |
Thanks for your contribution :) @ParsaArvanehPA This year we applied for some free licenses from Jetbrains(valid until Nov 14, 2024, it can be activated in All Products Pack of Jetbrains), please contact me if you need it, I've sent you a notification to your email! |
Hi Jason, Thank you so much for your wonderful message and your generous offer. I am very happy and honored to be part of the ng-zorro-antd open source community. I enjoy working on this project and I am glad that my code has been useful and helpful. I am very grateful for the license from JetBrains that you have applied for me. This is a very valuable and thoughtful gift, and I appreciate your kindness and generosity. I have registered an account on JetBrains with my email address, and I've sent you my account info via email. I look forward to using JetBrains products and continuing to support our ng-zorro-antd community. Thank you again for your support! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left some small nits :)
components/image/image.directive.ts
Outdated
@@ -45,6 +47,7 @@ export class NzImageDirective implements OnInit, OnChanges, OnDestroy { | |||
|
|||
@Input() nzSrc = ''; | |||
@Input() nzSrcset = ''; | |||
@Input() nzLoading: nzLoading = 'eager'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type nzLoading
can be removed.
-- @Input() nzLoading: nzLoading = 'eager';
++ @Input() nzLoading?: 'eager' | 'lazy';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
components/image/image.directive.ts
Outdated
|
||
private backLoadCompleteStateHandler(): void { | ||
this.status = 'normal'; | ||
this.getElement().nativeElement.loading = this.nzLoading; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This attribute loading
is not supported by chrome browsers lower than version 77, so it's better to not add it if nzLoadingType
is not set. (https://caniuse.com/?search=eager)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apologize for the delay.
This could be removed entirely, as setting the image element's lazy loading does not produce the desired outcome.
…component # Conflicts: # components/image/doc/index.en-US.md # components/image/doc/index.zh-CN.md # components/image/image.directive.ts
Greetings @simplejason, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, WDYT @HyperLife1119
this.getElement().nativeElement.src = this.nzSrc; | ||
this.getElement().nativeElement.srcset = this.nzSrcset; | ||
if (this.nzLoading === 'lazy') { | ||
const observer = new IntersectionObserver( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could trigger a memory leak, we need to call observer.disconnect()
in ngOnDestroy
. @ParsaArvanehPA
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently when you add
loading='lazy'
to an image, it doesn't work as intended. It loads the image like as the page is loaded.Issue Number: N/A
What is the new behavior?
Now if you add
nzLoading='lazy'
, the image is only loaded when image is in viewport.Does this PR introduce a breaking change?
Other information