Skip to content

Commit a9b5745

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 5d3a6d6 commit a9b5745

File tree

1 file changed

+47
-79
lines changed

1 file changed

+47
-79
lines changed

lib/widgets/emoji_reaction.dart

Lines changed: 47 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -755,32 +755,18 @@ class _ViewReactionsState extends State<ViewReactions> with PerAccountStoreAware
755755

756756
@override
757757
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-
]);
758+
return DraggableScrollableModalBottomSheet(
759+
header: ViewReactionsHeader(
760+
messageId: widget.messageId,
761+
reactionType: reactionType,
762+
emojiCode: emojiCode,
763+
onRequestSelect: _setSelection,
764+
),
765+
contentSliver: ViewReactionsUserListSliver(
766+
messageId: widget.messageId,
767+
reactionType: reactionType,
768+
emojiCode: emojiCode,
769+
emojiName: emojiName));
784770
}
785771
}
786772

@@ -830,26 +816,27 @@ class ViewReactionsHeader extends StatelessWidget {
830816
padding: const EdgeInsets.only(top: 16, bottom: 4),
831817
child: InsetShadowBox(start: 8, end: 8,
832818
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()))))));
819+
child: Center(
820+
child: SingleChildScrollView(
821+
// TODO(upstream) we want to pass excludeFromSemantics: true
822+
// to the underlying Scrollable to remove an unwanted node
823+
// in accessibility focus traversal when there are many items.
824+
scrollDirection: Axis.horizontal,
825+
child: Padding(
826+
padding: const EdgeInsets.symmetric(horizontal: 8),
827+
child: Semantics(
828+
role: SemanticsRole.tabBar,
829+
container: true,
830+
explicitChildNodes: true,
831+
label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total),
832+
child: Row(
833+
children: reactions.aggregated.mapIndexed((i, r) =>
834+
_ViewReactionsEmojiItem(
835+
reactionWithVotes: r,
836+
position: _emojiItemPosition(i, reactions.aggregated.length),
837+
selected: r.reactionType == reactionType && r.emojiCode == emojiCode,
838+
onRequestSelect: onRequestSelect),
839+
).toList())))))));
853840
}
854841
}
855842

@@ -957,7 +944,7 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
957944

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

962949
selected: selected,
963950
label: zulipLocalizations.seeWhoReactedSheetEmojiNameWithVoteCount(emojiName, count),
@@ -967,10 +954,9 @@ class _ViewReactionsEmojiItem extends StatelessWidget {
967954
}
968955
}
969956

970-
971957
@visibleForTesting
972-
class ViewReactionsUserList extends StatelessWidget {
973-
const ViewReactionsUserList({
958+
class ViewReactionsUserListSliver extends StatelessWidget {
959+
const ViewReactionsUserListSliver({
974960
super.key,
975961
required this.messageId,
976962
required this.reactionType,
@@ -979,17 +965,16 @@ class ViewReactionsUserList extends StatelessWidget {
979965
});
980966

981967
final int messageId;
982-
final ReactionType reactionType;
983-
final String emojiCode;
984-
final String emojiName;
968+
final ReactionType? reactionType;
969+
final String? emojiCode;
970+
final String? emojiName;
985971

986972
static const semanticsIdentifier = 'view-reactions-user-list';
987973

988974
@override
989975
Widget build(BuildContext context) {
990976
final zulipLocalizations = ZulipLocalizations.of(context);
991977
final store = PerAccountStoreWidget.of(context);
992-
final designVariables = DesignVariables.of(context);
993978

994979
final message = store.messages[messageId];
995980

@@ -1002,38 +987,21 @@ class ViewReactionsUserList extends StatelessWidget {
1002987

1003988
if (userIds == null) {
1004989
// This reaction lost all its votes, or the message was deleted.
1005-
return SizedBox.shrink();
990+
return SliverPadding(padding: EdgeInsets.zero);
1006991
}
992+
assert(emojiName != null);
1007993

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]))));
994+
Widget result = SliverList.builder(
995+
itemCount: userIds.length,
996+
itemBuilder: (_, index) => ViewReactionsUserItem(userId: userIds[index]));
1029997

1030-
return Semantics(
998+
return SliverSemantics(
1031999
identifier: semanticsIdentifier, // See note on `controlsNodes` on the tab.
1032-
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName, userIds.length),
1000+
label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName!, userIds.length),
10331001
role: SemanticsRole.tabPanel,
10341002
container: true,
10351003
explicitChildNodes: true,
1036-
child: result);
1004+
sliver: result);
10371005
}
10381006
}
10391007

0 commit comments

Comments
 (0)