Skip to content

Commit beed798

Browse files
sm-sayedism-sayedi
committed
action_sheet: Redesign bottom sheet
Fixes: #90
1 parent 72e1dc7 commit beed798

File tree

3 files changed

+170
-38
lines changed

3 files changed

+170
-38
lines changed

‎lib/widgets/action_sheet.dart

Lines changed: 89 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:io';
2+
13
import 'package:flutter/material.dart';
24
import 'package:flutter/services.dart';
35
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
@@ -12,10 +14,11 @@ import 'actions.dart';
1214
import 'clipboard.dart';
1315
import 'compose_box.dart';
1416
import 'dialog.dart';
15-
import 'draggable_scrollable_modal_bottom_sheet.dart';
1617
import 'icons.dart';
1718
import 'message_list.dart';
1819
import 'store.dart';
20+
import 'text.dart';
21+
import 'theme.dart';
1922

2023
/// Show a sheet of actions you can take on a message in the message list.
2124
///
@@ -41,25 +44,50 @@ void showMessageActionSheet({required BuildContext context, required Message mes
4144
&& reactionWithVotes.userIds.contains(store.selfUserId))
4245
?? false;
4346

44-
showDraggableScrollableModalBottomSheet<void>(
47+
final designVariables = DesignVariables.of(context);
48+
showModalBottomSheet<void>(
4549
context: context,
50+
clipBehavior: Clip.antiAlias,
51+
backgroundColor: designVariables.actionSheetBackground,
52+
shape: const RoundedRectangleBorder(borderRadius: BorderRadius.vertical(top: Radius.circular(20.0))),
53+
useSafeArea: true,
54+
isScrollControlled: true,
4655
builder: (BuildContext _) {
47-
return Column(children: [
48-
if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context),
49-
StarButton(message: message, messageListContext: context),
50-
if (isComposeBoxOffered) QuoteAndReplyButton(
51-
message: message,
52-
messageListContext: context,
53-
),
54-
if (showMarkAsUnreadButton) MarkAsUnreadButton(
55-
message: message,
56-
messageListContext: context,
57-
narrow: narrow,
56+
return Padding(
57+
padding: const EdgeInsets.all(16.0),
58+
child: Column(
59+
crossAxisAlignment: CrossAxisAlignment.stretch,
60+
mainAxisSize: MainAxisSize.min,
61+
children: [
62+
// TODO(#217): show message text
63+
Flexible(
64+
child: SingleChildScrollView(
65+
child: ClipRRect(
66+
borderRadius: BorderRadius.circular(7),
67+
child: Column(spacing: 1, children: [
68+
if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context),
69+
StarButton(message: message, messageListContext: context),
70+
if (isComposeBoxOffered) QuoteAndReplyButton(
71+
message: message,
72+
messageListContext: context,
73+
),
74+
if (showMarkAsUnreadButton) MarkAsUnreadButton(
75+
message: message,
76+
messageListContext: context,
77+
narrow: narrow,
78+
),
79+
CopyMessageTextButton(message: message, messageListContext: context),
80+
CopyMessageLinkButton(message: message, messageListContext: context),
81+
ShareButton(message: message, messageListContext: context),
82+
]),
83+
),
84+
),
85+
),
86+
const SizedBox(height: 8),
87+
const MessageActionSheetCancelButton(),
88+
],
5889
),
59-
CopyMessageTextButton(message: message, messageListContext: context),
60-
CopyMessageLinkButton(message: message, messageListContext: context),
61-
ShareButton(message: message, messageListContext: context),
62-
]);
90+
);
6391
});
6492
}
6593

