Skip to content

Commit

Permalink
fix(common): image placeholder not removed in OnPush component (#54515)
Browse files Browse the repository at this point in the history
Fixes that the placeholder wasn't being removed when an optimized image is placed in an `OnPush` component.

Fixes #54478.

PR Close #54515
  • Loading branch information
crisbeto authored and alxhub committed Feb 20, 2024
1 parent db35266 commit d34e329
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
ɵRuntimeError as RuntimeError,
ɵSafeValue as SafeValue,
ɵunwrapSafeValue as unwrapSafeValue,
ChangeDetectorRef,
} from '@angular/core';

import {RuntimeErrorCode} from '../../errors';
Expand Down Expand Up @@ -433,7 +434,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
}
}
if (this.placeholder) {
this.removePlaceholderOnLoad(this, this.imgElement, this.renderer);
this.removePlaceholderOnLoad(this.imgElement);
}
this.setHostAttributes();
}
Expand Down Expand Up @@ -648,22 +649,17 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
return Boolean(placeholderConfig.blur);
}

private removePlaceholderOnLoad(
dir: NgOptimizedImage,
img: HTMLImageElement,
renderer: Renderer2,
): void {
const removeLoadListenerFn = renderer.listen(img, 'load', () => {
private removePlaceholderOnLoad(img: HTMLImageElement): void {
const callback = () => {
const changeDetectorRef = this.injector.get(ChangeDetectorRef);
removeLoadListenerFn();
removeErrorListenerFn();
dir.placeholder = false;
});
this.placeholder = false;
changeDetectorRef.markForCheck();
};

const removeErrorListenerFn = renderer.listen(img, 'error', () => {
removeLoadListenerFn();
removeErrorListenerFn();
dir.placeholder = false;
});
const removeLoadListenerFn = this.renderer.listen(img, 'load', callback);
const removeErrorListenerFn = this.renderer.listen(img, 'error', callback);
}

/** @nodoc */
Expand Down
27 changes: 21 additions & 6 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {CommonModule, DOCUMENT, IMAGE_CONFIG, ImageConfig} from '@angular/common';
import {RuntimeErrorCode} from '@angular/common/src/errors';
import {PLATFORM_SERVER_ID} from '@angular/common/src/platform_id';
import {Component, PLATFORM_ID, Provider, Type} from '@angular/core';
import {ChangeDetectionStrategy, Component, PLATFORM_ID, Provider, Type} from '@angular/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {DomSanitizer, SafeResourceUrl} from '@angular/platform-browser';
import {expect} from '@angular/platform-browser/testing/src/matchers';
Expand Down Expand Up @@ -1078,7 +1078,6 @@ describe('Image directive', () => {
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
const styles = parseInlineStyles(img);
// Double quotes removed to account for different browser behavior.
expect(img.getAttribute('style')?.replace(/"/g, '').replace(/\s/g, '')).toBe(
`background-size:cover;background-position:50%50%;background-repeat:no-repeat;background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEU);filter:blur(${PLACEHOLDER_BLUR_AMOUNT}px);`,
Expand All @@ -1100,6 +1099,19 @@ describe('Image directive', () => {
);
});

it('should replace the placeholder with the actual image on load', () => {
setupTestingModule();
const template = '<img ngSrc="path/img.png" width="400" height="300" placeholder="true" />';
const fixture = createTestComponent(template, ChangeDetectionStrategy.OnPush);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(parseInlineStyles(img).has('background-image')).toBe(true);
img.dispatchEvent(new Event('load'));
fixture.detectChanges();
expect(parseInlineStyles(img).has('background-image')).toBe(false);
});

it('should use the placeholderResolution set in imageConfig', () => {
const imageConfig = {
placeholderResolution: 30,
Expand Down Expand Up @@ -2177,10 +2189,13 @@ function setUpModuleNoLoader() {
});
}

function createTestComponent(template: string): ComponentFixture<TestComponent> {
return TestBed.overrideComponent(TestComponent, {set: {template: template}}).createComponent(
TestComponent,
);
function createTestComponent(
template: string,
changeDetection = ChangeDetectionStrategy.OnPush,
): ComponentFixture<TestComponent> {
return TestBed.overrideComponent(TestComponent, {
set: {template, changeDetection},
}).createComponent(TestComponent);
}

function parseInlineStyles(img: HTMLImageElement): Map<string, string> {
Expand Down

0 comments on commit d34e329

Please sign in to comment.