Skip to content

Commit e32e34f

Browse files
committed
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 5115398 commit e32e34f

File tree

1 file changed

+62
-117
lines changed

1 file changed

+62
-117
lines changed

lib/widgets/emoji_reaction.dart

Lines changed: 62 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,8 @@ class ViewReactions extends StatefulWidget {
682682
final ReactionType? initialReactionType;
683683
final String? initialEmojiCode;
684684

685+
static String semanticsIdentifier = 'view-reactions-user-list';
686+
685687
@override
686688
State<ViewReactions> createState() => _ViewReactionsState();
687689
}
@@ -755,32 +757,44 @@ class _ViewReactionsState extends State<ViewReactions> with PerAccountStoreAware
755757

756758
@override
757759
Widget build(BuildContext context) {
758-
// TODO could pull out this layout/appearance code,
759-
// focusing this widget only on state management
760-
return Column(
761-
mainAxisSize: MainAxisSize.min,
762-
crossAxisAlignment: CrossAxisAlignment.center,
763-
children: [
764-
ViewReactionsHeader(
765-
messageId: widget.messageId,
766-
reactionType: reactionType,
767-
emojiCode: emojiCode,
768-
onRequestSelect: _setSelection,
769-
),
770-
// TODO if all reactions (or whole message) disappeared,
771-
// we show a message saying there are no reactions,
772-
// but the layout shifts (the sheet's height changes dramatically);
773-
// we should avoid this.
774-
if (reactionType != null && emojiCode != null) Flexible(
775-
child: ViewReactionsUserList(
776-
messageId: widget.messageId,
777-
reactionType: reactionType!,
778-
emojiCode: emojiCode!,
779-
emojiName: emojiName!)),
780-
Padding(
781-
padding: const EdgeInsets.symmetric(horizontal: 16),
782-
child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close))
783-
]);
760+
final zulipLocalizations = ZulipLocalizations.of(context);
761+
final message = PerAccountStoreWidget.of(context).messages[widget.messageId];
762+
763+
final userIds = message?.reactions?.aggregated.firstWhereOrNull(
764+
(x) => x.reactionType == reactionType && x.emojiCode == emojiCode
765+
)?.userIds.toList();
766+
767+
Widget contentSliver;
768+
if (reactionType != null && emojiCode != null && userIds != null) {
769+
assert(emojiName != null);
770+
771+
// (No filtering of muted or deactivated users.
772+
// Muted users will be shown as muted.)
773+
774+
contentSliver = SliverList.builder(
775+
itemCount: userIds.length,
776+
itemBuilder: (_, index) => ViewReactionsUserItem(userId: userIds[index]));
777+
778+
contentSliver = SliverSemantics(
779+
identifier: ViewReactions.semanticsIdentifier, // See note on `controlsNodes` on the tab.
780+
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName!, userIds.length),
781+
role: SemanticsRole.tabPanel,
782+
container: true,
783+
explicitChildNodes: true,
784+
sliver: contentSliver);
785+
} else {
786+
// (Not expected; see _reconcile; but at least we shouldn't crash.)
787+
contentSliver = SliverPadding(padding: EdgeInsets.zero);
788+
}
789+
790+
return DraggableScrollableModalBottomSheet(
791+
header: ViewReactionsHeader(
792+
messageId: widget.messageId,
793+
reactionType: reactionType,
794+
emojiCode: emojiCode,
795+
onRequestSelect: _setSelection,
796+
),
797+
contentSliver: contentSliver);
784798
}
785799
}
786800