@@ -79,11 +107,22 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget {
79107

80108
@override
81109
Widget build(BuildContext context) {
110+
final designVariables = DesignVariables.of(context);
82111
final zulipLocalizations = ZulipLocalizations.of(context);
83112
return MenuItemButton(
84-
leadingIcon: Icon(icon),
113+
trailingIcon: Icon(icon, color: designVariables.actionSheetMenuButtonForeground),
114+
style: MenuItemButton.styleFrom(
115+
padding: const EdgeInsets.symmetric(vertical: 12, horizontal: 16),
116+
tapTargetSize: MaterialTapTargetSize.shrinkWrap,
117+
minimumSize: Size.zero,
118+
backgroundColor: designVariables.actionSheetMenuButtonBackground,
119+
foregroundColor: designVariables.actionSheetMenuButtonForeground,
120+
),
85121
onPressed: () => onPressed(context),
86-
child: Text(label(zulipLocalizations)));
122+
child: Text(label(zulipLocalizations),
123+
style: const TextStyle(fontSize: 20)
124+
.merge(weightVariableTextStyle(context, wght: 600)),
125+
));
87126
}
88127
}
89128

@@ -96,7 +135,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton {
96135
required super.messageListContext,
97136
});
98137

99-
@override IconData get icon => Icons.add_reaction_outlined;
138+
@override IconData get icon => ZulipIcons.smile;
100139

101140
@override
102141
String label(ZulipLocalizations zulipLocalizations) {
@@ -137,11 +176,13 @@ class StarButton extends MessageActionSheetMenuItemButton {
137176
required super.messageListContext,
138177
});
139178

140-
@override IconData get icon => ZulipIcons.star_filled;
179+
@override IconData get icon => _isStarred ? ZulipIcons.star_filled : ZulipIcons.star;
180+
181+
bool get _isStarred => message.flags.contains(MessageFlag.starred);
141182

142183
@override
143184
String label(ZulipLocalizations zulipLocalizations) {
144-
return message.flags.contains(MessageFlag.starred)
185+
return _isStarred
145186
? zulipLocalizations.actionSheetOptionUnstarMessage
146187
: zulipLocalizations.actionSheetOptionStarMessage;
147188
}
@@ -233,7 +274,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
233274
required super.messageListContext,
234275
});
235276

236-
@override IconData get icon => Icons.format_quote_outlined;
277+
@override IconData get icon => ZulipIcons.format_quote;
237278

238279
@override
239280
String label(ZulipLocalizations zulipLocalizations) {
@@ -318,7 +359,7 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
318359
required super.messageListContext,
319360
});
320361

321-
@override IconData get icon => Icons.copy;
362+
@override IconData get icon => ZulipIcons.copy;
322363

323364
@override
324365
String label(ZulipLocalizations zulipLocalizations) {
@@ -386,7 +427,7 @@ class ShareButton extends MessageActionSheetMenuItemButton {
386427
required super.messageListContext,
387428
});
388429

389-
@override IconData get icon => Icons.adaptive.share;
430+
@override IconData get icon => Platform.isIOS ? ZulipIcons.share_ios : ZulipIcons.share;
390431

391432
@override
392433
String label(ZulipLocalizations zulipLocalizations) {
@@ -435,3 +476,26 @@ class ShareButton extends MessageActionSheetMenuItemButton {
435476
}
436477
}
437478
}
479+
480+
class MessageActionSheetCancelButton extends StatelessWidget {
481+
const MessageActionSheetCancelButton({super.key});
482+
483+
@override
484+
Widget build(BuildContext context) {
485+
final designVariables = DesignVariables.of(context);
486+
return TextButton(
487+
style: TextButton.styleFrom(
488+
padding: const EdgeInsets.all(10),
489+
tapTargetSize: MaterialTapTargetSize.shrinkWrap,
490+
minimumSize: Size.zero,
491+
backgroundColor: designVariables.actionSheetCancelButtonBackground,
492+
foregroundColor: designVariables.actionSheetCancelButtonForeground,
493+
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
494+
),
495+
onPressed: () => Navigator.pop(context),
496+
child: Text(ZulipLocalizations.of(context).dialogCancel,
497+
style: const TextStyle(fontSize: 20)
498+
.merge(weightVariableTextStyle(context, wght: 600))),
499+
);
500+
}
501+
}

