-
Notifications
You must be signed in to change notification settings - Fork 350
Show current organization's name and logo atop main menu #1937
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
87ddd72
c40a8d9
83f3f9e
c9c67d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import 'app.dart'; | |
| import 'app_bar.dart'; | ||
| import 'button.dart'; | ||
| import 'color.dart'; | ||
| import 'content.dart'; | ||
| import 'icons.dart'; | ||
| import 'inbox.dart'; | ||
| import 'inset_shadow.dart'; | ||
|
|
@@ -279,7 +280,6 @@ void _showMainMenu(BuildContext context, { | |
| _DirectMessagesButton(tabNotifier: tabNotifier), | ||
| // TODO(#1094): Users | ||
| const _MyProfileButton(), | ||
| const _SwitchAccountButton(), | ||
| // TODO(#198): Set my status | ||
| // const SizedBox(height: 8), | ||
| const _SettingsButton(), | ||
|
|
@@ -307,29 +307,113 @@ void _showMainMenu(BuildContext context, { | |
| builder: (BuildContext _) { | ||
| return PerAccountStoreWidget( | ||
| accountId: accountId, | ||
| child: SafeArea( | ||
| minimum: const EdgeInsets.only(bottom: 8), | ||
| child: Column( | ||
| crossAxisAlignment: CrossAxisAlignment.stretch, | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| Flexible(child: InsetShadowBox( | ||
| top: 8, bottom: 8, | ||
| color: designVariables.bgBotBar, | ||
| child: SingleChildScrollView( | ||
| padding: const EdgeInsets.symmetric(vertical: 8, horizontal: 8), | ||
| child: Column(children: menuItems)))), | ||
| const Padding( | ||
| padding: EdgeInsets.symmetric(horizontal: 16), | ||
| child: AnimatedScaleOnTap( | ||
| scaleEnd: 0.95, | ||
| duration: Duration(milliseconds: 100), | ||
| child: BottomSheetDismissButton( | ||
| style: BottomSheetDismissButtonStyle.close))), | ||
| ]))); | ||
| child: _MainMenu(menuItems: menuItems)); | ||
| }); | ||
| } | ||
|
|
||
| /// The main-menu sheet. | ||
| /// | ||
| /// Figma link: | ||
| /// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=143-10939&t=s7AS3nEgNgjyqHck-4 | ||
| class _MainMenu extends StatelessWidget { | ||
| const _MainMenu({ | ||
| required this.menuItems, | ||
| }); | ||
|
|
||
| final List<Widget> menuItems; | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final designVariables = DesignVariables.of(context); | ||
|
|
||
| return SafeArea( | ||
| minimum: const EdgeInsets.only(bottom: 8), | ||
| child: Column( | ||
| crossAxisAlignment: CrossAxisAlignment.stretch, | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| _MainMenuHeader(), | ||
| Flexible(child: InsetShadowBox( | ||
| top: 8, bottom: 8, | ||
| color: designVariables.bgBotBar, | ||
| child: SingleChildScrollView( | ||
| padding: const EdgeInsets.symmetric(vertical: 8, horizontal: 8), | ||
| child: Column(children: menuItems)))), | ||
| const Padding( | ||
| padding: EdgeInsets.symmetric(horizontal: 16), | ||
| child: AnimatedScaleOnTap( | ||
| scaleEnd: 0.95, | ||
| duration: Duration(milliseconds: 100), | ||
| child: BottomSheetDismissButton( | ||
| style: BottomSheetDismissButtonStyle.close))), | ||
| ])); | ||
| } | ||
| } | ||
|
|
||
| class _MainMenuHeader extends StatelessWidget { | ||
| const _MainMenuHeader(); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final designVariables = DesignVariables.of(context); | ||
| final store = PerAccountStoreWidget.of(context); | ||
|
|
||
| final realmIconUrl = store.resolveRealmIconUrl(); | ||
|
|
||
| final placeholder = ColoredBox(color: designVariables.avatarPlaceholderBg); | ||
| final logo = realmIconUrl != null | ||
| ? RealmContentNetworkImage( | ||
| realmIconUrl, | ||
| frameBuilder: (_, child, frame, _) { | ||
| if (frame == null) return placeholder; | ||
| return child; | ||
| }, | ||
| errorBuilder: (_, _, _) => placeholder) | ||
| : placeholder; | ||
|
|
||
| return Padding( | ||
| padding: const EdgeInsets.only(top: 6), | ||
| child: AnimatedScaleOnTap( | ||
| duration: const Duration(milliseconds: 100), | ||
| scaleEnd: 0.95, | ||
| child: TextButton( | ||
| onPressed: () { | ||
| Navigator.pop(context); // Close the main menu. | ||
| Navigator.push(context, | ||
| MaterialWidgetRoute(page: const ChooseAccountPage())); | ||
| }, | ||
| style: TextButton.styleFrom( | ||
| padding: EdgeInsets.zero, | ||
| shape: LinearBorder.none, | ||
| backgroundColor: Colors.transparent, | ||
| overlayColor: Colors.transparent, | ||
| splashFactory: NoSplash.splashFactory, | ||
| tapTargetSize: MaterialTapTargetSize.shrinkWrap), | ||
| child: Row(children: [ | ||
| Flexible(child: Padding( | ||
| padding: const EdgeInsetsDirectional.fromSTEB(12, 6, 4, 6), | ||
| child: Row(spacing: 8, children: [ | ||
| AvatarShape( | ||
| size: 28, | ||
| borderRadius: 4, | ||
| child: logo), | ||
| Flexible(child: Text(store.realmName, | ||
| overflow: TextOverflow.ellipsis, | ||
| style: TextStyle( | ||
| color: designVariables.title, | ||
| fontSize: 20, | ||
| height: 24 / 20, | ||
| ).merge(weightVariableTextStyle(context, wght: 600)))), | ||
| ]))), | ||
| Padding( | ||
| padding: EdgeInsetsDirectional.fromSTEB(8, 7, 14, 7), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if the end padding should be 12px, to match the padding before the org icon? This 14px value comes from the Figma, but the Figma uses end-aligned text, and I think the 14px value might be tailored to that case. Also maybe the 7px vertical padding? Seems like it would be simpler, and logical, if we just wrapped the whole row with 12px horizontal padding and 6px vertical padding, and said |
||
| child: Icon(ZulipIcons.arrow_left_right, | ||
| size: 19, | ||
| color: designVariables.icon)), | ||
|
Comment on lines
+410
to
+412
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this would be a good place to use |
||
| ])))); | ||
| } | ||
| } | ||
|
|
||
| abstract class _MenuButton extends StatelessWidget { | ||
| const _MenuButton(); | ||
|
|
||
|
|
@@ -575,23 +659,6 @@ class _MyProfileButton extends _MenuButton { | |
| } | ||
| } | ||
|
|
||
| class _SwitchAccountButton extends _MenuButton { | ||
| const _SwitchAccountButton(); | ||
|
|
||
| @override | ||
| IconData? get icon => ZulipIcons.arrow_left_right; | ||
|
|
||
| @override | ||
| String label(ZulipLocalizations zulipLocalizations) { | ||
| return zulipLocalizations.switchAccountButton; | ||
| } | ||
|
|
||
| @override | ||
| void onPressed(BuildContext context) { | ||
| Navigator.of(context).push(MaterialWidgetRoute(page: const ChooseAccountPage())); | ||
| } | ||
| } | ||
|
|
||
| class _SettingsButton extends _MenuButton { | ||
| const _SettingsButton(); | ||
|
|
||
|
|
||
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 think this would look better without the
AnimatedScaleOnTapeffect. It looks like two things that are moving, rather than one thing that's resizing.I think we should reserve this effect for buttons that are a rounded-rectangle surface:
Thoughts @alya?
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.
For touch feedback here, how about opacity? Per Vlad's suggestion at #mobile-design > all channels view design @ 💬:
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.
Agreed that the effect in that screen capture isn't working. Trying Vlad's suggestion sounds good to me.