Skip to content

Commit b32b0b1

Browse files
committed
action_sheet: Choose "Close" and "Cancel" consistently for bottom sheets
This fixes the two inconsistencies flagged in discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/bottom.20sheet.20.22Cancel.22.2F.22Close.22.20button/near/2216116 > I think it's reasonable to have both labels, but I think we should > choose them differently than now: > > - "Cancel" when the sheet is about doing an action: [etc.] > > - "Close" when the sheet just presents information or nav options: > [etc.]
1 parent 02a2898 commit b32b0b1

File tree

4 files changed

+28
-9
lines changed

4 files changed

+28
-9
lines changed

lib/widgets/action_sheet.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void _showActionSheet(
9292
child: SingleChildScrollView(
9393
padding: const EdgeInsets.symmetric(vertical: 8),
9494
child: MenuButtonsShape(buttons: optionButtons)))),
95-
const ActionSheetCancelButton(),
95+
const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.cancel),
9696
]))),
9797
]))));
9898
});
@@ -160,12 +160,22 @@ abstract class ActionSheetMenuItemButton extends StatelessWidget {
160160
}
161161
}
162162

163-
class ActionSheetCancelButton extends StatelessWidget {
164-
const ActionSheetCancelButton({super.key});
163+
/// A stretched gray "Cancel" / "Close" button for the bottom of a bottom sheet.
164+
class BottomSheetDismissButton extends StatelessWidget {
165+
const BottomSheetDismissButton({super.key, required this.style});
166+
167+
final BottomSheetDismissButtonStyle style;
165168

166169
@override
167170
Widget build(BuildContext context) {
168171
final designVariables = DesignVariables.of(context);
172+
final zulipLocalizations = ZulipLocalizations.of(context);
173+
174+
final label = switch (style) {
175+
BottomSheetDismissButtonStyle.cancel => zulipLocalizations.dialogCancel,
176+
BottomSheetDismissButtonStyle.close => zulipLocalizations.dialogClose,
177+
};
178+
169179
return TextButton(
170180
style: TextButton.styleFrom(
171181
minimumSize: const Size.fromHeight(44),
@@ -180,12 +190,20 @@ class ActionSheetCancelButton extends StatelessWidget {
180190
onPressed: () {
181191
Navigator.pop(context);
182192
},
183-
child: Text(ZulipLocalizations.of(context).dialogCancel,
193+
child: Text(label,
184194
style: const TextStyle(fontSize: 20, height: 24 / 20)
185195
.merge(weightVariableTextStyle(context, wght: 600))));
186196
}
187197
}
188198

199+
enum BottomSheetDismissButtonStyle {
200+
/// The "Cancel" label, for action sheets.
201+
cancel,
202+
203+
/// The "Close" label, for bottom sheets that are read-only or for navigation.
204+
close,
205+
}
206+
189207
/// Show a sheet of actions you can take on a channel.
190208
///
191209
/// Needs a [PageRoot] ancestor.

lib/widgets/emoji_reaction.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ class _EmojiPickerState extends State<EmojiPicker> with PerAccountStoreAwareStat
518518
states.contains(WidgetState.pressed)
519519
? designVariables.contextMenuItemBg.withFadedAlpha(0.20)
520520
: Colors.transparent)),
521-
child: Text(zulipLocalizations.dialogClose,
521+
child: Text(zulipLocalizations.dialogCancel,
522522
style: const TextStyle(fontSize: 20, height: 30 / 20))),
523523
])),
524524
Expanded(child: InsetShadowBox(

lib/widgets/home.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,8 @@ void _showMainMenu(BuildContext context, {
324324
child: AnimatedScaleOnTap(
325325
scaleEnd: 0.95,
326326
duration: Duration(milliseconds: 100),
327-
child: ActionSheetCancelButton())),
327+
child: BottomSheetDismissButton(
328+
style: BottomSheetDismissButtonStyle.close))),
328329
])));
329330
});
330331
}

test/widgets/home_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ void main () {
231231
await tapOpenMenuAndAwait(tester);
232232
checkIconSelected(tester, inboxMenuIconFinder);
233233
checkIconNotSelected(tester, channelsMenuIconFinder);
234-
await tapButtonAndAwaitTransition(tester, find.text('Cancel'));
234+
await tapButtonAndAwaitTransition(tester, find.text('Close'));
235235

236236
await tester.tap(find.byIcon(ZulipIcons.hash_italic));
237237
await tester.pump();
@@ -265,10 +265,10 @@ void main () {
265265
await tapButtonAndAwaitTransition(tester, channelsMenuIconFinder);
266266
});
267267

268-
testWidgets('cancel button dismisses the menu', (tester) async {
268+
testWidgets('close button dismisses the menu', (tester) async {
269269
await prepare(tester);
270270
await tapOpenMenuAndAwait(tester);
271-
await tapButtonAndAwaitTransition(tester, find.text('Cancel'));
271+
await tapButtonAndAwaitTransition(tester, find.text('Close'));
272272
});
273273

274274
testWidgets('menu buttons dismiss the menu', (tester) async {

0 commit comments

Comments
 (0)