Skip to content

Commit

Permalink
compose_box: Replace compose box with a banner in DMs with deactivate…
Browse files Browse the repository at this point in the history 
…d users

Fixes: zulip#675

Co-authored-by: Rajesh Malviya <rajveer0malviya@gmail.com>
  • Loading branch information
@sm-sayedi @rajveermalviya
sm-sayedi and rajveermalviya committed Jul 27, 2024
1 parent f0046c3 commit 1d3feb8
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 20 deletions.
4 changes: 4 additions & 0 deletions assets/l10n/app_en.arb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@
"@successMessageLinkCopied": {
"description": "Message when link of a message was copied to the user's system clipboard."
},
"errorBannerDeactivatedDmLabel": "You cannot send messages to deactivated users.",
"@errorBannerDeactivatedDmLabel": {
"description": "Label text for error banner when sending a message to one or multiple deactivated users."
},
"composeBoxAttachFilesTooltip": "Attach files",
"@composeBoxAttachFilesTooltip": {
"description": "Tooltip for compose box icon to attach a file to the message."
Expand Down
44 changes: 42 additions & 2 deletions lib/widgets/compose_box.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import '../model/store.dart';
import 'autocomplete.dart';
import 'dialog.dart';
import 'store.dart';
import 'theme.dart';

const double _inputVerticalPadding = 8;
const double _sendButtonSize = 36;
Expand Down Expand Up @@ -850,11 +851,13 @@ class _ComposeBoxLayout extends StatelessWidget {
required this.sendButton,
required this.contentController,
required this.contentFocusNode,
this.blockingErrorBanner,
});

final Widget? topicInput;
final Widget contentInput;
final Widget sendButton;
final Widget? blockingErrorBanner;
final ComposeContentController contentController;
final FocusNode contentFocusNode;

Expand Down Expand Up @@ -883,7 +886,7 @@ class _ComposeBoxLayout extends StatelessWidget {
minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8),
child: Padding(
padding: const EdgeInsets.only(top: 8.0),
child: Column(children: [
child: blockingErrorBanner ?? Column(children: [
Row(crossAxisAlignment: CrossAxisAlignment.end, children: [
Expanded(
child: Theme(
Expand Down Expand Up @@ -973,6 +976,29 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
}
}

class _ErrorBanner extends StatelessWidget {
const _ErrorBanner({required this.label});

final String label;

@override
Widget build(BuildContext context) {
final designVariables = DesignVariables.of(context);
return Container(
padding: const EdgeInsets.all(8),
decoration: BoxDecoration(
color: designVariables.errorBannerBackground,
border: Border.all(color: designVariables.errorBannerBorder),
borderRadius: BorderRadius.circular(5)),
child: Text(label,
maxLines: 2,
overflow: TextOverflow.ellipsis,
style: TextStyle(fontSize: 18, color: designVariables.errorBannerLabel),
),
);
}
}

class _FixedDestinationComposeBox extends StatefulWidget {
const _FixedDestinationComposeBox({super.key, required this.narrow});

Expand All @@ -998,6 +1024,19 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
super.dispose();
}

Widget? _errorBanner(BuildContext context) {
if (widget.narrow case DmNarrow(:final otherRecipientIds)) {
final store = PerAccountStoreWidget.of(context);
final hasDeactivatedUser = otherRecipientIds.any((id) =>
!(store.users[id]?.isActive ?? true));
if (hasDeactivatedUser) {
return _ErrorBanner(label: ZulipLocalizations.of(context)
.errorBannerDeactivatedDmLabel);
}
}
return null;
}

@override
Widget build(BuildContext context) {
return _ComposeBoxLayout(
Expand All @@ -1013,7 +1052,8 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
topicController: null,
contentController: _contentController,
getDestination: () => widget.narrow.destination,
));
),
blockingErrorBanner: _errorBanner(context));
}
}

Expand Down
37 changes: 19 additions & 18 deletions lib/widgets/message_list.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,24 +117,25 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=147%3A9088&mode=dev
body: Builder(
builder: (BuildContext context) => Center(
child: Column(children: [
MediaQuery.removePadding(
// Scaffold knows about the app bar, and so has run this
// BuildContext, which is under `body`, through
// MediaQuery.removePadding with `removeTop: true`.
context: context,

// The compose box, when present, pads the bottom inset.
// TODO this copies the details of when the compose box is shown;
// if those details get complicated, refactor to avoid copying.
// TODO(#311) If we have a bottom nav, it will pad the bottom
// inset, and this should always be true.
removeBottom: widget.narrow is! CombinedFeedNarrow,

child: Expanded(
child: MessageList(narrow: widget.narrow))),
ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow),
]))));
child: Column(crossAxisAlignment: CrossAxisAlignment.stretch,
children: [
MediaQuery.removePadding(
// Scaffold knows about the app bar, and so has run this
// BuildContext, which is under `body`, through
// MediaQuery.removePadding with `removeTop: true`.
context: context,

// The compose box, when present, pads the bottom inset.
// TODO this copies the details of when the compose box is shown;
// if those details get complicated, refactor to avoid copying.
// TODO(#311) If we have a bottom nav, it will pad the bottom
// inset, and this should always be true.
removeBottom: widget.narrow is! CombinedFeedNarrow,

child: Expanded(
child: MessageList(narrow: widget.narrow))),
ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow),
]))));
}
}

