Skip to content

Commit e075701

Browse files
chrisbobbePIG208
andcommitted
compose: Redesign error banner that replaces the compose box
This cherry-picks (with some adjustments) UI changes from Zixuan's current revision of PR zulip#924 (for issue zulip#720): compose: Support error banner redesign for replacing the compose box See the Figma: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-15770&node-type=frame&t=vOSEWlXAhBa5EsAv-0 The new design is meant to be flush with the top of the compose-box surface, so zulip#1089 (where the old banner's top margin disappeared, looking buggy) is resolved. The surface (but not content) of the redesigned banner seems like it should extend through any left/right device insets to those edges of the screen. By taking the banner as param separate from `body`, _ComposeBoxContainer lets the banner do that -- without dropping its responsibility for keeping the body (inputs, compose buttons, and send button) out of the insets. The body doesn't need to consume the insets itself and it's already complicated enough without needing its own SafeAreas. It also seems like the banner should pad the bottom inset when it's meant to replace the body, so we do that and make the expectation clear in the `errorBanner` dartdoc. Co-authored-by: Zixuan James Li <[email protected]> Fixes: zulip#1089
1 parent d0e2988 commit e075701

File tree

2 files changed

+96
-39
lines changed

2 files changed

+96
-39
lines changed

lib/widgets/compose_box.dart

+75-18
Original file line numberDiff line numberDiff line change
@@ -1010,23 +1010,60 @@ class _SendButtonState extends State<_SendButton> {
10101010
}
10111011

10121012
class _ComposeBoxContainer extends StatelessWidget {
1013-
const _ComposeBoxContainer({required this.child});
1013+
const _ComposeBoxContainer({
1014+
required this.body,
1015+
this.errorBanner,
1016+
}) : assert(body != null || errorBanner != null);
10141017

1015-
final Widget child;
1018+
/// The text inputs, compose-button row, and send button.
1019+
///
1020+
/// This widget does not need a [SafeArea] to consume any device insets.
1021+
///
1022+
/// Can be null, but only if [errorBanner] is non-null.
1023+
final Widget? body;
1024+
1025+
/// An error bar that goes at the top.
1026+
///
1027+
/// This may be present on its own or with a [body].
1028+
/// If [body] is null this must be present.
1029+
///
1030+
/// This widget should use a [SafeArea] to pad the left/right device insets.
1031+
/// If [body] is null it will also receive a bottom inset to pad;
1032+
/// it should pad that too.
1033+
final Widget? errorBanner;
1034+
1035+
Widget _paddedBody() {
1036+
assert(body != null);
1037+
return SafeArea(minimum: const EdgeInsets.symmetric(horizontal: 8),
1038+
child: body!);
1039+
}
10161040

10171041
@override
10181042
Widget build(BuildContext context) {
10191043
final designVariables = DesignVariables.of(context);
10201044

1045+
final List<Widget> children = switch ((errorBanner, body)) {
1046+
(Widget(), Widget()) => [
1047+
// _paddedBody() already pads the bottom inset,
1048+
// so make sure the error banner doesn't double-pad it.
1049+
MediaQuery.removePadding(context: context, removeBottom: true,
1050+
child: errorBanner!),
1051+
_paddedBody(),
1052+
],
1053+
(Widget(), null) => [errorBanner!],
1054+
(null, Widget()) => [_paddedBody()],
1055+
(null, null) => throw UnimplementedError(), // not allowed, see dartdoc
1056+
};
1057+
10211058
// TODO(design): Maybe put a max width on the compose box, like we do on
10221059
// the message list itself
10231060
return Container(width: double.infinity,
10241061
decoration: BoxDecoration(
10251062
border: Border(top: BorderSide(color: designVariables.borderBar))),
10261063
child: Material(
10271064
color: designVariables.composeBoxBg,
1028-
child: SafeArea(minimum: const EdgeInsets.symmetric(horizontal: 8),
1029-
child: child)));
1065+
child: Column(
1066+
children: children)));
10301067
}
10311068
}
10321069