‎lib/widgets/theme.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
114114
mainBackground: const Color(0xfff0f0f0),
115115
title: const Color(0xff1a1a1a),
116116
channelColorSwatches: ChannelColorSwatches.light,
117+
actionSheetBackground: const HSLColor.fromAHSL(1, 0, 0, 0.94).toColor(),
118+
actionSheetCancelButtonBackground: const HSLColor.fromAHSL(0.15, 240, 0.05, 0.50).toColor(),
119+
actionSheetCancelButtonForeground: const HSLColor.fromAHSL(1, 0, 0, 0.13).toColor(),
120+
actionSheetMenuButtonBackground: const HSLColor.fromAHSL(0.12, 243.53, 0.69, 0.61).toColor(),
121+
actionSheetMenuButtonForeground: const HSLColor.fromAHSL(1, 251.74, 0.70, 0.38).toColor(),
117122
atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(),
118123
dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(),
119124
errorBannerBackground: const HSLColor.fromAHSL(1, 4, 0.33, 0.90).toColor(),
@@ -142,6 +147,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
142147
mainBackground: const Color(0xff1d1d1d),
143148
title: const Color(0xffffffff),
144149
channelColorSwatches: ChannelColorSwatches.dark,
150+
actionSheetBackground: const HSLColor.fromAHSL(1, 0, 0, 0.14).toColor(),
151+
actionSheetCancelButtonBackground: const HSLColor.fromAHSL(0.15, 240, 0.05, 0.50).toColor(),
152+
actionSheetCancelButtonForeground: const HSLColor.fromAHSL(0.75, 0, 0, 1).toColor(),
153+
actionSheetMenuButtonBackground: const HSLColor.fromAHSL(0.12, 240.89, 0.98, 0.73).toColor(),
154+
actionSheetMenuButtonForeground: const HSLColor.fromAHSL(1, 237.17, 0.96, 0.78).toColor(),
145155
// TODO(design-dark) need proper dark-theme color (this is ad hoc)
146156
atMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(),
147157
dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(),
@@ -176,6 +186,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
176186
required this.mainBackground,
177187
required this.title,
178188
required this.channelColorSwatches,
189+
required this.actionSheetBackground,
190+
required this.actionSheetCancelButtonBackground,
191+
required this.actionSheetCancelButtonForeground,
192+
required this.actionSheetMenuButtonBackground,
193+
required this.actionSheetMenuButtonForeground,
179194
required this.atMentionMarker,
180195
required this.dmHeaderBg,
181196
required this.errorBannerBackground,
@@ -216,6 +231,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
216231
final ChannelColorSwatches channelColorSwatches;
217232

218233
// Not named variables in Figma; taken from older Figma drafts, or elsewhere.
234+
final Color actionSheetBackground;
235+
final Color actionSheetCancelButtonBackground;
236+
final Color actionSheetCancelButtonForeground;
237+
final Color actionSheetMenuButtonBackground;
238+
final Color actionSheetMenuButtonForeground;
219239
final Color atMentionMarker;
220240
final Color dmHeaderBg;
221241
final Color errorBannerBackground;
@@ -243,6 +263,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
243263
Color? mainBackground,
244264
Color? title,
245265
ChannelColorSwatches? channelColorSwatches,
266+
Color? actionSheetBackground,
267+
Color? actionSheetCancelButtonBackground,
268+
Color? actionSheetCancelButtonForeground,
269+
Color? actionSheetMenuButtonBackground,
270+
Color? actionSheetMenuButtonForeground,
246271
Color? atMentionMarker,
247272
Color? dmHeaderBg,
248273
Color? errorBannerBackground,
@@ -269,6 +294,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
269294
mainBackground: mainBackground ?? this.mainBackground,
270295
title: title ?? this.title,
271296
channelColorSwatches: channelColorSwatches ?? this.channelColorSwatches,
297+
actionSheetBackground: actionSheetBackground ?? this.actionSheetBackground,
298+
actionSheetCancelButtonBackground: actionSheetCancelButtonBackground ?? this.actionSheetCancelButtonBackground,
299+
actionSheetCancelButtonForeground: actionSheetCancelButtonForeground ?? this.actionSheetCancelButtonForeground,
300+
actionSheetMenuButtonBackground: actionSheetMenuButtonBackground ?? this.actionSheetMenuButtonBackground,
301+
actionSheetMenuButtonForeground: actionSheetMenuButtonForeground ?? this.actionSheetMenuButtonBackground,
272302
atMentionMarker: atMentionMarker ?? this.atMentionMarker,
273303
dmHeaderBg: dmHeaderBg ?? this.dmHeaderBg,
274304
errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground,
@@ -302,6 +332,11 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
302332
mainBackground: Color.lerp(mainBackground, other.mainBackground, t)!,
303333
title: Color.lerp(title, other.title, t)!,
304334
channelColorSwatches: ChannelColorSwatches.lerp(channelColorSwatches, other.channelColorSwatches, t),
335+
actionSheetBackground: Color.lerp(actionSheetBackground, other.actionSheetBackground, t)!,
336+
actionSheetCancelButtonBackground: Color.lerp(actionSheetCancelButtonBackground, other.actionSheetCancelButtonBackground, t)!,
337+
actionSheetCancelButtonForeground: Color.lerp(actionSheetCancelButtonForeground, other.actionSheetCancelButtonForeground, t)!,
338+
actionSheetMenuButtonBackground: Color.lerp(actionSheetMenuButtonBackground, other.actionSheetMenuButtonBackground, t)!,
339+
actionSheetMenuButtonForeground: Color.lerp(actionSheetMenuButtonForeground, other.actionSheetMenuButtonBackground, t)!,
305340
atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!,
306341
dmHeaderBg: Color.lerp(dmHeaderBg, other.dmHeaderBg, t)!,
307342
errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!,