@@ -830,26 +844,27 @@ class ViewReactionsHeader extends StatelessWidget {
830844
padding: const EdgeInsets.only(top: 16, bottom: 4),
831845
child: InsetShadowBox(start: 8, end: 8,
832846
color: designVariables.bgContextMenu,
833-
child: SingleChildScrollView(
834-
// TODO(upstream) we want to pass excludeFromSemantics: true
835-
// to the underlying Scrollable to remove an unwanted node
836-
// in accessibility focus traversal when there are many items.
837-
scrollDirection: Axis.horizontal,
838-
child: Padding(
839-
padding: const EdgeInsets.symmetric(horizontal: 8),
840-
child: Semantics(
841-
role: SemanticsRole.tabBar,
842-
container: true,
843-
explicitChildNodes: true,
844-
label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total),
845-
child: Row(
846-
children: reactions.aggregated.mapIndexed((i, r) =>
847-
_ViewReactionsEmojiItem(
848-
reactionWithVotes: r,
849-
position: _emojiItemPosition(i, reactions.aggregated.length),
850-
selected: r.reactionType == reactionType && r.emojiCode == emojiCode,
851-
onRequestSelect: onRequestSelect),
852-
).toList()))))));
847+
child: Center(
848+
child: SingleChildScrollView(
849+
// TODO(upstream) we want to pass excludeFromSemantics: true
850+
// to the underlying Scrollable to remove an unwanted node
851+
// in accessibility focus traversal when there are many items.
852+
scrollDirection: Axis.horizontal,
853+
child: Padding(
854+
padding: const EdgeInsets.symmetric(horizontal: 8),
855+
child: Semantics(
856+
role: SemanticsRole.tabBar,
857+
container: true,
858+
explicitChildNodes: true,
859+
label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total),
860+
child: Row(
861+
children: reactions.aggregated.mapIndexed((i, r) =>
862+
_ViewReactionsEmojiItem(
863+
reactionWithVotes: r,
864+
position: _emojiItemPosition(i, reactions.aggregated.length),
865+
selected: r.reactionType == reactionType && r.emojiCode == emojiCode,
866+
onRequestSelect: onRequestSelect),
867+
).toList())))))));
853868
}
854869
}
855870

@@ -957,7 +972,7 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
957972

958973
// I *think* we're following the doc with this but it's hard to tell;
959974
// I've only tested on iOS and I didn't notice a behavior change.
960-
controlsNodes: {ViewReactionsUserList.semanticsIdentifier},
975+
controlsNodes: {ViewReactions.semanticsIdentifier},
961976

962977
selected: selected,
963978
label: zulipLocalizations.seeWhoReactedSheetEmojiNameWithVoteCount(emojiName, count),
@@ -967,76 +982,6 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
967982
}
968983
}
969984

970-
971-
@visibleForTesting
972-
class ViewReactionsUserList extends StatelessWidget {
973-
const ViewReactionsUserList({
974-
super.key,
975-
required this.messageId,
976-
required this.reactionType,
977-
required this.emojiCode,
978-
required this.emojiName,
979-
});
980-
981-
final int messageId;
982-
final ReactionType reactionType;
983-
final String emojiCode;
984-
final String emojiName;
985-
986-
static const semanticsIdentifier = 'view-reactions-user-list';
987-
988-
@override
989-
Widget build(BuildContext context) {
990-
final zulipLocalizations = ZulipLocalizations.of(context);
991-
final store = PerAccountStoreWidget.of(context);
992-
final designVariables = DesignVariables.of(context);
993-
994-
final message = store.messages[messageId];
995-
996-
final userIds = message?.reactions?.aggregated.firstWhereOrNull(
997-
(x) => x.reactionType == reactionType && x.emojiCode == emojiCode
998-
)?.userIds.toList();
999-
1000-
// (No filtering of muted or deactivated users.
1001-
// Muted users will be shown as muted.)
1002-
1003-
if (userIds == null) {
1004-
// This reaction lost all its votes, or the message was deleted.
1005-
return SizedBox.shrink();
1006-
}
1007-
1008-
Widget result = SizedBox(
1009-
height: 400, // TODO(design) tune
1010-
child: InsetShadowBox(
1011-
top: 8,
1012-
bottom: 8,
1013-
color: designVariables.bgContextMenu,
1014-
// TODO(upstream) we want to pass excludeFromSemantics: true
1015-
// to the underlying Scrollable to remove an unwanted node
1016-
// in accessibility focus traversal when there are many items.
1017-
child: ListView.builder(
1018-
padding: EdgeInsets.only(
1019-
// The Figma excludes the 8px top padding, which is unusual with the
1020-
// shadow effect (our InsetShadowBox). We include it so that the
1021-
// first item's touch feedback is shadow-free in the item's initial/
1022-
// scrolled-to-top position.
1023-
top: 8,
1024-
bottom: 8,
1025-
),
1026-
itemCount: userIds.length,
1027-
itemBuilder: (_, index) =>
1028-
ViewReactionsUserItem(userId: userIds[index]))));
1029-
1030-
return Semantics(
1031-
identifier: semanticsIdentifier, // See note on `controlsNodes` on the tab.
1032-
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName, userIds.length),
1033-
role: SemanticsRole.tabPanel,
1034-
container: true,
1035-
explicitChildNodes: true,
1036-
child: result);
1037-
}
1038-
}
1039-
1040985
// TODO: deduplicate the code with [ReadReceiptsUserItem]
1041986
@visibleForTesting
1042987
class ViewReactionsUserItem extends StatelessWidget {

0 commit comments

Comments
 (0)