Skip to content

Conversation

@tvolkert
Copy link
Contributor

This adds a new ResizeImage.policy property that controls how ResizeImage will interpret its width and height properties. The existing behavior is preserved via ResizeImagePolicy.exact (default), but there is now the option to use ResizeImagePolicy.fit, which satisfies the use case outlined in #118543.

The API doc assets were added in flutter/assets-for-api-docs#209

Fixes #118543

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Feb 21, 2023
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

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

LGTM with some doc nits

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice to be more specific about how the new aspect ratio is applied.

Maybe

output image will have the spedified width and height, with the aspect ratio of the new width and height regardless of whether it matches the intrinsic aspect ratio of the image's width and height.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

This may result in a different aspect ratio than the aspect ratio specified by the target width and height, if for example the height gets clamped downwards but the width does not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, consider mentioning that this is similar to [BoxFit.fill] with consideration for null values on width and height, so setting one of those to null would end up corresponding to [BoxFit.fitWidth] or [BoxFit.fitHeight].

Consider mentioning that the fit member is similar to [BoxFit.contain].

I'm not sure if that would confuse more people or just lead to asking why we don't use BoxFit here, but to me having that linkage helps.

This adds a new `ResizeImage.policy` property that controls how `ResizeImage`
will interpret its `width` and `height` properties. The existing behavior is
preserved via `ResizeImagePolicy.exact` (default), but there is now the option
to use `ResizeImagePolicy.fit`, which satisfies the use case outlined in
#118543.

The API doc assets were added in flutter/assets-for-api-docs#209

Fixes #118543
@tvolkert tvolkert added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2023
@auto-submit auto-submit bot merged commit 71c4570 into flutter:master Feb 23, 2023
@tvolkert tvolkert deleted the resize_image branch February 23, 2023 03:53
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to use ResizeImage effectively when you don't know the aspect ratio of the image ahead of time

3 participants