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

feat(common): Add fill mode to NgOptimizedImage #47738

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions aio/content/guide/image-directive.md
Expand Up @@ -81,6 +81,18 @@ providers: [

</code-example>

### Using `fill` mode

In cases where you want to have an image fill a containing element, you can use the `fill` attribute. This is often useful when you want to achieve a "background image" behavior, or when you don't know the exact width and height of your image.

When you add the `fill` attribute to your image, you do not need and should not include a `width` and `height`, as in this example:
Copy link
Contributor

Choose a reason for hiding this comment

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

(can be follow up PR): can we add something here about why width and height aren't necessary? e.g. that we usually require them to prevent layout shift but here it's unnecessary


<code-example format="typescript" language="typescript">

&lt;img ngSrc="cat.jpg" fill&gt;

</code-example>

### Adjusting image styling

Depending on the image's styling, adding `width` and `height` attributes may cause the image to render differently. `NgOptimizedImage` warns you if your image styling renders the image at a distorted aspect ratio.
Expand Down
5 changes: 4 additions & 1 deletion goldens/public-api/common/index.md
Expand Up @@ -549,6 +549,9 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
set disableOptimizedSrcset(value: string | boolean | undefined);
// (undocumented)
get disableOptimizedSrcset(): boolean;
set fill(value: string | boolean | undefined);
// (undocumented)
get fill(): boolean;
set height(value: string | number | undefined);
// (undocumented)
get height(): number | undefined;
Expand All @@ -571,7 +574,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
// (undocumented)
get width(): number | undefined;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<NgOptimizedImage, "img[ngSrc],img[rawSrc]", never, { "rawSrc": "rawSrc"; "ngSrc": "ngSrc"; "ngSrcset": "ngSrcset"; "sizes": "sizes"; "width": "width"; "height": "height"; "loading": "loading"; "priority": "priority"; "disableOptimizedSrcset": "disableOptimizedSrcset"; "src": "src"; "srcset": "srcset"; }, {}, never, never, true, never>;
static ɵdir: i0.ɵɵDirectiveDeclaration<NgOptimizedImage, "img[ngSrc],img[rawSrc]", never, { "rawSrc": "rawSrc"; "ngSrc": "ngSrc"; "ngSrcset": "ngSrcset"; "sizes": "sizes"; "width": "width"; "height": "height"; "loading": "loading"; "priority": "priority"; "disableOptimizedSrcset": "disableOptimizedSrcset"; "fill": "fill"; "src": "src"; "srcset": "srcset"; }, {}, never, never, true, never>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<NgOptimizedImage, never>;
}
Expand Down
Expand Up @@ -202,6 +202,12 @@ export const IMAGE_CONFIG = new InjectionToken<ImageConfig>(
@Directive({
standalone: true,
selector: 'img[ngSrc],img[rawSrc]',
host: {
'[style.position]': 'fill ? "absolute" : null',
'[style.width]': 'fill ? "100%" : null',
'[style.height]': 'fill ? "100%" : null',
'[style.inset]': 'fill ? "0px" : null'
}
})
export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
private imageLoader = inject(IMAGE_LOADER);
Expand Down Expand Up @@ -330,6 +336,19 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
}
private _disableOptimizedSrcset = false;

/**
* Sets the image to "fill mode," which eliminates the height/width requirement and adds
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo, let's fix it in a followup PR.

Suggested change
* Sets the image to "fill mode," which eliminates the height/width requirement and adds
* Sets the image to "fill mode", which eliminates the height/width requirement and adds

* styles such that the image fills its containing element.
*/
@Input()
set fill(value: string|boolean|undefined) {
this._fill = inputToBoolean(value);
}
get fill(): boolean {
return this._fill;
}
private _fill = false;

