-
Notifications
You must be signed in to change notification settings - Fork 310
Migrate to Material 3 #225
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
Comments
I wonder if the "Add an account" button in the choose-account screen is really the normal UI idiom anyway, with or without the pastel-and-rounded look. Consider this change, which replaces it with an "add" button in the app bar, tooltipped "Add an account":
(I think having multiple code--- lib/widgets/app.dart
+++ lib/widgets/app.dart
@@ -60,7 +60,10 @@ class ChooseAccountPage extends StatelessWidget {
return Scaffold(
appBar: AppBar(
title: const Text('Choose account'),
- actions: const [ChooseAccountPageOverflowButton()]),
+ actions: const [
+ AddAccountButton(),
+ ChooseAccountPageOverflowButton(),
+ ]),
body: SafeArea(
minimum: const EdgeInsets.all(8),
child: Center(
@@ -72,16 +75,25 @@ class ChooseAccountPage extends StatelessWidget {
accountId: accountId,
title: Text(account.realmUrl.toString()),
subtitle: Text(account.email)),
- const SizedBox(height: 12),
- ElevatedButton(
- onPressed: () => Navigator.push(context,
- AddAccountPage.buildRoute()),
- child: const Text('Add an account')),
]))),
));
}
}
+class AddAccountButton extends StatelessWidget {
+ const AddAccountButton({super.key});
+
+ @override
+ Widget build(BuildContext context) {
+ return IconButton(
+ tooltip: 'Add an account',
+ icon: const Icon(Icons.add),
+ onPressed: () => Navigator.push(context,
+ AddAccountPage.buildRoute()),
+ );
+ }
+}
+
enum ChooseAccountPageOverflowMenuItem { aboutZulip }
class ChooseAccountPageOverflowButton extends StatelessWidget { |
Yeah, we may well want to change the design of that screen. (In the full redesign where we change the app's overall navigation structure, I think the screen will go away and its function will be served by other pieces of UI.) But I wouldn't want to take it in a direction that de-emphasizes the "add an account" option to that degree, because that's an important element in the onboarding flow. If it were up there in the app bar like that, I think a lot of people would try to figure out how to log into an account, and find themselves stuck. Even if we added something special for the case where you have zero logged-in accounts, to cover initial onboarding, I'd predict that we'd eventually hear people complain that the Zulip mobile app only supports using a single account — because they'd gone looking for how to add another one and couldn't find a way. I think the orthodox Material Design solution would probably be a floating action button. I'm not sure that would be an improvement on what we have, though. In any case, even if we were to replace the "Add an account" button with something else, there'd still be the button to navigate to each existing account. |
For these, and much else, I think the stakes are low even if we're not completely satisfied with M3's defaults. In these areas, we still have work ahead of us to implement Vlad's designs, and that's equally true for whichever starting point. FWIW, though, I think M3 defaults in these areas are fine. I would favor checking off or removing these items. 🙂 |
Yeah, those two were a jarring contrast with the status quo ante, but they've grown on me and I agree they're as fine a starting point as the old version. I'll check them off. Relatedly, on this item:
I guess I'll leave it because we still might in principle spot something. But it's not an independent item of its own to block on, just a placeholder for zero-or-more other things. So at this point the only thing between us and switching to Material 3 is that one item about the bottom app bar in the lightbox. (OK, and I edited that placeholder so it's no longer formatted as a checkbox.) |
Uh oh!
There was an error while loading. Please reload this page.
Flutter's Material implementation recently changed its default from Material 2 to Material 3:
ThemeData.useMaterial3
to true flutter/flutter#129724useMaterial3: true
by default flutter/flutter#127064Chat discussion here.
We should follow along, because M2 support will eventually be deprecated and removed. But first, we should fix things that currently look wrong in M3 mode.
So:
useMaterial3: false
again, temporarily -> design: Stay on Material 2 for now #231useMaterial3: false
and return to the new default of trueTODO(material-3)
sThe text was updated successfully, but these errors were encountered: