-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add runWidget to bootstrap a widget tree without a default View #141484
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
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.
In the future this doc will mention that on desktop platforms runApp will create a window by default into which the provided app is rendered. If developers want to control how that window is created, they can set renderIntoDefaultSurface to false and include their own Window widget (configured to their liking) in the provided app (examples from the comment below will be added here to the docs when that is ready).
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.
Some considerations to see what this API looks and feels like in various scenarios:
Backwards compatibility
Existing apps that just do runApp(Text('Hello World')) Need to continue to work. In other words, by default runApp needs to render the provided widget into some kind of a default view. For now, this will be the implicit view. In the future, on platforms that don't provide an implicit view runApp will create a new Window behind the scenes and render the provided widget into that.
runApp(MyApp());Multi-view scenarios
For multi view scenarios where the Runner provides multiple views to render into we want to allow developers to exactly specify what FlutterView specific parts are rendered into:
final FlutterView myView = ...; // Up to the app to fish this out of `PlatformDispatcher.views`.
runApp(
renderIntoDefaultSurface: false,
View(
view: myView,
child: MyApp(),
),
);There may of course also be multiple top level views:
final FlutterView myView = ...;
final FlutterView myOtherView = ...;
runApp(
renderIntoDefaultSurface: false,
ViewCollection(
views: <Widget>[
View(
view: myView,
child: MyWidget(),
),
View(
view: myOtherView,
child: MyOtherWidget(),
),
],
),
);Of course, more likely then not the view management is actually implemented inside some other Widget and the callside would just look like this:
runApp(
renderIntoDefaultSurface: false,
MyMultiViewApp(),
);Custom Window
On Desktop platforms, apps may want to be in charge of creating their own window to configure it to their liking. This can be done as follows:
final WindowController controller = WindowController( /* window config options go here */ );
runApp(
renderIntoDefaultSurface: false,
Window(
controller: controller,
child: MyApp(),
),
);Auxiliary Windows
No matter which of the options presented above is used to bootstrap the app, auxiliary windows (for tooltips, menus, etc) can be created. Setting renderIntoDefaultSurface to false is not required to have multiple windows. It just defines whether the root of the widget tree is rendered into the default view/window or not.
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.
Explicit request to bikeshed:
- Is
renderIntoDefaultSurfacethe best name for this parameter? Are there better ones? Ideally, we'd want to avoid any reference to view or window since this parameter will be used for both contexts (see above). - Instead of adding a parameter to
runApp, should we create a new top-level run method for this? Something likerunAppWithoutDefaultSurface(terrible name). What would be a great name for that?
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 apologize as I know I've asked this before but I've forgotten the discussion)
Could you remind me why we can't change runApp's behavior based off the existence of the implicit view? Something like:
- If the implicit view exists, we use the implicit view by default. The widget you provide must be a "render tree" widget.
- If the implicit view does not exist, the widget you provide must be a "non render tree" widget like
Window,View, etc... - We add an optional
allowImplicitViewparameter torunApp. If this isfalse, you must provide a "non render tree" widget. If this istrueornull, you get the behavior above. Other potential names for such a flag:useImplicitViewIfExists,defaultToImplicitView,ignoreImplicitView(double-negative, not ideal)
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.
That would break existing code when we switch the desktop embedders to not use the implicit view: Suddenly, code samples that do something like runApp(MyApp()); wouldn't work anymore because you'd be required to wrap that in a Window. Also, cross-platform development gets more complicated: If you want to run on platforms that use the implicit view and those that don't you now have to conditionally wrap your app widget in a Window for the platforms without implicit view and not do that for platforms with implicit view. I believe, even on platforms without an implicit view you should be able to just call runApp(MyApp()); and have a default window be created for you. Only when you realize that you actually want to customize that default window should you be required to do additional work of manually creating that window.
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.
Gotcha that make sense. I was envisioning that we would ask users to change their runner & Dart apps if they wanted to opt-out of the implicit view, but I completely agree with your point that this complicates cross-platform development.
What would be the behavior be if I provide runApp a View but forget to pass renderIntoDefaultSurface? Or what if I provide a View and renderIntoDefaultSurface: true?
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.
If you do something illegal like that you will get an error. The current one is a bit generic, but I think we should add these specific cases as suggestions to them. I'll do that in this PR once we have settled on a name for this.
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.
Offline, @loic-sharma came up with the suggestion of introducing a new method for this and naming it runWidget. This is a suggestion I really like. I had considered a new method, but couldn't find a great name for it. This, however, seems to be a very fitting name. runApp would then be re-implemented in terms of runWidget.
9aff8fb to
4da7136
Compare
|
FYI @ditman @jacobsimionato With this, its gonna be time to say good bye to copying the Edit: PR for MVP is goderbauer/mvp#20, to land after this change lands. |
gspencergoog
left a comment
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.
Co-authored-by: Greg Spencer <[email protected]>
| /// provided `app` widget is rendered into. It is up to the caller to include at | ||
| /// least one [View] widget in the provided `app` widget that will bootstrap a | ||
| /// render tree and define the [FlutterView] into which the provided widget is | ||
| /// rendered. Failure to include a [View] widget as an ancestor to all |
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.
One might misread this as "Only a single view can be the ancestor to all render object widgets".
What do you think of something like "[RenderObjectWidget]s without an ancestor [View] widget will result in an exception"?
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.
That phrasing is clearer. Updated.
| /// | ||
| /// See also: | ||
| /// | ||
| /// * [WidgetsBinding.attachRootWidget], which creates the root widget for the |
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.
Should we include runApp here as well? Or is our pattern to not repeat things that have already been mentioned in the docs?
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.
Also, should we add runWidget to runApp's "See also" section?
I don't feel strongly about either of these.
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.
Doesn't hurt to mention it there again. Will add.
|
@goderbauer thanks for letting me know, I'll update my couple of experiments once this lands! |
|
Failures were found in post submit that seem related to this change: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20framework_tests_misc/15405/overview Reverting to open the tree |
…iew" (#142339) Reverts #141484 Initiated by: eliasyishak This change reverts the following previous change: Original Description: The existing `runApp` bootstraps the widget tree and renders the provided widget into the default view (which is currently the implicit View from `PlatformDispatcher.implicitView` and - in the future - may be a default-created window). Apps, that want more control over the View they are rendered in, need a new way to bootstrap the widget tree: `runWidget`. It does not make any assumptions about the View the provided widget is rendered into. Instead, it is up to the caller to include a View widget in the provided widget tree that specifies where content should be rendered. In the future, this may enable developers to create a custom window for their app instead of relying on the default-created one.
|
Reland is #142344. |

The existing
runAppbootstraps the widget tree and renders the provided widget into the default view (which is currently the implicit View fromPlatformDispatcher.implicitViewand - in the future - may be a default-created window). Apps, that want more control over the View they are rendered in, need a new way to bootstrap the widget tree:runWidget. It does not make any assumptions about the View the provided widget is rendered into. Instead, it is up to the caller to include a View widget in the provided widget tree that specifies where content should be rendered. In the future, this may enable developers to create a custom window for their app instead of relying on the default-created one.