Skip to content

Conversation

@goderbauer
Copy link
Member

@goderbauer goderbauer commented Jan 12, 2024

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.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jan 12, 2024
Copy link
Member Author

@goderbauer goderbauer Jan 12, 2024

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).

Copy link
Member Author

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.

Copy link
Member Author

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 renderIntoDefaultSurface the 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 like runAppWithoutDefaultSurface (terrible name). What would be a great name for that?

Copy link
Member

@loic-sharma loic-sharma Jan 12, 2024

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:

  1. If the implicit view exists, we use the implicit view by default. The widget you provide must be a "render tree" widget.
  2. If the implicit view does not exist, the widget you provide must be a "non render tree" widget like Window, View, etc...
  3. We add an optional allowImplicitView parameter to runApp. If this is false, you must provide a "non render tree" widget. If this is true or null, you get the behavior above. Other potential names for such a flag: useImplicitViewIfExists, defaultToImplicitView, ignoreImplicitView (double-negative, not ideal)

Copy link
Member Author

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.

Copy link
Member

@loic-sharma loic-sharma Jan 13, 2024

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?

Copy link
Member Author

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.

Copy link
Member Author

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.

@goderbauer goderbauer force-pushed the bikeshed branch 2 times, most recently from 9aff8fb to 4da7136 Compare January 18, 2024 21:45
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Jan 18, 2024
@goderbauer goderbauer changed the title Add option to disable default view to runApp Add runWidget to bootstrap a widget tree without a default View Jan 19, 2024
@goderbauer goderbauer marked this pull request as ready for review January 19, 2024 18:57
@goderbauer
Copy link
Member Author

goderbauer commented Jan 19, 2024

FYI @ditman @jacobsimionato With this, its gonna be time to say good bye to copying the runAppWithoutImplicitView around. Once this lands, I'll remove it from the MVP and we should also remove that from google3.

Edit: PR for MVP is goderbauer/mvp#20, to land after this change lands.

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

/// 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
Copy link
Member

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"?

Copy link
Member Author

@goderbauer goderbauer Jan 19, 2024

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
Copy link
Member

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?

Copy link
Member

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.

Copy link
Member Author

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.

@ditman
Copy link
Member

ditman commented Jan 26, 2024

@goderbauer thanks for letting me know, I'll update my couple of experiments once this lands!

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024
@auto-submit auto-submit bot merged commit 5b44596 into flutter:master Jan 26, 2024
@eliasyishak
Copy link
Contributor

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

@eliasyishak eliasyishak added the revert Autorevert PR (with "Reason for revert:" comment) label Jan 26, 2024
auto-submit bot pushed a commit that referenced this pull request Jan 26, 2024
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Jan 26, 2024
auto-submit bot added a commit that referenced this pull request Jan 26, 2024
…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.
@goderbauer goderbauer deleted the bikeshed branch January 26, 2024 21:45
@goderbauer
Copy link
Member Author

Reland is #142344.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: tests "flutter test", flutter_test, or one of our tests autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants