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

Material App is complicated/confusing #140762

Open
caseycrogers opened this issue Dec 31, 2023 · 7 comments
Open

Material App is complicated/confusing #140762

caseycrogers opened this issue Dec 31, 2023 · 7 comments
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team

Comments

@caseycrogers
Copy link
Contributor

caseycrogers commented Dec 31, 2023

Use case

MaterialApp wraps the tree in a navigator/router (transitively via WidgetsApp). Since there are so many different approaches to navigation now (Nav 1, Nav 2, GoRouter, Nav 1 router based, etc) this massively balloons MaterialApp's constructor. Not to mention, MaterialApp breaks Nav 2 on Android (system back closes the entire app) and requires a workaround to use.

Here is a list of navigation related constructor arguments in MaterialApp:

  1. navigatorKey
  2. home - this would probably just be replaced by child if nav logic were removed
  3. initialRoute
  4. onGenerateRoute
  5. onGenerateInitialRoutes
  6. onNavigationNotification
  7. navigatorObservers

Then there's the entire MaterialApp.router alternate constructor.

Many of these args have non-obvious runtime constraints too:

/// At least one of [home], [routes], [onGenerateRoute], or [builder] must be

And, since most of these arguments are just proxies for WidgetApps/Router/Navigator, they're a maintenance burden to keep in sync, have unreadable inline docs (may depend on IDE settings), and their logic is hard to trace when clicking through the source code:
image

Proposal

Handling navigation logic in MaterialApp made sense when Flutter's navigation options were simple and limited. We are well past that point.

Here are a couple proposals of various levels of disruption that would resolve this problem. Note that all of these proposals probably involve modifying WidgetsApp to mirror the MaterialApp changes

  1. Allow the developer to provide a navigator key corresponding to their own navigator, MaterialApp will bind the system back gesture to the navigator associated with that key, and provide no other navigation logic of its own. Pros: no disruption, minimal boilerplate. Cons: MaterialApp becomes more complicated instead of less, would be hard to disambiguate this new arg from the existing navigatorKey arg.
  2. Alternate constructor that has no navigation based args, developer is responsible for putting a navigator in the tree and binding the system back gesture to it. Pros: no disruption, no subtle behavior. Cons: MaterialApp remains complicated, developer has to write a fair amount of boilerplate to opt out of material app nav.
  3. Remove all navigation related logic from MaterialApp. Pros: simplifies dramatically. Cons: high disruption, newbies who would've been happy with basic default behavior now have to write a lot of boilerplate (PopHandler, Navigator).

3 feels very "pure" but is quite disruptive. 1 feels a little sloppy/awkward. 2 feels like the sweet spot as it both avoids disruption while providing an elegant escape hatch.

Happy to hear your all's thoughts and am more than willing to write the associated PR(s) myself to make any of these 3 happen.

@caseycrogers caseycrogers changed the title Remove Navigator From MaterialApp Allow Devs to use MaterialApp Without it's Nav Logic Dec 31, 2023
@matanlurey
Copy link
Contributor

I wonder if there is a way to have the existing constructor more or less work the same by implicitly wrapping and we create a new constructor without any navigation logic, basically option 3 without a breaking change:

class MaterialApp {
  factory MaterialApp(/*existing args*/) {
    return _ImplicitNavMaterialApp(...);
  }

  factory MaterialApp.withoutNavigation(/**/) {
    // TODO: Implement.
  }
}

Based on reception/feedback we could either rename constructors in the future, or even have some sort of auto-fix that inlines the default constructor for users.

@samithe7
Copy link

i use go_router for navigation . how can i use go_router with proposals 3 !!!!

@caseycrogers
Copy link
Contributor Author

caseycrogers commented Dec 31, 2023

@samithe7 I haven't used GoRouter at all so I'm not familiar with it-perhaps someone else could pipe in on what that would look like? Though I'm increasingly thinking option 3 isn't a great approach anyway and I doubt the Flutter team would go for it so maybe it's a moot point?

FWIW It occurs to me the problem here might be one more of API discovery than anything else.
The problem isn't so much that MaterialApp is intercepting the system back as much as it is that if I want my navigator to respond to the system back gesture, something has to bind it to my navigator.

So right now the best workaround (which I am already using) I'd still have to use under proposals 2 and 3. This makes option 1 sound a lot more appealing.

Here's "option 2b" with this in mind:
2b. MaterialApp has an alternate constructor with the existing nav related arguments removed. It has a new VoidcallBack onSystemBack argument that it routes to a NavigationPopHandler.

I like that this approach gives the developer arbitrary navigation flexibility while guiding them through the process of binding their custom solution to the system back gesture. But it's still a little awkward. Namely, MaterialApp.onSystemBack will be above my Navigator in the widget tree but it'll need to access the navigator so I'll have to use a global key to get around this.

@danagbemava-nc danagbemava-nc added in triage Presently being triaged by the triage team framework flutter/packages/flutter repository. See also f: labels. f: routes Navigator, Router, and related APIs. customer: crowd Affects or could affect many people, though not necessarily a specific customer. c: proposal A detailed proposal for a change to Flutter team-framework Owned by Framework team and removed in triage Presently being triaged by the triage team labels Jan 2, 2024
@goderbauer
Copy link
Member

To answer the request made in the title of this issue: It is already today possible to use MaterialApp without the nav logic by specifying the builder argument, see https://main-api.flutter.dev/flutter/material/MaterialApp/builder.html. Of course, this doesn't address the other issue of MaterialApp being to complex...

@caseycrogers
Copy link
Contributor Author

caseycrogers commented Jan 2, 2024

To answer the request made in the title of this issue: It is already today possible to use MaterialApp without the nav logic by specifying the builder argument...

Then it looks like the problem I have then is much more about the interpretability of MaterialApp rather than it's features or lack thereof. I think there might be a cascading series of UX issues that I'm having trouble with here.
I had checked the Builder argument before and skipped over it, maybe running through my thought process when exploring material App's API would be helpful:
image

  1. TransitionBuilder feels like a misnomer-it implies to me that this is for animating child widgets in and out. This was the primary reason I skimmed over MaterialApp.builder without realizing that it provides what I want. Perhaps a name change or equivalent typedef with an alternate name (similar to AnimatedBuilder/ListenableBuilder) could be helpful here?
  2. The @Macro makes the inline documentation hard to follow, I usually just give up and move on when faced with @macro docs. Is there an IDE plugin/setting that can at the very least provide functional click through here?
  3. The WidgetsApp.builder docs are pretty complicated and hard to follow. After reading it, I still don't really understand it. The hardest part is it seems to have a couple fairly unrelated purposes (overriding navigator behavior, overriding MediaQuery, etc).

All this considered, maybe this PR was pretty narrowly scoped in the end and my topline issue is more "Material App is complicated/confusing" than specifically the interaction of MaterialApp <> Nav. Is there any appetite for a broad design refresh here? IMO it's worth it-especially considering that this is the first widget most new flutter devs will interact with-but it feels like cleaning up the dev ex here will be pretty hairy and takes a bit more of a holistic perspective than I've had so far.

@goderbauer goderbauer changed the title Allow Devs to use MaterialApp Without it's Nav Logic Material App is complicated/confusing Jan 2, 2024
@goderbauer goderbauer added team-design Owned by Design Languages team and removed team-framework Owned by Framework team labels Jan 2, 2024
@giannissterg
Copy link

giannissterg commented Jan 3, 2024

I would like to provide further insight from my experience. As was already mentioned and I agree, both MaterialApp and WidgetsApp are too complicated and have too many responsibilities. From that point of view, builder argument seems more like a workaround around an inflexible api rather than a solution to the problem.

I think Navigation should be opt in and a responsibility of the consumer of the API. What if I don't want a navigator at all and I just want a single page application? What if I want nested navigators? Most of the times navigation is gradual, meaning that you start with a simple navigation flow, consisting of a couple of pages. Then you realize you want parallel navigation histories, where you introduce nested navigators with the top one orchestrating which one is active. After that, you may want to add a loading or a sign in flow before the nested navigator. To conclude, I think navigation should be a separate issue not a responsibility of the top level widget. This would unlock many possibilities of independent evolution.

It is the same as the implicit/explicit view in a multiview app, that is coming soon. Implicit is good for faster development, but explicitly manifesting the view hierarchy is a must for more advanced usecases.

@HansMuller HansMuller added P2 Important issues not at the top of the work list triaged-design Triaged by Design Languages team labels Jan 5, 2024
@Cierra-Runis
Copy link

Cierra-Runis commented Feb 22, 2024

@caseycrogers In my application, I disabled title bar of window and use builder to customize it.

Widget builder(BuildContext context, Widget? child) {
  return Column(
    children: [
      if (Platform.isWindows || Platform.isMacOS) const WindowAppBar(),
      Expanded(child: ClipRRect(child: child)),
    ],
  );
}

It's a good example to explain what builder does here.

Colunm(
  children: [
    if (Platform.isWindows || Platform.isMacOS) const WindowAppBar(),
    Expanded(child: ClipRRect(child: MaterialApp(...))),
  ],
);

Instead, using the example code as above, WindowAppBar can't get variable that Material specific features need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
None yet
Development

No branches or pull requests

8 participants