@@ -1194,16 +1231,30 @@ class _ErrorBanner extends StatelessWidget {
11941231
@override
11951232
Widget build(BuildContext context) {
11961233
final designVariables = DesignVariables.of(context);
1197-
return Container(
1198-
padding: const EdgeInsets.all(8),
1234+
final labelTextStyle = TextStyle(
1235+
fontSize: 17,
1236+
height: 22 / 17,
1237+
color: designVariables.btnLabelAttMediumDanger,
1238+
).merge(weightVariableTextStyle(context, wght: 600));
1239+
1240+
return DecoratedBox(
11991241
decoration: BoxDecoration(
1200-
color: designVariables.errorBannerBackground,
1201-
border: Border.all(color: designVariables.errorBannerBorder),
1202-
borderRadius: BorderRadius.circular(5)),
1203-
child: Text(label,
1204-
style: TextStyle(fontSize: 18, color: designVariables.errorBannerLabel),
1205-
),
1206-
);
1242+
color: designVariables.bannerBgIntDanger),
1243+
child: SafeArea(
1244+
minimum: const EdgeInsetsDirectional.only(start: 8)
1245+
// (SafeArea.minimum doesn't take an EdgeInsetsDirectional)
1246+
.resolve(Directionality.of(context)),
1247+
child: Row(
1248+
children: [
1249+
Expanded(
1250+
child: Padding(
1251+
padding: const EdgeInsetsDirectional.fromSTEB(8, 9, 0, 9),
1252+
child: Text(style: labelTextStyle,
1253+
label))),
1254+
const SizedBox(width: 8),
1255+
// TODO(#720) "x" button goes here.
1256+
// 24px square with 8px touchable padding in all directions?
1257+
])));
12071258
}
12081259
}
12091260

@@ -1291,12 +1342,18 @@ class _ComposeBoxState extends State<ComposeBox> implements ComposeBoxState {
12911342

12921343
@override
12931344
Widget build(BuildContext context) {
1294-
final Widget body;
1345+
final Widget? body;
1346+
Widget? errorBanner;
12951347

1296-
final errorBanner = _errorBanner(context);
1348+
errorBanner = _errorBanner(context);
12971349
if (errorBanner != null) {
1298-
body = errorBanner;
1350+
body = null;
12991351
} else {
1352+
// TODO(#720) dismissable message-send error, maybe something like:
1353+
// if (controller.sendMessageError.value != null) {
1354+
// errorBanner = _ErrorBanner(label:
1355+
// ZulipLocalizations.of(context).errorSendMessageTimeout);
1356+
// }
13001357
final narrow = widget.narrow;
13011358
switch (narrow) {
13021359
case ChannelNarrow():
@@ -1315,10 +1372,10 @@ class _ComposeBoxState extends State<ComposeBox> implements ComposeBoxState {
13151372
case MentionsNarrow():
13161373
case StarredMessagesNarrow():
13171374
assert(false);
1318-
body = const SizedBox.shrink();
1375+
body = null;
13191376
}
13201377
}
13211378

1322-
return _ComposeBoxContainer(child: body);
1379+
return _ComposeBoxContainer(body: body, errorBanner: errorBanner);
13231380
}
13241381
}

lib/widgets/theme.dart

+21-21
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
114114
DesignVariables.light() :
115115
this._(
116116
background: const Color(0xffffffff),
117+
bannerBgIntDanger: const Color(0xfff2e4e4),
117118
bgContextMenu: const Color(0xfff2f2f2),
118119
bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15),
119120
bgTopBar: const Color(0xfff5f5f5),
120121
borderBar: Colors.black.withValues(alpha: 0.2),
122+
btnLabelAttLowIntDanger: const Color(0xffc0070a),
123+
btnLabelAttMediumDanger: const Color(0xffac0508),
121124
composeBoxBg: const Color(0xffffffff),
122125
contextMenuCancelText: const Color(0xff222222),
123126
contextMenuItemBg: const Color(0xff6159e1),
@@ -135,9 +138,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
135138
atMentionMarker: const HSLColor.fromAHSL(0.5, 0, 0, 0.2).toColor(),
136139
contextMenuCancelBg: const Color(0xff797986),
137140
dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.35, 0.93).toColor(),
138-
errorBannerBackground: const HSLColor.fromAHSL(1, 4, 0.33, 0.90).toColor(),
139-
errorBannerBorder: const HSLColor.fromAHSL(0.4, 3, 0.57, 0.33).toColor(),
140-
errorBannerLabel: const HSLColor.fromAHSL(1, 4, 0.58, 0.33).toColor(),
141141
groupDmConversationIcon: Colors.black.withValues(alpha: 0.5),
142142
groupDmConversationIconBg: const Color(0x33808080),
143143
loginOrDivider: const Color(0xffdedede),
@@ -154,10 +154,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
154154
DesignVariables.dark() :
155155
this._(
156156
background: const Color(0xff000000),
157+
bannerBgIntDanger: const Color(0xff461616),
157158
bgContextMenu: const Color(0xff262626),
158159
bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37),
159160
bgTopBar: const Color(0xff242424),
160161
borderBar: Colors.black.withValues(alpha: 0.5),
162+
btnLabelAttLowIntDanger: const Color(0xffff8b7c),
163+
btnLabelAttMediumDanger: const Color(0xffff8b7c),
161164
composeBoxBg: const Color(0xff0f0f0f),
162165
contextMenuCancelText: const Color(0xffffffff).withValues(alpha: 0.75),
163166
contextMenuItemBg: const Color(0xff7977fe),
@@ -176,9 +179,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
176179
// TODO(design-dark) need proper dark-theme color (this is ad hoc)
177180
atMentionMarker: const HSLColor.fromAHSL(0.4, 0, 0, 1).toColor(),
178181
dmHeaderBg: const HSLColor.fromAHSL(1, 46, 0.15, 0.2).toColor(),
179-
errorBannerBackground: const HSLColor.fromAHSL(1, 0, 0.61, 0.19).toColor(),
180-
errorBannerBorder: const HSLColor.fromAHSL(0.4, 3, 0.73, 0.74).toColor(),
181-
errorBannerLabel: const HSLColor.fromAHSL(1, 2, 0.73, 0.80).toColor(),
182182
// TODO(design-dark) need proper dark-theme color (this is ad hoc)
183183
groupDmConversationIcon: Colors.white.withValues(alpha: 0.5),
184184
// TODO(design-dark) need proper dark-theme color (this is ad hoc)
@@ -201,10 +201,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
201201