Expand Down
21 changes: 21 additions & 0 deletions lib/widgets/theme.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
streamColorSwatches: StreamColorSwatches.light,
star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
errorBannerLabel: const Color.fromRGBO(133, 42, 35, 1),
errorBannerBackground: const Color.fromRGBO(238, 222, 221, 1),
errorBannerBorder: const Color.fromRGBO(132, 41, 36, 0.4),
);

DesignVariables.dark() :
Expand All @@ -142,6 +145,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
// TODO(#95) need dark-theme color
editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
errorBannerLabel: const Color.fromRGBO(241, 170, 167, 1),
errorBannerBackground: const Color.fromRGBO(78, 19, 19, 1),
errorBannerBorder: const Color.fromRGBO(237, 145, 140, 0.4),
);

DesignVariables._({
Expand All @@ -153,6 +159,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
required this.streamColorSwatches,
required this.star,
required this.editedMovedMarkerCollapsed,
required this.errorBannerLabel,
required this.errorBannerBackground,
required this.errorBannerBorder,
});

/// The [DesignVariables] from the context's active theme.
Expand All @@ -177,6 +186,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
// Not named variables in Figma; taken from older Figma drafts, or elsewhere.
final Color star;
final Color editedMovedMarkerCollapsed;
final Color errorBannerLabel;
final Color errorBannerBackground;
final Color errorBannerBorder;

@override
DesignVariables copyWith({
Expand All @@ -188,6 +200,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
StreamColorSwatches? streamColorSwatches,
Color? star,
Color? editedMovedMarkerCollapsed,
Color? errorBannerLabel,
Color? errorBannerBackground,
Color? errorBannerBorder,
}) {
return DesignVariables._(
bgTopBar: bgTopBar ?? this.bgTopBar,
Expand All @@ -198,6 +213,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches,
star: star ?? this.star,
editedMovedMarkerCollapsed: editedMovedMarkerCollapsed ?? this.editedMovedMarkerCollapsed,
errorBannerLabel: errorBannerLabel ?? this.errorBannerLabel,
errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground,
errorBannerBorder: errorBannerBorder ?? this.errorBannerBorder,
);
}

Expand All @@ -215,6 +233,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
streamColorSwatches: StreamColorSwatches.lerp(streamColorSwatches, other.streamColorSwatches, t),
star: Color.lerp(star, other.star, t)!,
editedMovedMarkerCollapsed: Color.lerp(editedMovedMarkerCollapsed, other.editedMovedMarkerCollapsed, t)!,
errorBannerLabel: Color.lerp(errorBannerLabel, other.errorBannerLabel, t)!,
errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!,
errorBannerBorder: Color.lerp(errorBannerBorder, other.errorBannerBorder, t)!,
);
}
}
Expand Down
109 changes: 109 additions & 0 deletions test/widgets/compose_box_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:http/http.dart' as http;
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:image_picker/image_picker.dart';
import 'package:zulip/api/model/events.dart';
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/localizations.dart';
Expand Down Expand Up @@ -366,4 +367,112 @@ void main() {
// TODO test what happens when capturing/uploading fails
});
});

