Skip to content
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

Document that OpacityLayer's children are nonempty #8707

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

liyuqian
Copy link
Contributor

@dnfield
Copy link
Contributor

dnfield commented Apr 23, 2019

This LGTM, but I suspect we should be documenting this somewhere in the framework (e.g. on the Opacity widget, and probably on OpacityLayer) - that it should be avoided and if it's done the framework treats it as a no-op.

@liyuqian
Copy link
Contributor Author

liyuqian commented Apr 23, 2019

@dnfield : I tried Opacity(child: null) and the framework seems to treat that as a no-op even before my fix today. The difficulty of documenting Opacity is that it's hard to explain the number of children. For example, Opacity(child: Container(width: 1, height: 1)) is a no-op with 0 child layer, but Opacity(child: Container(width: 1, height: 1, color: Colors.blue)) has 1 child layer and hence is not a no-op.

Documenting OpacityLayer is a really good idea. It even discovers another bug flutter/flutter#31524 !

@liyuqian
Copy link
Contributor Author

https://docs.flutter.io/flutter/rendering/OpacityLayer/OpacityLayer.html is back and I'm adding docs to framework OpacityLayer in flutter/flutter#31520

@liyuqian liyuqian merged commit 7c8ec37 into flutter:master Apr 23, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 23, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 24, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 24, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 24, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 25, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Apr 25, 2019
flutter/engine@3e47b4b...26b30a4

git log 3e47b4b..26b30a4 --no-merges --oneline
26b30a4 Roll src/third_party/skia 9adc82c73df0..46d0f9aad1e6 (41 commits) (flutter/engine#8736)
fdd8fdb Fix reflective ctor invocation in FlutterFragment (flutter/engine#8735)
a56aa95 [scenic] Purge references to Mozart (flutter/engine#8712)
e4c439d Fix include paths in libtxt to prepare for upcoming Skia build change (flutter/engine#8723)
7c8ec37 Document that OpacityLayer's children are nonempty (flutter/engine#8707)
30fb4a6 Increase the memory usage estimate for EngineLayer (flutter/engine#8700)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff (liyuqian@google.com), and stop
the roller if necessary.
@liyuqian liyuqian deleted the opacity_doc branch May 23, 2019 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants