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

Star and unstar messages; show starred status #449

Merged
merged 3 commits into from
Jan 25, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file modified assets/icons/ZulipIcons.ttf
Binary file not shown.
5 changes: 5 additions & 0 deletions assets/icons/star_filled.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 16 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@
"@actionSheetOptionQuoteAndReply": {
"description": "Label for Quote and reply button on action sheet."
},
"actionSheetOptionStarMessage": "Star message",
"@actionSheetOptionStarMessage": {
"description": "Label for star button on action sheet."
},
"actionSheetOptionUnstarMessage": "Unstar message",
"@actionSheetOptionUnstarMessage": {
"description": "Label for unstar button on action sheet."
},
"errorCouldNotFetchMessageSource": "Could not fetch message source",
"@errorCouldNotFetchMessageSource": {
"description": "Error message when the source of a message could not be fetched."
Expand Down Expand Up @@ -128,6 +136,14 @@
"@errorSharingFailed": {
"description": "Error message when sharing a message failed."
},
"errorStarMessageFailedTitle": "Failed to star message",
"@errorStarMessageFailedTitle": {
"description": "Error title when starring a message failed."
},
"errorUnstarMessageFailedTitle": "Failed to unstar message",
"@errorUnstarMessageFailedTitle": {
"description": "Error title when unstarring a message failed."
},
"successLinkCopied": "Link copied",
"@successLinkCopied": {
"description": "Success message after copy link action completed."
Expand Down
50 changes: 50 additions & 0 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'clipboard.dart';
import 'compose_box.dart';
import 'dialog.dart';
import 'draggable_scrollable_modal_bottom_sheet.dart';
import 'icons.dart';
import 'message_list.dart';
import 'store.dart';

Expand Down Expand Up @@ -38,6 +39,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes
builder: (BuildContext _) {
return Column(children: [
if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context),
StarButton(message: message, messageListContext: context),
ShareButton(message: message, messageListContext: context),
if (isComposeBoxOffered) QuoteAndReplyButton(
message: message,
Expand Down Expand Up @@ -115,6 +117,54 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton {
};
}

class StarButton extends MessageActionSheetMenuItemButton {
StarButton({
super.key,
required super.message,
required super.messageListContext,
});

@override get icon => ZulipIcons.star_filled;

@override
String label(ZulipLocalizations zulipLocalizations) {
return message.flags.contains(MessageFlag.starred)
? zulipLocalizations.actionSheetOptionUnstarMessage
: zulipLocalizations.actionSheetOptionStarMessage;
}

@override get onPressed => (BuildContext context) async {
Navigator.of(context).pop();
final zulipLocalizations = ZulipLocalizations.of(messageListContext);
final op = message.flags.contains(MessageFlag.starred)
? UpdateMessageFlagsOp.remove
: UpdateMessageFlagsOp.add;

try {
final connection = PerAccountStoreWidget.of(messageListContext).connection;
await updateMessageFlags(connection, messages: [message.id],
op: op, flag: MessageFlag.starred);
} catch (e) {
if (!messageListContext.mounted) return;

String? errorMessage;
switch (e) {
case ZulipApiException():
errorMessage = e.message;
// TODO specific messages for common errors, like network errors
// (support with reusable code)
default:
}

await showErrorDialog(context: messageListContext,
title: switch(op) {
UpdateMessageFlagsOp.remove => zulipLocalizations.errorUnstarMessageFailedTitle,
UpdateMessageFlagsOp.add => zulipLocalizations.errorStarMessageFailedTitle,
}, message: errorMessage);
}
};
}

class ShareButton extends MessageActionSheetMenuItemButton {
ShareButton({
super.key,
Expand Down
9 changes: 6 additions & 3 deletions lib/widgets/icons.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,17 @@ abstract final class ZulipIcons {
/// The Zulip custom icon "read_receipts".
static const IconData read_receipts = IconData(0xf10b, fontFamily: "Zulip Icons");

/// The Zulip custom icon "star_filled".
static const IconData star_filled = IconData(0xf10c, fontFamily: "Zulip Icons");

/// The Zulip custom icon "topic".
static const IconData topic = IconData(0xf10c, fontFamily: "Zulip Icons");
static const IconData topic = IconData(0xf10d, fontFamily: "Zulip Icons");

/// The Zulip custom icon "unmute".
static const IconData unmute = IconData(0xf10d, fontFamily: "Zulip Icons");
static const IconData unmute = IconData(0xf10e, fontFamily: "Zulip Icons");

/// The Zulip custom icon "user".
static const IconData user = IconData(0xf10e, fontFamily: "Zulip Icons");
static const IconData user = IconData(0xf10f, fontFamily: "Zulip Icons");

// END GENERATED ICON DATA
}
Expand Down
11 changes: 10 additions & 1 deletion lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,8 @@ class MessageWithPossibleSender extends StatelessWidget {

final MessageListMessageItem item;

static final _starColor = const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor();

@override
Widget build(BuildContext context) {
final message = item.message;
Expand Down Expand Up @@ -913,7 +915,14 @@ class MessageWithPossibleSender extends StatelessWidget {
if ((message.reactions?.total ?? 0) > 0)
ReactionChipsList(messageId: message.id, reactions: message.reactions!)
])),
const SizedBox(width: 16),
SizedBox(width: 16,
child: message.flags.contains(MessageFlag.starred)
// TODO(#157): fix how star marker aligns with message content
// Design from Figma at:
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=813%3A28817&mode=dev .
? Padding(padding: const EdgeInsets.only(top: 4),
child: Icon(ZulipIcons.star_filled, size: 16, color: _starColor))
: null),
]),
])));
}
Expand Down
100 changes: 100 additions & 0 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:convert';

import 'package:checks/checks.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
Expand All @@ -7,10 +9,12 @@ import 'package:http/http.dart' as http;
import 'package:zulip/api/model/model.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/compose.dart';
import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/widgets/compose_box.dart';
import 'package:zulip/widgets/content.dart';
import 'package:zulip/widgets/icons.dart';
import 'package:zulip/widgets/message_list.dart';
import 'package:zulip/widgets/store.dart';
import 'package:share_plus_platform_interface/method_channel/method_channel_share.dart';
Expand Down Expand Up @@ -142,6 +146,102 @@ void main() {
});
});

group('StarButton', () {
Future<void> tapButton(WidgetTester tester) async {
// Starred messages include the same icon so we need to
// match only by descendants of [BottomSheet].
await tester.ensureVisible(find.descendant(
of: find.byType(BottomSheet),
matching: find.byIcon(ZulipIcons.star_filled, skipOffstage: false)));
await tester.tap(find.descendant(
of: find.byType(BottomSheet),
matching: find.byIcon(ZulipIcons.star_filled)));
await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e
}

testWidgets('star success', (WidgetTester tester) async {
final message = eg.streamMessage(flags: []);
await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

final connection = store.connection as FakeApiConnection;
connection.prepare(json: {});
await tapButton(tester);
await tester.pump(Duration.zero);

check(connection.lastRequest).isA<http.Request>()
..method.equals('POST')
..url.path.equals('/api/v1/messages/flags')
..bodyFields.deepEquals({
'messages': jsonEncode([message.id]),
'op': 'add',
'flag': 'starred',
});
});

testWidgets('unstar success', (WidgetTester tester) async {
final message = eg.streamMessage(flags: [MessageFlag.starred]);
await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);

final connection = store.connection as FakeApiConnection;
connection.prepare(json: {});
await tapButton(tester);
await tester.pump(Duration.zero);

check(connection.lastRequest).isA<http.Request>()
..method.equals('POST')
..url.path.equals('/api/v1/messages/flags')
..bodyFields.deepEquals({
'messages': jsonEncode([message.id]),
'op': 'remove',
'flag': 'starred',
});
});

testWidgets('star request has an error', (WidgetTester tester) async {
final message = eg.streamMessage(flags: []);
await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;

final connection = store.connection as FakeApiConnection;

connection.prepare(httpStatus: 400, json: {
'code': 'BAD_REQUEST',
'msg': 'Invalid message(s)',
'result': 'error',
});
await tapButton(tester);
await tester.pump(Duration.zero); // error arrives; error dialog shows

await tester.tap(find.byWidget(checkErrorDialog(tester,
expectedTitle: zulipLocalizations.errorStarMessageFailedTitle,
expectedMessage: 'Invalid message(s)')));
});

testWidgets('unstar request has an error', (WidgetTester tester) async {
final message = eg.streamMessage(flags: [MessageFlag.starred]);
await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message));
final store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;

final connection = store.connection as FakeApiConnection;

connection.prepare(httpStatus: 400, json: {
'code': 'BAD_REQUEST',
'msg': 'Invalid message(s)',
'result': 'error',
});
await tapButton(tester);
await tester.pump(Duration.zero); // error arrives; error dialog shows

await tester.tap(find.byWidget(checkErrorDialog(tester,
expectedTitle: zulipLocalizations.errorUnstarMessageFailedTitle,
expectedMessage: 'Invalid message(s)')));
});
});

group('ShareButton', () {
// Tests should call this.
MockSharePlus setupMockSharePlus() {
Expand Down
14 changes: 14 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,20 @@ void main() {
});
});

group('Starred messages', () {
testWidgets('unstarred message', (WidgetTester tester) async {
final message = eg.streamMessage(flags: []);
await setupMessageListPage(tester, messages: [message]);
check(find.byIcon(ZulipIcons.star_filled).evaluate()).isEmpty();
});

testWidgets('starred message', (WidgetTester tester) async {
final message = eg.streamMessage(flags: [MessageFlag.starred]);
await setupMessageListPage(tester, messages: [message]);
check(find.byIcon(ZulipIcons.star_filled).evaluate()).length.equals(1);
});
});

group('_UnreadMarker animations', () {
// TODO: Improve animation state testing so it is less tied to
// implementation details and more focused on output, see:
Expand Down