/**
* Value of the `src` attribute if set on the host `<img>` element.
* This input is exclusively read to assert that `src` is not set in conflict
Expand All @@ -356,7 +375,11 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
}
assertNotBase64Image(this);
assertNotBlobUrl(this);
assertNonEmptyWidthAndHeight(this);
if (this.fill) {
assertEmptyWidthAndHeight(this);
} else {
assertNonEmptyWidthAndHeight(this);
}
assertValidLoadingInput(this);
assertNoImageDistortion(this, this.imgElement, this.renderer);
if (!this.ngSrcset) {
Expand All @@ -383,8 +406,14 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
private setHostAttributes() {
// Must set width/height explicitly in case they are bound (in which case they will
// only be reflected and not found by the browser)
this.setHostAttribute('width', this.width!.toString());
this.setHostAttribute('height', this.height!.toString());
if (this.fill) {
if (!this.sizes) {
this.sizes = '100vw';
}
} else {
this.setHostAttribute('width', this.width!.toString());
this.setHostAttribute('height', this.height!.toString());
}
Comment on lines 407 to +416
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think this section would require some extra comments (and updates to existing ones):

  • the width/height comment can be updated to better reflect the logic
  • it's be great to add a comment on why we define sizes as 100vw (related question: what happens when the sizes are defined differently by a user?)
  • we could also add a quick comment above the code that sets width/height and explain why we don't do it for "fill" mode

We could do it in a followup PR as well.


this.setHostAttribute('loading', this.getLoadingBehavior());
this.setHostAttribute('fetchpriority', this.getFetchPriority());
Expand Down Expand Up @@ -805,6 +834,22 @@ function assertNonEmptyWidthAndHeight(dir: NgOptimizedImage) {
}
}

/**
* Verifies that width and height are not set. Used in fill mode, where those attributes don't make
* sense.
*/
function assertEmptyWidthAndHeight(dir: NgOptimizedImage) {
if (dir.width || dir.height) {
throw new RuntimeError(
RuntimeErrorCode.INVALID_INPUT,
`${
imgDirectiveDetails(
dir.ngSrc)} the attributes \`height\` and/or \`width\` are present ` +
`along with the \`fill\` attribute. Because \`fill\` mode causes an image to fill its containing ` +
`element, the size attributes have no effect and should be removed.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the size might be confused with the sizes input, so we can rephrase this a bit:

Suggested change
`element, the size attributes have no effect and should be removed.`);
`element, the width/height attributes have no effect and should be removed.`);

}
}

/**
* Verifies that the `loading` attribute is set to a valid input &
* is not used on priority images.
Expand Down
117 changes: 117 additions & 0 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -794,6 +794,123 @@ describe('Image directive', () => {
});
});

describe('fill mode', () => {
it('should allow unsized images in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" fill>';
expect(() => {
const fixture = createTestComponent(template);
fixture.detectChanges();
}).not.toThrow();
});
it('should throw if width is provided for fill mode image', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" width="500" fill>';
expect(() => {
const fixture = createTestComponent(template);
fixture.detectChanges();
})
.toThrowError(
'NG02952: The NgOptimizedImage directive (activated on an <img> element with the ' +
'`ngSrc="path/img.png"`) has detected that the attributes `height` and/or `width` ' +
'are present along with the `fill` attribute. Because `fill` mode causes an image ' +
'to fill its containing element, the size attributes have no effect and should be removed.');
});
it('should throw if height is provided for fill mode image', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" height="500" fill>';
expect(() => {
const fixture = createTestComponent(template);
fixture.detectChanges();
})
.toThrowError(
'NG02952: The NgOptimizedImage directive (activated on an <img> element with the ' +
'`ngSrc="path/img.png"`) has detected that the attributes `height` and/or `width` ' +
'are present along with the `fill` attribute. Because `fill` mode causes an image ' +
'to fill its containing element, the size attributes have no effect and should be removed.');
});
it('should apply appropriate styles in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" fill>';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('style')?.replace(/\s/g, ''))
.toBe('position:absolute;width:100%;height:100%;inset:0px;');
});
it('should augment existing styles in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" style="border-radius: 5px; padding: 10px" fill>';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('style')?.replace(/\s/g, ''))
.toBe(
'border-radius:5px;padding:10px;position:absolute;width:100%;height:100%;inset:0px;');
});
it('should not add fill styles if not in fill mode', () => {
setupTestingModule();

const template =
'<img ngSrc="path/img.png" width="400" height="300" style="position: relative; border-radius: 5px">';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('style')?.replace(/\s/g, ''))
.toBe('position:relative;border-radius:5px;');
});
it('should add default sizes value in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" fill>';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('sizes')).toBe('100vw');
});
it('should not overwrite sizes value in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" sizes="50vw" fill>';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('sizes')).toBe('50vw');
});
it('should cause responsive srcset to be generated in fill mode', () => {
setupTestingModule();

const template = '<img ngSrc="path/img.png" fill>';

const fixture = createTestComponent(template);
fixture.detectChanges();
const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('srcset'))
.toBe(
`${IMG_BASE_URL}/path/img.png 640w, ${IMG_BASE_URL}/path/img.png 750w, ${
IMG_BASE_URL}/path/img.png 828w, ` +
`${IMG_BASE_URL}/path/img.png 1080w, ${IMG_BASE_URL}/path/img.png 1200w, ${
IMG_BASE_URL}/path/img.png 1920w, ` +
`${IMG_BASE_URL}/path/img.png 2048w, ${IMG_BASE_URL}/path/img.png 3840w`);
});
});

describe('preconnect detector', () => {
const imageLoader = () => {
// We need something different from the `localhost` (as we don't want to produce
Expand Down