Skip to content

Improve Documentation for ResizeImage Dimensions and Usage #154212

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

Merged
merged 10 commits into from
Sep 3, 2024

Conversation

RamonFarizel
Copy link
Contributor

@RamonFarizel RamonFarizel commented Aug 27, 2024

This pull request enhances the documentation for the ResizeImage widget to provide clearer guidance on the use of the width and height parameters.

Fixes #136508

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 27, 2024
@goderbauer goderbauer requested review from nate-thegrate and removed request for navaronbracke August 28, 2024 22:11
Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I sort of crammed a bunch of feedback into this tiny addition, so take a look when you have a chance :)

Copy link
Contributor

@nate-thegrate nate-thegrate 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 optional whitespace nit)

Thanks for making the improvement here!

@RamonFarizel
Copy link
Contributor Author

Thank you all for your support! It was nice to learn a bit more about all those details.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

@hwh97
Copy link

hwh97 commented Oct 25, 2024

@RamonFarizel @nate-thegrate Hey, Can someone confirm whether the width parameter in ResizeImage refers to logical pixels instead of physical pixels? The original issue seems to describe it differently. Any clarification would be appreciated!

@nate-thegrate
Copy link
Contributor

nate-thegrate commented Oct 25, 2024

Thank you @hwh97, this is a really good question.

If you set width: 200, the image will be 200 physical pixels wide and will also size itself to a width of 200 logical pixels if unconstrained.

This means that if the image is constrained, the width parameter will only change the width with regards to image resolution.

I apologize for the confusion—originally my feedback on this PR was focused on syntax/grammar/style guidelines and I neglected to double-check that it matched the behavior. I'll go ahead and re-open #136508.

auto-submit bot pushed a commit that referenced this pull request Oct 30, 2024
Whoever reviewed the documentation changes in #154212 neglected to double-check that the information was accurate (it was me who did this).

Fixes #136508
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.

ResizeImage should make clear in its documentation that the width represents the physical pixels and not the logical pixels
5 participants