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

action_sheet: Redesign bottom sheet #853

Merged
merged 2 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified assets/icons/ZulipIcons.ttf
Binary file not shown.
4 changes: 4 additions & 0 deletions assets/icons/copy.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/icons/format_quote.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/icons/share.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/icons/share_ios.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 6 additions & 0 deletions assets/icons/smile.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/icons/star.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
115 changes: 93 additions & 22 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
Expand All @@ -14,10 +15,12 @@ import 'actions.dart';
import 'clipboard.dart';
import 'compose_box.dart';
import 'dialog.dart';
import 'draggable_scrollable_modal_bottom_sheet.dart';
import 'icons.dart';
import 'inset_shadow.dart';
import 'message_list.dart';
import 'store.dart';
import 'text.dart';
import 'theme.dart';

/// Show a sheet of actions you can take on a message in the message list.
///
Expand All @@ -43,21 +46,48 @@ void showMessageActionSheet({required BuildContext context, required Message mes
&& reactionWithVotes.userIds.contains(store.selfUserId))
?? false;

showDraggableScrollableModalBottomSheet<void>(
final optionButtons = [
if (!hasThumbsUpReactionVote)
AddThumbsUpButton(message: message, messageListContext: context),
StarButton(message: message, messageListContext: context),
if (isComposeBoxOffered)
QuoteAndReplyButton(message: message, messageListContext: context),
if (showMarkAsUnreadButton)
MarkAsUnreadButton(message: message, messageListContext: context, narrow: narrow),
CopyMessageTextButton(message: message, messageListContext: context),
CopyMessageLinkButton(message: message, messageListContext: context),
ShareButton(message: message, messageListContext: context),
];

showModalBottomSheet<void>(
context: context,
// Clip.hardEdge looks bad; Clip.antiAliasWithSaveLayer looks pixel-perfect
// on my iPhone 13 Pro but is marked as "much slower":
// https://api.flutter.dev/flutter/dart-ui/Clip.html
clipBehavior: Clip.antiAlias,
useSafeArea: true,
isScrollControlled: true,
builder: (BuildContext _) {
return Column(children: [
if (!hasThumbsUpReactionVote)
AddThumbsUpButton(message: message, messageListContext: context),
StarButton(message: message, messageListContext: context),
if (isComposeBoxOffered)
QuoteAndReplyButton(message: message, messageListContext: context),
if (showMarkAsUnreadButton)
MarkAsUnreadButton(message: message, messageListContext: context, narrow: narrow),
CopyMessageTextButton(message: message, messageListContext: context),
CopyMessageLinkButton(message: message, messageListContext: context),
ShareButton(message: message, messageListContext: context),
]);
return SafeArea(
minimum: const EdgeInsets.only(bottom: 16),
child: Padding(
padding: const EdgeInsets.fromLTRB(16, 0, 16, 0),
child: Column(
crossAxisAlignment: CrossAxisAlignment.stretch,
mainAxisSize: MainAxisSize.min,
children: [
// TODO(#217): show message text
Flexible(child: InsetShadowBox(
top: 8, bottom: 8,
color: DesignVariables.of(context).bgContextMenu,
child: SingleChildScrollView(
padding: const EdgeInsets.only(top: 16, bottom: 8),
child: ClipRRect(
borderRadius: BorderRadius.circular(7),
child: Column(spacing: 1,
children: optionButtons))))),
const MessageActionSheetCancelButton(),
])));
});
}

Expand All @@ -77,11 +107,47 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget {

@override
Widget build(BuildContext context) {
final designVariables = DesignVariables.of(context);
final zulipLocalizations = ZulipLocalizations.of(context);
return MenuItemButton(
leadingIcon: Icon(icon),
trailingIcon: Icon(icon, color: designVariables.contextMenuItemText),
style: MenuItemButton.styleFrom(
padding: const EdgeInsets.symmetric(vertical: 12, horizontal: 16),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I remember Vlad saying that he doesn't want the material "splash" animation on button presses, so let's specify that:

      style: MenuItemButton.styleFrom(
        splashFactory: NoSplash.splashFactory,

Then to differentiate the background color between pressed and not-pressed, it looks like we're supposed to apply 12% opacity to the contextMenuItemBg variable for not-pressed, and 20% opacity for pressed:

(Those links are for light mode, but it turns out the same values 12% and 20% are used in dark mode.)

MenuItemButton.styleFrom doesn't seem to support specifying different background colors based on pressed and not-pressed, but it returns a ButtonStyle, and ButtonStyle does support that. So, instead of passing backgroundColor to .styleFrom, could just add after the styleFrom(…):

      )
        .copyWith(
          backgroundColor: WidgetStateColor.resolveWith((states) =>
            designVariables.contextMenuItemBg.withOpacity(
              states.contains(WidgetState.pressed) ? 0.2 : 0.12))),

foregroundColor: designVariables.contextMenuItemText,
splashFactory: NoSplash.splashFactory,
).copyWith(backgroundColor: WidgetStateColor.resolveWith((states) =>
designVariables.contextMenuItemBg.withValues(
alpha: states.contains(WidgetState.pressed) ? 0.20 : 0.12))),
onPressed: () => onPressed(context),
child: Text(label(zulipLocalizations)));
child: Text(label(zulipLocalizations),
style: const TextStyle(fontSize: 20, height: 24 / 20)
.merge(weightVariableTextStyle(context, wght: 600)),
));
}
}

class MessageActionSheetCancelButton extends StatelessWidget {
const MessageActionSheetCancelButton({super.key});

@override
Widget build(BuildContext context) {
final designVariables = DesignVariables.of(context);
return TextButton(
style: TextButton.styleFrom(
padding: const EdgeInsets.all(10),
foregroundColor: designVariables.contextMenuCancelText,
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
splashFactory: NoSplash.splashFactory,
).copyWith(backgroundColor: WidgetStateColor.resolveWith((states) =>
designVariables.contextMenuCancelBg.withValues(
alpha: states.contains(WidgetState.pressed) ? 0.20 : 0.15))),
onPressed: () {
Navigator.pop(context);
},
child: Text(ZulipLocalizations.of(context).dialogCancel,
style: const TextStyle(fontSize: 20, height: 24 / 20)
.merge(weightVariableTextStyle(context, wght: 600))),
);
}
}

Expand All @@ -94,7 +160,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton {
required super.messageListContext,
});

@override IconData get icon => Icons.add_reaction_outlined;
@override IconData get icon => ZulipIcons.smile;

@override
String label(ZulipLocalizations zulipLocalizations) {
Expand Down Expand Up @@ -135,11 +201,13 @@ class StarButton extends MessageActionSheetMenuItemButton {
required super.messageListContext,
});

@override IconData get icon => ZulipIcons.star_filled;
@override IconData get icon => _isStarred ? ZulipIcons.star_filled : ZulipIcons.star;

bool get _isStarred => message.flags.contains(MessageFlag.starred);

@override
String label(ZulipLocalizations zulipLocalizations) {
return message.flags.contains(MessageFlag.starred)
return _isStarred
? zulipLocalizations.actionSheetOptionUnstarMessage
: zulipLocalizations.actionSheetOptionStarMessage;
}
Expand Down Expand Up @@ -231,7 +299,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
required super.messageListContext,
});

@override IconData get icon => Icons.format_quote_outlined;
@override IconData get icon => ZulipIcons.format_quote;

@override
String label(ZulipLocalizations zulipLocalizations) {
Expand Down Expand Up @@ -316,7 +384,7 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
required super.messageListContext,
});

@override IconData get icon => Icons.copy;
@override IconData get icon => ZulipIcons.copy;

@override
String label(ZulipLocalizations zulipLocalizations) {
Expand Down Expand Up @@ -384,7 +452,10 @@ class ShareButton extends MessageActionSheetMenuItemButton {
required super.messageListContext,
});

@override IconData get icon => Icons.adaptive.share;
@override
IconData get icon => defaultTargetPlatform == TargetPlatform.iOS
? ZulipIcons.share_ios
: ZulipIcons.share;

@override
String label(ZulipLocalizations zulipLocalizations) {
Expand Down
118 changes: 0 additions & 118 deletions lib/widgets/draggable_scrollable_modal_bottom_sheet.dart

This file was deleted.

Loading