Skip to content

Conversation

@LongCatIsLooong
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong commented Mar 20, 2021

This reverts commit c0bcb4f. Diff 69fe39e

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 feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Mar 20, 2021
@google-cla google-cla bot added the cla: yes label Mar 20, 2021
@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review March 20, 2021 08:52
@Kavantix
Copy link
Contributor

Shouldn't the constraintsTransform also be used when computing the intrinsic sizes?

@LongCatIsLooong
Copy link
Contributor Author

LongCatIsLooong commented Mar 21, 2021

@Kavantix the new render object is merely a container and typically doesn't have opinions about the intrinsic size. It's possible for a particular transform to always return a tight BoxConstraints and potentially change the intrinsic size but I can't come up with a practical use case (usually there're better render objects for the job). Do you have an example where the new render object wants to modify its child's intrinsic dimensions?

@Kavantix
Copy link
Contributor

@LongCatIsLooong
Well I mention it because with the current version this widget would cause similar issues as constrainedbox currently (see this issue)

So yeah indeed it should still let the child determine the intrinsic size but when asking the child it should apply the constraints, just like Padding does and like ConstrainedBox should do but still doesn’t

@LongCatIsLooong
Copy link
Contributor Author

@Kavantix Ah I see. You're right the transform matters for flow layout. Let me run this against internal tests and see if anything fails.

Copy link
Contributor

@mdebbar mdebbar 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 fixing the analyzer errors!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants