Skip to content

Conversation

ny927
Copy link
Contributor

@ny927 ny927 commented Sep 10, 2020

Description

Offstage didn't have sample code, so I added it.

Related Issues

Fixes #62737

Tests

N/A. Only the doc changed.

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.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

  • No, no existing tests failed, so this is not a breaking change.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 10, 2020
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@shihaohong shihaohong added the d: api docs Issues with https://api.flutter.dev/ label Sep 14, 2020
@shihaohong
Copy link
Contributor

Hi @ny927 and thanks for the contribution! I think we should try to take this a step further, since the sample code does not exactly show an example of measuring the dimensions of a widget without bringing it on screen. Maybe we can improve the example by adding something functional that demonstrates this behavior.

@ny927
Copy link
Contributor Author

ny927 commented Sep 19, 2020

@shihaohong Thank you for letting me know. I changed it to show the example of measuring the dimensions of a widget without bringing it on screen. Hope this one would be better.

@Piinks Piinks added d: examples Sample code and demos documentation labels Sep 22, 2020
@Piinks
Copy link
Contributor

Piinks commented Sep 22, 2020

Hi @ny927 can you take a look at the failing checks here? It looks like the sample has errors in it.

Comment on lines +2719 to +2730
/// Offstage(
/// offstage: !_offstage,
/// child: Column(
/// children: <Widget>[
/// RaisedButton(
/// child: const Text('Get Size Of The Invisible Logo'),
/// onPressed: () { _getInvisibleLogoSize(); },
/// ),
/// Text('${_invisibleLogoSize ?? ''}'),
/// ],
/// ),
/// ),
Copy link
Contributor

@shihaohong shihaohong Sep 23, 2020

Choose a reason for hiding this comment

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

This particular widget doesn't have to be offstage, does it? My assumption is that you're trying to demonstrate that we can get the size of FlutterLogo, even when it is offstage.

I think a better example could be something like so:

  • We have an Offstage widget, much like yours, containing a widget (FlutterLogo) of a certain size.
  • Have a button to toggle offstage, just to demonstrate what happens visually when offstage is set to true/false.
  • Have a text widget toggles to display the following:
if offstage is false, show 
"FlutterLogo widget is not currently offstage, 
so it is painted, available for hit testing, and painted."

if offstage is true, show 
"FlutterLogo widget is offstage. 
It is laid out in the widget tree, but occupies no space visually, 
cannot be hit tested, and is not painted. Yet, we are able to get its 
size as if it were painted: ${logoSize}"

Comment on lines +2676 to +2681
/// class OffstageLogo extends StatefulWidget {
/// @override
/// _OffstageLogoState createState() => _OffstageLogoState();
/// }
///
/// class _OffstageLogoState extends State<OffstageLogo> {
Copy link
Contributor

@shihaohong shihaohong Sep 23, 2020

Choose a reason for hiding this comment

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

This section isn't necessary (lines 2676 to 2681) because the template you're using already assumes a stateful widget (you can immediately start from the contents of your stateful widget).

See the logs here for details: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket.appspot.com/8868703876011722080/+/steps/prepare_environment/0/steps/Analyze/0/steps/analyze/0/stdout

@goderbauer
Copy link
Member

@ny927 Do you have plans to follow-up on this PR?

@goderbauer
Copy link
Member

(PR triage): I am going to close this one for now due to inactivity. Feel free to re-open when you find the time to work on it again.

@goderbauer goderbauer closed this Oct 7, 2020
@shihaohong shihaohong mentioned this pull request Oct 8, 2020
13 tasks
@shihaohong shihaohong mentioned this pull request Nov 2, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a sample code in documentation to demonstrate how to use Offstage widget to measure the dimensions.

5 participants