Skip to content

Commit d9ab075

Browse files
chrisbobbegnprice
authored andcommitted
emoji_reaction: Use DraggableScrollableModalBottomSheet for view-reactions
Like we did for read-receipts in a recent commit. I *think* this behavior is implied in the Figma, but it's not as explicit: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=5878-19207&m=dev ...anyway, helpful to be consistent with read-receipts, I think, which is similarly a read-only view that might have a long list to show. I thought this would be more complicated than it turned out to be -- thanks to SliverSemantics, I can actually wrap the list of voters in a labeled container node, because DraggableScrollableModalBottomSheet's API takes a sliver for the content.
1 parent 49a5a0d commit d9ab075

File tree

1 file changed

+57
-80
lines changed

1 file changed

+57
-80
lines changed

lib/widgets/emoji_reaction.dart

Lines changed: 57 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -753,32 +753,18 @@ class _ViewReactionsState extends State<ViewReactions> with PerAccountStoreAware
753753

754754
@override
755755
Widget build(BuildContext context) {
756-
// TODO could pull out this layout/appearance code,
757-
// focusing this widget only on state management
758-
return Column(
759-
mainAxisSize: MainAxisSize.min,
760-
crossAxisAlignment: CrossAxisAlignment.center,
761-
children: [
762-
ViewReactionsHeader(
763-
messageId: widget.messageId,
764-
reactionType: reactionType,
765-
emojiCode: emojiCode,
766-
onRequestSelect: _setSelection,
767-
),
768-
// TODO if all reactions (or whole message) disappeared,
769-
// we show a message saying there are no reactions,
770-
// but the layout shifts (the sheet's height changes dramatically);
771-
// we should avoid this.
772-
if (reactionType != null && emojiCode != null) Flexible(
773-
child: ViewReactionsUserList(
774-
messageId: widget.messageId,
775-
reactionType: reactionType!,
776-
emojiCode: emojiCode!,
777-
emojiName: emojiName!)),
778-
Padding(
779-
padding: const EdgeInsets.symmetric(horizontal: 16),
780-
child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close))
781-
]);
756+
return DraggableScrollableModalBottomSheet(
757+
header: ViewReactionsHeader(
758+
messageId: widget.messageId,
759+
reactionType: reactionType,
760+
emojiCode: emojiCode,
761+
onRequestSelect: _setSelection,
762+
),
763+
contentSliver: ViewReactionsUserListSliver(
764+
messageId: widget.messageId,
765+
reactionType: reactionType,
766+
emojiCode: emojiCode,
767+
emojiName: emojiName));
782768
}
783769
}
784770

@@ -828,26 +814,27 @@ class ViewReactionsHeader extends StatelessWidget {
828814
padding: const EdgeInsets.only(top: 16, bottom: 4),
829815
child: InsetShadowBox(start: 8, end: 8,
830816
color: designVariables.bgContextMenu,
831-
child: SingleChildScrollView(
832-
// TODO(upstream) we want to pass excludeFromSemantics: true
833-
// to the underlying Scrollable to remove an unwanted node
834-
// in accessibility focus traversal when there are many items.
835-
scrollDirection: Axis.horizontal,
836-
child: Padding(
837-
padding: const EdgeInsets.symmetric(horizontal: 8),
838-
child: Semantics(
839-
role: SemanticsRole.tabBar,
840-
container: true,
841-
explicitChildNodes: true,
842-
label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total),
843-
child: Row(
844-
children: reactions.aggregated.mapIndexed((i, r) =>
845-
_ViewReactionsEmojiItem(
846-
reactionWithVotes: r,
847-
position: _emojiItemPosition(i, reactions.aggregated.length),
848-
selected: r.reactionType == reactionType && r.emojiCode == emojiCode,
849-
onRequestSelect: onRequestSelect),
850-
).toList()))))));
817+
child: Center(
818+
child: SingleChildScrollView(
819+
// TODO(upstream) we want to pass excludeFromSemantics: true
820+
// to the underlying Scrollable to remove an unwanted node
821+
// in accessibility focus traversal when there are many items.
822+
scrollDirection: Axis.horizontal,
823+
child: Padding(
824+
padding: const EdgeInsets.symmetric(horizontal: 8),
825+
child: Semantics(
826+
role: SemanticsRole.tabBar,
827+
container: true,
828+
explicitChildNodes: true,
829+
label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total),
830+
child: Row(
831+
children: reactions.aggregated.mapIndexed((i, r) =>
832+
_ViewReactionsEmojiItem(
833+
reactionWithVotes: r,
834+
position: _emojiItemPosition(i, reactions.aggregated.length),
835+
selected: r.reactionType == reactionType && r.emojiCode == emojiCode,
836+
onRequestSelect: onRequestSelect),
837+
).toList())))))));
851838
}
852839
}
853840

@@ -955,7 +942,7 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
955942

956943
// I *think* we're following the doc with this but it's hard to tell;
957944
// I've only tested on iOS and I didn't notice a behavior change.
958-
controlsNodes: {ViewReactionsUserList.semanticsIdentifier},
945+
controlsNodes: {ViewReactionsUserListSliver.semanticsIdentifier},
959946

960947
selected: selected,
961948
label: zulipLocalizations.seeWhoReactedSheetEmojiNameWithVoteCount(emojiName, count),
@@ -965,10 +952,9 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
965952
}
966953
}
967954

968-
969955
@visibleForTesting
970-
class ViewReactionsUserList extends StatelessWidget {
971-
const ViewReactionsUserList({
956+
class ViewReactionsUserListSliver extends StatelessWidget {
957+
const ViewReactionsUserListSliver({
972958
super.key,
973959
required this.messageId,
974960
required this.reactionType,
@@ -977,17 +963,25 @@ class ViewReactionsUserList extends StatelessWidget {
977963
});
978964

979965
final int messageId;
980-
final ReactionType reactionType;
981-
final String emojiCode;
982-
final String emojiName;
966+
final ReactionType? reactionType;
967+
final String? emojiCode;
968+
final String? emojiName;
983969

984970
static const semanticsIdentifier = 'view-reactions-user-list';
985971

986972
@override
987973
Widget build(BuildContext context) {
988974
final zulipLocalizations = ZulipLocalizations.of(context);
989975
final store = PerAccountStoreWidget.of(context);
990-
final designVariables = DesignVariables.of(context);
976+
977+
if (reactionType == null || emojiCode == null) {
978+
// The emoji selection was cleared,
979+
// which happens when the message is deleted or loses all its reactions.
980+
// The sheet's header will have a message like
981+
// "This message has no reactions."
982+
return SliverPadding(padding: EdgeInsets.zero);
983+
}
984+
assert(emojiName != null);
991985

992986
final message = store.messages[messageId];
993987

@@ -999,39 +993,22 @@ class ViewReactionsUserList extends StatelessWidget {
999993
// Muted users will be shown as muted.)
1000994

1001995
if (userIds == null) {
1002-
// This reaction lost all its votes, or the message was deleted.
1003-
return SizedBox.shrink();
996+
// The selected emoji lost all its votes. This won't show long if at all;
997+
// a different emoji will be automatically selected if there is one.
998+
return SliverPadding(padding: EdgeInsets.zero);
1004999
}
10051000

1006-
Widget result = SizedBox(
1007-
height: 400, // TODO(design) tune
1008-
child: InsetShadowBox(
1009-
top: 8,
1010-
bottom: 8,
1011-
color: designVariables.bgContextMenu,
1012-
// TODO(upstream) we want to pass excludeFromSemantics: true
1013-
// to the underlying Scrollable to remove an unwanted node
1014-
// in accessibility focus traversal when there are many items.
1015-
child: ListView.builder(
1016-
padding: EdgeInsets.only(
1017-
// The Figma excludes the 8px top padding, which is unusual with the
1018-
// shadow effect (our InsetShadowBox). We include it so that the
1019-
// first item's touch feedback is shadow-free in the item's initial/
1020-
// scrolled-to-top position.
1021-
top: 8,
1022-
bottom: 8,
1023-
),
1024-
itemCount: userIds.length,
1025-
itemBuilder: (_, index) =>
1026-
ViewReactionsUserItem(userId: userIds[index]))));
1001+
Widget result = SliverList.builder(
1002+
itemCount: userIds.length,
1003+
itemBuilder: (_, index) => ViewReactionsUserItem(userId: userIds[index]));
10271004

1028-
return Semantics(
1005+
return SliverSemantics(
10291006
identifier: semanticsIdentifier, // See note on `controlsNodes` on the tab.
1030-
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName, userIds.length),
1007+
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName!, userIds.length),
10311008
role: SemanticsRole.tabPanel,
10321009
container: true,
10331010
explicitChildNodes: true,
1034-
child: result);
1011+
sliver: result);
10351012
}
10361013
}
10371014

0 commit comments

Comments
 (0)