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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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); | ||||||
|
@@ -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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: typo, let's fix it in a followup PR.
Suggested change
|
||||||
* 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 | ||||||
|
@@ -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) { | ||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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):
We could do it in a followup PR as well. |
||||||
|
||||||
this.setHostAttribute('loading', this.getLoadingBehavior()); | ||||||
this.setHostAttribute('fetchpriority', this.getFetchPriority()); | ||||||
|
@@ -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.`); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: the
Suggested change
|
||||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* Verifies that the `loading` attribute is set to a valid input & | ||||||
* is not used on priority images. | ||||||
|
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.
(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