‎test/widgets/action_sheet_test.dart

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ void main() {
100100

101101
group('AddThumbsUpButton', () {
102102
Future<void> tapButton(WidgetTester tester) async {
103-
await tester.ensureVisible(find.byIcon(Icons.add_reaction_outlined, skipOffstage: false));
104-
await tester.tap(find.byIcon(Icons.add_reaction_outlined));
103+
await tester.ensureVisible(find.byIcon(ZulipIcons.smile, skipOffstage: false));
104+
await tester.tap(find.byIcon(ZulipIcons.smile));
105105
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
106106
}
107107

@@ -147,15 +147,15 @@ void main() {
147147
});
148148

149149
group('StarButton', () {
150-
Future<void> tapButton(WidgetTester tester) async {
150+
Future<void> tapButton(WidgetTester tester, {bool starred = false}) async {
151151
// Starred messages include the same icon so we need to
152152
// match only by descendants of [BottomSheet].
153153
await tester.ensureVisible(find.descendant(
154154
of: find.byType(BottomSheet),
155-
matching: find.byIcon(ZulipIcons.star_filled, skipOffstage: false)));
155+
matching: find.byIcon(starred ? ZulipIcons.star_filled : ZulipIcons.star, skipOffstage: false)));
156156
await tester.tap(find.descendant(
157157
of: find.byType(BottomSheet),
158-
matching: find.byIcon(ZulipIcons.star_filled)));
158+
matching: find.byIcon(starred ? ZulipIcons.star_filled : ZulipIcons.star)));
159159
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
160160
}
161161

@@ -186,7 +186,7 @@ void main() {
186186

187187
final connection = store.connection as FakeApiConnection;
188188
connection.prepare(json: {});
189-
await tapButton(tester);
189+
await tapButton(tester, starred: true);
190190
await tester.pump(Duration.zero);
191191

192192
check(connection.lastRequest).isA<http.Request>()
@@ -233,7 +233,7 @@ void main() {
233233
'msg': 'Invalid message(s)',
234234
'result': 'error',
235235
});
236-
await tapButton(tester);
236+
await tapButton(tester, starred: true);
237237
await tester.pump(Duration.zero); // error arrives; error dialog shows
238238

239239
await tester.tap(find.byWidget(checkErrorDialog(tester,
@@ -249,14 +249,14 @@ void main() {
249249
}
250250

251251
Widget? findQuoteAndReplyButton(WidgetTester tester) {
252-
return tester.widgetList(find.byIcon(Icons.format_quote_outlined)).singleOrNull;
252+
return tester.widgetList(find.byIcon(ZulipIcons.format_quote)).singleOrNull;
253253
}
254254

255255
/// Simulates tapping the quote-and-reply button in the message action sheet.
256256
///
257257
/// Checks that there is a quote-and-reply button.
258258
Future<void> tapQuoteAndReplyButton(WidgetTester tester) async {
259-
await tester.ensureVisible(find.byIcon(Icons.format_quote_outlined, skipOffstage: false));
259+
await tester.ensureVisible(find.byIcon(ZulipIcons.format_quote, skipOffstage: false));
260260
final quoteAndReplyButton = findQuoteAndReplyButton(tester);
261261
check(quoteAndReplyButton).isNotNull();
262262
await tester.tap(find.byWidget(quoteAndReplyButton!));
@@ -462,8 +462,8 @@ void main() {
462462
});
463463

464464
Future<void> tapCopyMessageTextButton(WidgetTester tester) async {
465-
await tester.ensureVisible(find.byIcon(Icons.copy, skipOffstage: false));
466-
await tester.tap(find.byIcon(Icons.copy));
465+
await tester.ensureVisible(find.byIcon(ZulipIcons.copy, skipOffstage: false));
466+
await tester.tap(find.byIcon(ZulipIcons.copy));
467467
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
468468
}
469469

@@ -559,8 +559,8 @@ void main() {
559559
}
560560

561561
Future<void> tapShareButton(WidgetTester tester) async {
562-
await tester.ensureVisible(find.byIcon(Icons.adaptive.share, skipOffstage: false));
563-
await tester.tap(find.byIcon(Icons.adaptive.share));
562+
await tester.ensureVisible(find.byIcon(ZulipIcons.share, skipOffstage: false));
563+
await tester.tap(find.byIcon(ZulipIcons.share));
564564
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
565565
}
566566

@@ -610,4 +610,37 @@ void main() {
610610
check(mockSharePlus.sharedString).isNull();
611611
});
612612
});
613+
614+
group('MessageActionSheetCancelButton', () {
615+
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
616+
617+
Finder cancelButtonFinder() => find.text(zulipLocalizations.dialogCancel);
618+
619+
void checkActionSheet(WidgetTester tester, {required bool isShown}) {
620+
// TODO(i18n) skip translation for now
621+
check(find.text('React with 👍').evaluate().length).equals(isShown ? 1 : 0);
622+
check(find.text(zulipLocalizations.actionSheetOptionStarMessage)
623+
.evaluate().length).equals(isShown ? 1 : 0);
624+
check(find.text(zulipLocalizations.actionSheetOptionQuoteAndReply)
625+
.evaluate().length).equals(isShown ? 1 : 0);
626+
check(find.text(zulipLocalizations.actionSheetOptionCopyMessageText)
627+
.evaluate().length).equals(isShown ? 1 : 0);
628+
check(find.text(zulipLocalizations.actionSheetOptionCopyMessageLink)
629+
.evaluate().length).equals(isShown ? 1 : 0);
630+
check(find.text(zulipLocalizations.actionSheetOptionShare)
631+
.evaluate().length).equals(isShown ? 1 : 0);
632+
633+
check(cancelButtonFinder().evaluate().length).equals(isShown ? 1 : 0);
634+
}
635+
636+
testWidgets('pressing the button dismisses the action sheet', (tester) async {
637+
final message = eg.streamMessage();
638+
await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));
639+
checkActionSheet(tester, isShown: true);
640+
641+
await tester.tap(cancelButtonFinder());
642+
await tester.pumpAndSettle();
643+
checkActionSheet(tester, isShown: false);
644+
});
645+
});
613646
}

0 commit comments

Comments
 (0)