202202
DesignVariables._({
203203
required this.background,
204+
required this.bannerBgIntDanger,
204205
required this.bgContextMenu,
205206
required this.bgCounterUnread,
206207
required this.bgTopBar,
207208
required this.borderBar,
209+
required this.btnLabelAttLowIntDanger,
210+
required this.btnLabelAttMediumDanger,
208211
required this.composeBoxBg,
209212
required this.contextMenuCancelText,
210213
required this.contextMenuItemBg,
@@ -222,9 +225,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
222225
required this.atMentionMarker,
223226
required this.contextMenuCancelBg,
224227
required this.dmHeaderBg,
225-
required this.errorBannerBackground,
226-
required this.errorBannerBorder,
227-
required this.errorBannerLabel,
228228
required this.groupDmConversationIcon,
229229
required this.groupDmConversationIconBg,
230230
required this.loginOrDivider,
@@ -249,10 +249,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
249249
}
250250

251251
final Color background;
252+
final Color bannerBgIntDanger;
252253
final Color bgContextMenu;
253254
final Color bgCounterUnread;
254255
final Color bgTopBar;
255256
final Color borderBar;
257+
final Color btnLabelAttLowIntDanger;
258+
final Color btnLabelAttMediumDanger;
256259
final Color composeBoxBg;
257260
final Color contextMenuCancelText;
258261
final Color contextMenuItemBg;
@@ -274,9 +277,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
274277
final Color atMentionMarker;
275278
final Color contextMenuCancelBg; // In Figma, but unnamed.
276279
final Color dmHeaderBg;
277-
final Color errorBannerBackground;
278-
final Color errorBannerBorder;
279-
final Color errorBannerLabel;
280280
final Color groupDmConversationIcon;
281281
final Color groupDmConversationIconBg;
282282
final Color loginOrDivider; // TODO(design-dark) need proper dark-theme color (this is ad hoc)
@@ -292,10 +292,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
292292
@override
293293
DesignVariables copyWith({
294294
Color? background,
295+
Color? bannerBgIntDanger,
295296
Color? bgContextMenu,
296297
Color? bgCounterUnread,
297298
Color? bgTopBar,
298299
Color? borderBar,
300+
Color? btnLabelAttLowIntDanger,
301+
Color? btnLabelAttMediumDanger,
299302
Color? composeBoxBg,
300303
Color? contextMenuCancelText,
301304
Color? contextMenuItemBg,
@@ -313,9 +316,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
313316
Color? atMentionMarker,
314317
Color? contextMenuCancelBg,
315318
Color? dmHeaderBg,
316-
Color? errorBannerBackground,
317-
Color? errorBannerBorder,
318-
Color? errorBannerLabel,
319319
Color? groupDmConversationIcon,
320320
Color? groupDmConversationIconBg,
321321
Color? loginOrDivider,
@@ -330,10 +330,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
330330
}) {
331331
return DesignVariables._(
332332
background: background ?? this.background,
333+
bannerBgIntDanger: bannerBgIntDanger ?? this.bannerBgIntDanger,
333334
bgContextMenu: bgContextMenu ?? this.bgContextMenu,
334335
bgCounterUnread: bgCounterUnread ?? this.bgCounterUnread,
335336
bgTopBar: bgTopBar ?? this.bgTopBar,
336337
borderBar: borderBar ?? this.borderBar,
338+
btnLabelAttLowIntDanger: btnLabelAttLowIntDanger ?? this.btnLabelAttLowIntDanger,
339+
btnLabelAttMediumDanger: btnLabelAttMediumDanger ?? this.btnLabelAttMediumDanger,
337340
composeBoxBg: composeBoxBg ?? this.composeBoxBg,
338341
contextMenuCancelText: contextMenuCancelText ?? this.contextMenuCancelText,
339342
contextMenuItemBg: contextMenuItemBg ?? this.contextMenuItemBg,
@@ -351,9 +354,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
351354
atMentionMarker: atMentionMarker ?? this.atMentionMarker,
352355
contextMenuCancelBg: contextMenuCancelBg ?? this.contextMenuCancelBg,
353356
dmHeaderBg: dmHeaderBg ?? this.dmHeaderBg,
354-
errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground,
355-
errorBannerBorder: errorBannerBorder ?? this.errorBannerBorder,
356-
errorBannerLabel: errorBannerLabel ?? this.errorBannerLabel,
357357
groupDmConversationIcon: groupDmConversationIcon ?? this.groupDmConversationIcon,
358358
groupDmConversationIconBg: groupDmConversationIconBg ?? this.groupDmConversationIconBg,
359359
loginOrDivider: loginOrDivider ?? this.loginOrDivider,
@@ -375,10 +375,13 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
375375
}
376376
return DesignVariables._(
377377
background: Color.lerp(background, other.background, t)!,
378+
bannerBgIntDanger: Color.lerp(bannerBgIntDanger, other.bannerBgIntDanger, t)!,
378379
bgContextMenu: Color.lerp(bgContextMenu, other.bgContextMenu, t)!,
379380
bgCounterUnread: Color.lerp(bgCounterUnread, other.bgCounterUnread, t)!,
380381
bgTopBar: Color.lerp(bgTopBar, other.bgTopBar, t)!,
381382
borderBar: Color.lerp(borderBar, other.borderBar, t)!,
383+
btnLabelAttLowIntDanger: Color.lerp(btnLabelAttLowIntDanger, other.btnLabelAttLowIntDanger, t)!,
384+
btnLabelAttMediumDanger: Color.lerp(btnLabelAttMediumDanger, other.btnLabelAttMediumDanger, t)!,
382385
composeBoxBg: Color.lerp(composeBoxBg, other.composeBoxBg, t)!,
383386
contextMenuCancelText: Color.lerp(contextMenuCancelText, other.contextMenuCancelText, t)!,
384387
contextMenuItemBg: Color.lerp(contextMenuItemBg, other.contextMenuItemBg, t)!,
@@ -396,9 +399,6 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
396399
atMentionMarker: Color.lerp(atMentionMarker, other.atMentionMarker, t)!,
397400
contextMenuCancelBg: Color.lerp(contextMenuCancelBg, other.contextMenuCancelBg, t)!,
398401
dmHeaderBg: Color.lerp(dmHeaderBg, other.dmHeaderBg, t)!,
399-
errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!,
400-
errorBannerBorder: Color.lerp(errorBannerBorder, other.errorBannerBorder, t)!,
401-
errorBannerLabel: Color.lerp(errorBannerLabel, other.errorBannerLabel, t)!,
402402
groupDmConversationIcon: Color.lerp(groupDmConversationIcon, other.groupDmConversationIcon, t)!,
403403
groupDmConversationIconBg: Color.lerp(groupDmConversationIconBg, other.groupDmConversationIconBg, t)!,
404404
loginOrDivider: Color.lerp(loginOrDivider, other.loginOrDivider, t)!,

0 commit comments

Comments
 (0)