group('compose box in DMs with deactivated users', () {
Finder contentFieldFinder() => find.descendant(
of: find.byType(ComposeBox),
matching: find.byType(TextField));

Finder attachButtonFinder(IconData icon) => find.descendant(
of: find.byType(ComposeBox),
matching: find.widgetWithIcon(IconButton, icon));

void checkComposeBoxParts({required bool areShown}) {
check(contentFieldFinder().evaluate().length).equals(areShown ? 1 : 0);
check(attachButtonFinder(Icons.attach_file).evaluate().length).equals(areShown ? 1 : 0);
check(attachButtonFinder(Icons.image).evaluate().length).equals(areShown ? 1 : 0);
check(attachButtonFinder(Icons.camera_alt).evaluate().length).equals(areShown ? 1 : 0);
}

void checkBanner({required bool isShown}) {
final bannerTextFinder = find.text(GlobalLocalizations.zulipLocalizations
.errorBannerDeactivatedDmLabel);
check(bannerTextFinder.evaluate().length).equals(isShown ? 1 : 0);
}

void checkComposeBox({required bool isShown}) {
checkComposeBoxParts(areShown: isShown);
checkBanner(isShown: !isShown);
}

Future<void> changeUserStatus(WidgetTester tester,
{required User user, required bool isActive}) async {
await store.handleEvent(RealmUserUpdateEvent(id: 1,
userId: user.userId, isActive: isActive));
await tester.pump();
}

final selfUser = eg.selfUser;

DmNarrow dmNarrowWith(User otherUser) => DmNarrow.withUser(otherUser.userId,
selfUserId: selfUser.userId);

DmNarrow groupDmNarrowWith(List<User> otherUsers) => DmNarrow.withOtherUsers(
otherUsers.map((u) => u.userId), selfUserId: selfUser.userId);

group('1:1 DMs', () {
testWidgets('compose box replaced with a banner', (tester) async {
final deactivatedUser = eg.user(isActive: false);
await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser),
users: [deactivatedUser]);
checkComposeBox(isShown: false);
});

testWidgets('active user becomes deactivated -> '
'compose box is replaced with a banner', (tester) async {
final activeUser = eg.user(isActive: true);
await prepareComposeBox(tester, narrow: dmNarrowWith(activeUser),
users: [activeUser]);
checkComposeBox(isShown: true);

await changeUserStatus(tester, user: activeUser, isActive: false);
checkComposeBox(isShown: false);
});

testWidgets('deactivated user becomes active -> '
'banner is replaced with the compose box', (tester) async {
final deactivatedUser = eg.user(isActive: false);
await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser),
users: [deactivatedUser]);
checkComposeBox(isShown: false);

await changeUserStatus(tester, user: deactivatedUser, isActive: true);
checkComposeBox(isShown: true);
});
});

group('group DMs', () {
testWidgets('compose box replaced with a banner', (tester) async {
final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)];
await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers),
users: deactivatedUsers);
checkComposeBox(isShown: false);
});

testWidgets('at least one user becomes deactivated -> '
'compose box is replaced with a banner', (tester) async {
final activeUsers = [eg.user(isActive: true), eg.user(isActive: true)];
await prepareComposeBox(tester, narrow: groupDmNarrowWith(activeUsers),
users: activeUsers);
checkComposeBox(isShown: true);

await changeUserStatus(tester, user: activeUsers[0], isActive: false);
checkComposeBox(isShown: false);
});

testWidgets('all deactivated users become active -> '
'banner is replaced with the compose box', (tester) async {
final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)];
await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers),
users: deactivatedUsers);
checkComposeBox(isShown: false);

await changeUserStatus(tester, user: deactivatedUsers[0], isActive: true);
checkComposeBox(isShown: false);

await changeUserStatus(tester, user: deactivatedUsers[1], isActive: true);
checkComposeBox(isShown: true);
});
});
});
}

0 comments on commit 1d3feb8

Please sign in to comment.