-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
Various doc fixes #35548
Various doc fixes #35548
Conversation
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.
LGTM +/- nits below. :)
/// Since [Container] combines a number of other widgets each with their own | ||
/// layout behavior, [Container]'s layout behavior is somewhat complicated. | ||
/// | ||
/// tl;dr: [Container] tries, in order: to honor [alignment], to size itself to |
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.
Is tl;dr ok in API docs? Ref: #18542
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.
Fixed.
/// | ||
/// tl;dr: [Container] tries, in order: to honor [alignment], to size itself to | ||
/// the [child], to honor the `width`, `height`, and [constraints], to expand to | ||
/// fit the parent, to be as small as possible. |
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.
/// fit the parent, to be as small as possible. | |
/// fit the parent, and to be as small as possible. |
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.
The and seems to make it ambitious.
/// rules described above.) The [decoration] can implicitly increase the | ||
/// [padding] (e.g. borders in a [BoxDecoration] contribute to the [padding]); | ||
/// see [Decoration.padding]. | ||
/// |
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.
Would a diagram for any of these scenarios listed above be beneficial in illustrating the point? Not sure.
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.
I'll leave that for a follow-up :)
On cirrus it shows that all checks passed: https://cirrus-ci.com/build/5674139195539456 Not sure why Github is disagreeing. Submitting. |
Description
Takes the doc fixes from #33460 and applies them without making the analyzer mad. Also filed https://github.com/flutter/flutter/issues/35547 to allow "Sample Code" headings in the future and move the discussion around those to the issue.
/cc @Hixie @gspencergoog
Related Issues
None
Tests
I added the following tests:
None, doc only change.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?