From 9884df7229eaa0061a781fa772f13c6e53952d16 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Aug 2025 18:32:23 -0700 Subject: [PATCH 1/6] inset_shadow [nfc]: Bring out a public helper, to use soon --- lib/widgets/inset_shadow.dart | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/widgets/inset_shadow.dart b/lib/widgets/inset_shadow.dart index 3c2533f1d8..fe72d9ead6 100644 --- a/lib/widgets/inset_shadow.dart +++ b/lib/widgets/inset_shadow.dart @@ -48,12 +48,6 @@ class InsetShadowBox extends StatelessWidget { final Widget child; - BoxDecoration _shadowFrom(AlignmentGeometry begin) { - return BoxDecoration(gradient: LinearGradient( - begin: begin, end: -begin, - colors: [color, color.withValues(alpha: 0)])); - } - @override Widget build(BuildContext context) { return Stack( @@ -63,13 +57,32 @@ class InsetShadowBox extends StatelessWidget { children: [ child, if (top != 0) Positioned(top: 0, height: top, left: 0, right: 0, - child: DecoratedBox(decoration: _shadowFrom(Alignment.topCenter))), + child: DecoratedBox( + decoration: fadeToTransparencyDecoration(FadeToTransparencyDirection.down, color))), if (bottom != 0) Positioned(bottom: 0, height: bottom, left: 0, right: 0, - child: DecoratedBox(decoration: _shadowFrom(Alignment.bottomCenter))), + child: DecoratedBox( + decoration: fadeToTransparencyDecoration(FadeToTransparencyDirection.up, color))), if (start != 0) PositionedDirectional(start: 0, width: start, top: 0, bottom: 0, - child: DecoratedBox(decoration: _shadowFrom(AlignmentDirectional.centerStart))), + child: DecoratedBox( + decoration: fadeToTransparencyDecoration(FadeToTransparencyDirection.end, color))), if (end != 0) PositionedDirectional(end: 0, width: end, top: 0, bottom: 0, - child: DecoratedBox(decoration: _shadowFrom(AlignmentDirectional.centerEnd))), + child: DecoratedBox( + decoration: fadeToTransparencyDecoration(FadeToTransparencyDirection.start, color))), ]); } } + +enum FadeToTransparencyDirection { down, up, end, start } + +BoxDecoration fadeToTransparencyDecoration(FadeToTransparencyDirection direction, Color color) { + final begin = switch (direction) { + FadeToTransparencyDirection.down => Alignment.topCenter, + FadeToTransparencyDirection.up => Alignment.bottomCenter, + FadeToTransparencyDirection.end => AlignmentDirectional.centerStart, + FadeToTransparencyDirection.start => AlignmentDirectional.centerEnd, + }; + + return BoxDecoration(gradient: LinearGradient( + begin: begin, end: -begin, + colors: [color, color.withValues(alpha: 0)])); +} From 4c057353daa36aa2706a7cc9361ad57ef369a350 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Aug 2025 19:05:13 -0700 Subject: [PATCH 2/6] code [nfc]: s/localizations/zulipLocalizations/g There are other localization classes, like MaterialLocalizations, and in any case it seems right to have a consistent naming pattern. --- lib/model/store.dart | 6 +++--- lib/widgets/action_sheet.dart | 8 ++++---- lib/widgets/app.dart | 6 +++--- lib/widgets/autocomplete.dart | 20 ++++++++++---------- lib/widgets/profile.dart | 8 ++++---- lib/widgets/read_receipts.dart | 6 +++--- lib/widgets/set_status.dart | 30 +++++++++++++++--------------- test/widgets/profile_test.dart | 8 ++++---- 8 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lib/model/store.dart b/lib/model/store.dart index e74a22c623..c240d47920 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -1460,10 +1460,10 @@ class UpdateMachine { } void _reportToUserErrorConnectingToServer(Object error) { - final localizations = GlobalLocalizations.zulipLocalizations; + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; reportErrorToUserBriefly( - localizations.errorConnectingToServerShort, - details: localizations.errorConnectingToServerDetails( + zulipLocalizations.errorConnectingToServerShort, + details: zulipLocalizations.errorConnectingToServerDetails( store.realmUrl.toString(), error.toString())); } diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 56729e8bfc..06cf84bdbc 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -504,11 +504,11 @@ class CopyChannelLinkButton extends ActionSheetMenuItemButton { @override void onPressed() async { - final localizations = ZulipLocalizations.of(pageContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final store = PerAccountStoreWidget.of(pageContext); PlatformActions.copyWithPopup(context: pageContext, - successContent: Text(localizations.successChannelLinkCopied), + successContent: Text(zulipLocalizations.successChannelLinkCopied), data: ClipboardData(text: narrowLink(store, ChannelNarrow(channelId)).toString())); } } @@ -915,11 +915,11 @@ class CopyTopicLinkButton extends ActionSheetMenuItemButton { } @override void onPressed() async { - final localizations = ZulipLocalizations.of(pageContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final store = PerAccountStoreWidget.of(pageContext); PlatformActions.copyWithPopup(context: pageContext, - successContent: Text(localizations.successTopicLinkCopied), + successContent: Text(zulipLocalizations.successTopicLinkCopied), data: ClipboardData(text: narrowLink(store, narrow).toString())); } } diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index d3ed5c463d..8902eecb04 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -112,7 +112,7 @@ class ZulipApp extends StatefulWidget { return; } - final localizations = ZulipLocalizations.of(navigatorKey.currentContext!); + final zulipLocalizations = ZulipLocalizations.of(navigatorKey.currentContext!); final newSnackBar = scaffoldMessenger!.showSnackBar( snackBarAnimationStyle: AnimationStyle( duration: const Duration(milliseconds: 200), @@ -120,9 +120,9 @@ class ZulipApp extends StatefulWidget { SnackBar( content: Text(message), action: (details == null) ? null : SnackBarAction( - label: localizations.snackBarDetails, + label: zulipLocalizations.snackBarDetails, onPressed: () => showErrorDialog(context: navigatorKey.currentContext!, - title: localizations.errorDialogTitle, + title: zulipLocalizations.errorDialogTitle, message: details)))); _snackBarCount++; diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart index e345969189..59b70a11d6 100644 --- a/lib/widgets/autocomplete.dart +++ b/lib/widgets/autocomplete.dart @@ -178,8 +178,8 @@ class ComposeAutocomplete extends AutocompleteField= 247; // TODO(server-9) - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); return switch (wildcardOption) { WildcardMentionOption.all || WildcardMentionOption.everyone => isDmNarrow - ? localizations.wildcardMentionAllDmDescription + ? zulipLocalizations.wildcardMentionAllDmDescription : isChannelWildcardAvailable - ? localizations.wildcardMentionChannelDescription - : localizations.wildcardMentionStreamDescription, - WildcardMentionOption.channel => localizations.wildcardMentionChannelDescription, + ? zulipLocalizations.wildcardMentionChannelDescription + : zulipLocalizations.wildcardMentionStreamDescription, + WildcardMentionOption.channel => zulipLocalizations.wildcardMentionChannelDescription, WildcardMentionOption.stream => isChannelWildcardAvailable - ? localizations.wildcardMentionChannelDescription - : localizations.wildcardMentionStreamDescription, - WildcardMentionOption.topic => localizations.wildcardMentionTopicDescription, + ? zulipLocalizations.wildcardMentionChannelDescription + : zulipLocalizations.wildcardMentionStreamDescription, + WildcardMentionOption.topic => zulipLocalizations.wildcardMentionTopicDescription, }; } diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index 3748e39280..42521e63bf 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -252,15 +252,15 @@ class _SetStatusButton extends StatelessWidget { @override Widget build(BuildContext context) { - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); final store = PerAccountStoreWidget.of(context); final userStatus = store.getUserStatus(store.selfUserId); return ZulipMenuItemButton( style: ZulipMenuItemButtonStyle.list, label: userStatus == UserStatus.zero - ? localizations.statusButtonLabelStatusUnset - : localizations.statusButtonLabelStatusSet, + ? zulipLocalizations.statusButtonLabelStatusUnset + : zulipLocalizations.statusButtonLabelStatusSet, subLabel: userStatus == UserStatus.zero ? null : TextSpan(children: [ UserStatusEmoji.asWidgetSpan( userId: store.selfUserId, @@ -270,7 +270,7 @@ class _SetStatusButton extends StatelessWidget { neverAnimate: false, ), userStatus.text == null - ? TextSpan(text: localizations.noStatusText, + ? TextSpan(text: zulipLocalizations.noStatusText, style: TextStyle(fontStyle: FontStyle.italic)) : TextSpan(text: userStatus.text), ]), diff --git a/lib/widgets/read_receipts.dart b/lib/widgets/read_receipts.dart index 0df4eecb5c..21def2ecdb 100644 --- a/lib/widgets/read_receipts.dart +++ b/lib/widgets/read_receipts.dart @@ -137,15 +137,15 @@ class _ReadReceiptsUserList extends StatelessWidget { @override Widget build(BuildContext context) { - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); return switch(status) { FetchStatus.loading => BottomSheetEmptyContentPlaceholder(loading: true), FetchStatus.error => BottomSheetEmptyContentPlaceholder( - message: localizations.actionSheetReadReceiptsErrorReadCount), + message: zulipLocalizations.actionSheetReadReceiptsErrorReadCount), FetchStatus.success => userIds.isEmpty ? BottomSheetEmptyContentPlaceholder( - message: localizations.actionSheetReadReceiptsZeroReadCount) + message: zulipLocalizations.actionSheetReadReceiptsZeroReadCount) : InsetShadowBox( top: 8, bottom: 8, color: DesignVariables.of(context).bgContextMenu, diff --git a/lib/widgets/set_status.dart b/lib/widgets/set_status.dart index 4a7c87ae86..4e13c13cab 100644 --- a/lib/widgets/set_status.dart +++ b/lib/widgets/set_status.dart @@ -84,16 +84,16 @@ class _SetStatusPageState extends State { List statusSuggestions(BuildContext context) { final store = PerAccountStoreWidget.of(context); - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); final values = [ - ('1f6e0', localizations.userStatusBusy), - ('1f4c5', localizations.userStatusInAMeeting), - ('1f68c', localizations.userStatusCommuting), - ('1f912', localizations.userStatusOutSick), - ('1f334', localizations.userStatusVacationing), - ('1f3e0', localizations.userStatusWorkingRemotely), - ('1f3e2', localizations.userStatusAtTheOffice), + ('1f6e0', zulipLocalizations.userStatusBusy), + ('1f4c5', zulipLocalizations.userStatusInAMeeting), + ('1f68c', zulipLocalizations.userStatusCommuting), + ('1f912', zulipLocalizations.userStatusOutSick), + ('1f334', zulipLocalizations.userStatusVacationing), + ('1f3e0', zulipLocalizations.userStatusWorkingRemotely), + ('1f3e2', zulipLocalizations.userStatusAtTheOffice), ]; return [ for (final (emojiCode, statusText) in values) @@ -114,7 +114,7 @@ class _SetStatusPageState extends State { Future handleStatusSave() async { final store = PerAccountStoreWidget.of(context); - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); Navigator.pop(context); if (newStatus == oldStatus) return; @@ -122,7 +122,7 @@ class _SetStatusPageState extends State { try { await updateStatus(store.connection, change: statusChange.value); } catch (e) { - reportErrorToUserBriefly(localizations.updateStatusErrorTitle); + reportErrorToUserBriefly(zulipLocalizations.updateStatusErrorTitle); } } @@ -151,18 +151,18 @@ class _SetStatusPageState extends State { @override Widget build(BuildContext context) { final designVariables = DesignVariables.of(context); - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); final suggestions = statusSuggestions(context); return Scaffold( - appBar: ZulipAppBar(title: Text(localizations.setStatusPageTitle), + appBar: ZulipAppBar(title: Text(zulipLocalizations.setStatusPageTitle), actions: [ ValueListenableBuilder( valueListenable: statusChange, builder: (_, _, _) { return _ActionButton( - label: localizations.statusClearButtonLabel, + label: zulipLocalizations.statusClearButtonLabel, icon: ZulipIcons.remove, onPressed: newStatus == UserStatus.zero ? null @@ -173,7 +173,7 @@ class _SetStatusPageState extends State { valueListenable: statusChange, builder: (_, change, _) { return _ActionButton( - label: localizations.statusSaveButtonLabel, + label: zulipLocalizations.statusSaveButtonLabel, icon: ZulipIcons.check, onPressed: switch ((change.text, change.emoji)) { (OptionNone(), OptionNone()) => null, @@ -233,7 +233,7 @@ class _SetStatusPageState extends State { // TODO: display a counter as suggested in CZO discussion: // https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/Set.20user.20status/near/2224549 counterText: '', - hintText: localizations.statusTextHint, + hintText: zulipLocalizations.statusTextHint, hintStyle: TextStyle(color: designVariables.labelSearchPrompt), isDense: true, contentPadding: EdgeInsets.symmetric( diff --git a/test/widgets/profile_test.dart b/test/widgets/profile_test.dart index c79e3fb5b9..c50a02563e 100644 --- a/test/widgets/profile_test.dart +++ b/test/widgets/profile_test.dart @@ -446,13 +446,13 @@ void main() { }); group('user status', () { - final localizations = GlobalLocalizations.zulipLocalizations; + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; Finder findStatusButton({required bool statusSet}) { return find.widgetWithText(ZulipMenuItemButton, statusSet - ? localizations.statusButtonLabelStatusSet - : localizations.statusButtonLabelStatusUnset); + ? zulipLocalizations.statusButtonLabelStatusSet + : zulipLocalizations.statusButtonLabelStatusUnset); } testWidgets('non-self profile, status set: no status button, status info appears', (tester) async { @@ -514,7 +514,7 @@ void main() { final statusButtonFinder = findStatusButton(statusSet: true); final textPlaceholderFinder = findText( - includePlaceholders: false, localizations.noStatusText); + includePlaceholders: false, zulipLocalizations.noStatusText); check(statusButtonFinder).findsOne(); check(textPlaceholderFinder).findsOne(); From ee303d016c292af799069e3187af11609c8eefc9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Aug 2025 18:37:43 -0700 Subject: [PATCH 3/6] read_receipts [nfc]: Inline _ReadReceiptsUserList --- lib/widgets/read_receipts.dart | 49 +++++++++++++--------------------- 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/lib/widgets/read_receipts.dart b/lib/widgets/read_receipts.dart index 21def2ecdb..73e6848b31 100644 --- a/lib/widgets/read_receipts.dart +++ b/lib/widgets/read_receipts.dart @@ -80,15 +80,33 @@ class _ReadReceiptsState extends State with PerAccountStoreAwareSt @override Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); // TODO could pull out this layout/appearance code, // focusing this widget only on state management + final content = switch (status) { + FetchStatus.loading => BottomSheetEmptyContentPlaceholder(loading: true), + FetchStatus.error => BottomSheetEmptyContentPlaceholder( + message: zulipLocalizations.actionSheetReadReceiptsErrorReadCount), + FetchStatus.success => userIds.isEmpty + ? BottomSheetEmptyContentPlaceholder( + message: zulipLocalizations.actionSheetReadReceiptsZeroReadCount) + : InsetShadowBox( + top: 8, bottom: 8, + color: DesignVariables.of(context).bgContextMenu, + child: ListView.builder( + padding: EdgeInsets.symmetric(vertical: 8), + itemCount: userIds.length, + itemBuilder: (context, index) => + ReadReceiptsUserItem(userId: userIds[index]))) + }; + return SizedBox( height: 500, // TODO(design) tune child: Column( children: [ _ReadReceiptsHeader(receiptCount: userIds.length, status: status), - Expanded(child: _ReadReceiptsUserList(userIds: userIds, status: status)), + Expanded(child: content), Padding( padding: const EdgeInsets.symmetric(horizontal: 16), child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close)) @@ -129,35 +147,6 @@ class _ReadReceiptsHeader extends StatelessWidget { } } -class _ReadReceiptsUserList extends StatelessWidget { - const _ReadReceiptsUserList({required this.userIds, required this.status}); - - final List userIds; - final FetchStatus status; - - @override - Widget build(BuildContext context) { - final zulipLocalizations = ZulipLocalizations.of(context); - - return switch(status) { - FetchStatus.loading => BottomSheetEmptyContentPlaceholder(loading: true), - FetchStatus.error => BottomSheetEmptyContentPlaceholder( - message: zulipLocalizations.actionSheetReadReceiptsErrorReadCount), - FetchStatus.success => userIds.isEmpty - ? BottomSheetEmptyContentPlaceholder( - message: zulipLocalizations.actionSheetReadReceiptsZeroReadCount) - : InsetShadowBox( - top: 8, bottom: 8, - color: DesignVariables.of(context).bgContextMenu, - child: ListView.builder( - padding: EdgeInsets.symmetric(vertical: 8), - itemCount: userIds.length, - itemBuilder: (context, index) => - ReadReceiptsUserItem(userId: userIds[index]))) - }; - } -} - // TODO: deduplicate the code with [ViewReactionsUserItem] @visibleForTesting class ReadReceiptsUserItem extends StatelessWidget { From c79ffd3fce38152a02166835df35902e85ca82f0 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Aug 2025 18:41:21 -0700 Subject: [PATCH 4/6] read_receipts: Improve UX by making the sheet draggable-scrollable The Figma asks that the sheet be expandable to fill the screen: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=11367-21131&m=dev and that's implemented here. Compare f8ddff2f3, where we removed an earlier implementation. I hadn't tried to bring that back yet because I wanted to support triggering resize from a drag handle at the top, and I couldn't figure out how to do that. IIRC I could only get the drag handle to respond to drag-down gestures (via `enableDrag: true`) by shifting the sheet's position downward. That worked as a way to dismiss the sheet, but it was frustratingly different from the gesture handling on the scrollable list: - The slide-to-dismiss looked different from the shrink-and-dismiss, an awkward inconsistency - The list would respond to upward drags, too (by growing and showing more content) I've managed it here, modulo with a header instead of a drag handle, by making sure the scrollable area extends through the top of the sheet. (Done with a CustomScrollView, pinning the header to the viewport top.) --- lib/widgets/action_sheet.dart | 96 ++++++++++++++++++++++++++++++++++ lib/widgets/read_receipts.dart | 41 ++++++--------- 2 files changed, 111 insertions(+), 26 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 06cf84bdbc..dcf82389bc 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -225,6 +225,102 @@ class BottomSheetEmptyContentPlaceholder extends StatelessWidget { } } +/// A bottom sheet that resizes, scrolls, and dismisses in response to dragging. +/// +/// [header] is assumed to occupy the full width its parent allows. +/// (This is important for the clipping/shadow effect when [contentSliver] +/// scrolls under the header.) +/// +/// The sheet's initial height and minimum height before dismissing +/// are set proportionally to the screen's height. +/// The screen's height is read from the parent's max-height constraint, +/// so the caller should not introduce widgets that interfere with that. +/// (Non-layout wrapper widgets such as [InheritedWidget]s are OK.) +/// +/// The sheet's dismissal works like this: +/// - A "Close" button is offered. +/// - A drag-down or fling on the header or the [contentSliver] +/// causes those areas to shrink past a threshold at which the sheet +/// decides to dismiss. +/// - The [enableDrag] param of upstream's [showModalBottomSheet] +/// only seems to affect gesture handling on the Close button and its padding +/// (which are not part of the resizable/scrollable area): +/// - When true, the Close button responds to a downward fling by +/// sliding the sheet downward and dismissing it +/// (i.e. not by the usual behavior where the header- and-content height +/// shrinks past a threshold, causing dismissal). +/// - When false, the Close button doesn't respond to a downward fling. +class DraggableScrollableModalBottomSheet extends StatelessWidget { + const DraggableScrollableModalBottomSheet({ + super.key, + required this.header, + required this.contentSliver, + }); + + final Widget header; + final Widget contentSliver; + + @override + Widget build(BuildContext context) { + return DraggableScrollableSheet( + expand: false, + builder: (context, controller) { + final backgroundColor = Theme.of(context).bottomSheetTheme.backgroundColor!; + + // The "inset shadow" effect in Figma is a bit awkwardly + // implemented here, and there might be a better factoring: + // 1. This effect leans on the abstraction that [contentSliver] + // is simply a scrollable area in its own viewport. + // We'd normally just wrap that viewport in [InsetShadowBox]. + // 2. Really, though, the scrollable includes the header, + // pinned to the viewport top. We do this to support resizing + // (and dismiss-on-min-height) on gestures in the header, too, + // uniformly with the content. + // 3. So for the top shadow, we tack a shadow gradient onto the header, + // exploiting the header's pinning behavior to keep it fixed. + // 3. For the bottom, I haven't found a nice sliver-based implementation + // that supports pinning a shadow overlay at the viewport bottom. + // So for the bottom we use [InsetShadowBox] around the viewport, + // with just `bottom:` and no `top:`. + + final headerWithShadow = Column( + mainAxisSize: MainAxisSize.min, + children: [ + ColoredBox( + color: backgroundColor, + child: header), + SizedBox(height: 8, width: double.infinity, + child: DecoratedBox(decoration: fadeToTransparencyDecoration( + FadeToTransparencyDirection.down, backgroundColor))), + ]); + + return Column( + mainAxisSize: MainAxisSize.min, + children: [ + Flexible( + child: InsetShadowBox( + bottom: 8, + color: backgroundColor, + child: CustomScrollView( + // The iOS default "bouncing" effect would look uncoordinated + // in the common case where overscroll co-occurs with + // shrinking the sheet past the threshold where it dismisses. + physics: ClampingScrollPhysics(), + controller: controller, + slivers: [ + PinnedHeaderSliver(child: headerWithShadow), + SliverPadding( + padding: EdgeInsets.only(bottom: 8), + sliver: contentSliver), + ]))), + Padding( + padding: const EdgeInsets.symmetric(horizontal: 16), + child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close)) + ]); + }); + } +} + /// A button in an action sheet. /// /// When built from server data, the action sheet ignores changes in that data; diff --git a/lib/widgets/read_receipts.dart b/lib/widgets/read_receipts.dart index 73e6848b31..46c920372a 100644 --- a/lib/widgets/read_receipts.dart +++ b/lib/widgets/read_receipts.dart @@ -6,7 +6,6 @@ import '../generated/l10n/zulip_localizations.dart'; import 'action_sheet.dart'; import 'actions.dart'; import 'color.dart'; -import 'inset_shadow.dart'; import 'profile.dart'; import 'store.dart'; import 'text.dart'; @@ -81,36 +80,26 @@ class _ReadReceiptsState extends State with PerAccountStoreAwareSt @override Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); - // TODO could pull out this layout/appearance code, - // focusing this widget only on state management + final receiptCount = userIds.length; final content = switch (status) { - FetchStatus.loading => BottomSheetEmptyContentPlaceholder(loading: true), - FetchStatus.error => BottomSheetEmptyContentPlaceholder( - message: zulipLocalizations.actionSheetReadReceiptsErrorReadCount), + FetchStatus.loading => SliverToBoxAdapter( + child: BottomSheetEmptyContentPlaceholder(loading: true)), + FetchStatus.error => SliverToBoxAdapter( + child: BottomSheetEmptyContentPlaceholder( + message: zulipLocalizations.actionSheetReadReceiptsErrorReadCount)), FetchStatus.success => userIds.isEmpty - ? BottomSheetEmptyContentPlaceholder( - message: zulipLocalizations.actionSheetReadReceiptsZeroReadCount) - : InsetShadowBox( - top: 8, bottom: 8, - color: DesignVariables.of(context).bgContextMenu, - child: ListView.builder( - padding: EdgeInsets.symmetric(vertical: 8), - itemCount: userIds.length, - itemBuilder: (context, index) => - ReadReceiptsUserItem(userId: userIds[index]))) + ? SliverToBoxAdapter( + child: BottomSheetEmptyContentPlaceholder( + message: zulipLocalizations.actionSheetReadReceiptsZeroReadCount)) + : SliverList.builder( + itemCount: receiptCount, + itemBuilder: (_, index) => ReadReceiptsUserItem(userId: userIds[index])), }; - return SizedBox( - height: 500, // TODO(design) tune - child: Column( - children: [ - _ReadReceiptsHeader(receiptCount: userIds.length, status: status), - Expanded(child: content), - Padding( - padding: const EdgeInsets.symmetric(horizontal: 16), - child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close)) - ])); + return DraggableScrollableModalBottomSheet( + header: _ReadReceiptsHeader(receiptCount: receiptCount, status: status), + contentSliver: content); } } From 49a5a0d9dce709e9abab4f917a1421f8821c47cf Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 14 Aug 2025 12:31:39 -0700 Subject: [PATCH 5/6] emoji_reaction: Pass `explicitChildNodes: true` in a Semantics container This fixes a test failure when we make an upcoming refactor. (The container node's label would apparently have its children's labels appended to it...I haven't seen that behavior in manual testing on my iPhone with VoiceOver, before or after that refactor, but, shrug; this change does seem correct anyway.) --- lib/widgets/emoji_reaction.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index f494b02e79..ac12c2a03d 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -1030,6 +1030,7 @@ class ViewReactionsUserList extends StatelessWidget { label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName, userIds.length), role: SemanticsRole.tabPanel, container: true, + explicitChildNodes: true, child: result); } } From d9ab0757c0133e474cbf8f8a6ee16b338f74b0d7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 13 Aug 2025 18:15:21 -0700 Subject: [PATCH 6/6] 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. --- lib/widgets/emoji_reaction.dart | 137 +++++++++++++------------------- 1 file changed, 57 insertions(+), 80 deletions(-) diff --git a/lib/widgets/emoji_reaction.dart b/lib/widgets/emoji_reaction.dart index ac12c2a03d..b3117e465e 100644 --- a/lib/widgets/emoji_reaction.dart +++ b/lib/widgets/emoji_reaction.dart @@ -753,32 +753,18 @@ class _ViewReactionsState extends State with PerAccountStoreAware @override Widget build(BuildContext context) { - // TODO could pull out this layout/appearance code, - // focusing this widget only on state management - return Column( - mainAxisSize: MainAxisSize.min, - crossAxisAlignment: CrossAxisAlignment.center, - children: [ - ViewReactionsHeader( - messageId: widget.messageId, - reactionType: reactionType, - emojiCode: emojiCode, - onRequestSelect: _setSelection, - ), - // TODO if all reactions (or whole message) disappeared, - // we show a message saying there are no reactions, - // but the layout shifts (the sheet's height changes dramatically); - // we should avoid this. - if (reactionType != null && emojiCode != null) Flexible( - child: ViewReactionsUserList( - messageId: widget.messageId, - reactionType: reactionType!, - emojiCode: emojiCode!, - emojiName: emojiName!)), - Padding( - padding: const EdgeInsets.symmetric(horizontal: 16), - child: const BottomSheetDismissButton(style: BottomSheetDismissButtonStyle.close)) - ]); + return DraggableScrollableModalBottomSheet( + header: ViewReactionsHeader( + messageId: widget.messageId, + reactionType: reactionType, + emojiCode: emojiCode, + onRequestSelect: _setSelection, + ), + contentSliver: ViewReactionsUserListSliver( + messageId: widget.messageId, + reactionType: reactionType, + emojiCode: emojiCode, + emojiName: emojiName)); } } @@ -828,26 +814,27 @@ class ViewReactionsHeader extends StatelessWidget { padding: const EdgeInsets.only(top: 16, bottom: 4), child: InsetShadowBox(start: 8, end: 8, color: designVariables.bgContextMenu, - child: SingleChildScrollView( - // TODO(upstream) we want to pass excludeFromSemantics: true - // to the underlying Scrollable to remove an unwanted node - // in accessibility focus traversal when there are many items. - scrollDirection: Axis.horizontal, - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 8), - child: Semantics( - role: SemanticsRole.tabBar, - container: true, - explicitChildNodes: true, - label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total), - child: Row( - children: reactions.aggregated.mapIndexed((i, r) => - _ViewReactionsEmojiItem( - reactionWithVotes: r, - position: _emojiItemPosition(i, reactions.aggregated.length), - selected: r.reactionType == reactionType && r.emojiCode == emojiCode, - onRequestSelect: onRequestSelect), - ).toList())))))); + child: Center( + child: SingleChildScrollView( + // TODO(upstream) we want to pass excludeFromSemantics: true + // to the underlying Scrollable to remove an unwanted node + // in accessibility focus traversal when there are many items. + scrollDirection: Axis.horizontal, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 8), + child: Semantics( + role: SemanticsRole.tabBar, + container: true, + explicitChildNodes: true, + label: zulipLocalizations.seeWhoReactedSheetHeaderLabel(reactions.total), + child: Row( + children: reactions.aggregated.mapIndexed((i, r) => + _ViewReactionsEmojiItem( + reactionWithVotes: r, + position: _emojiItemPosition(i, reactions.aggregated.length), + selected: r.reactionType == reactionType && r.emojiCode == emojiCode, + onRequestSelect: onRequestSelect), + ).toList()))))))); } } @@ -955,7 +942,7 @@ class _ViewReactionsEmojiItem extends StatelessWidget { // I *think* we're following the doc with this but it's hard to tell; // I've only tested on iOS and I didn't notice a behavior change. - controlsNodes: {ViewReactionsUserList.semanticsIdentifier}, + controlsNodes: {ViewReactionsUserListSliver.semanticsIdentifier}, selected: selected, label: zulipLocalizations.seeWhoReactedSheetEmojiNameWithVoteCount(emojiName, count), @@ -965,10 +952,9 @@ class _ViewReactionsEmojiItem extends StatelessWidget { } } - @visibleForTesting -class ViewReactionsUserList extends StatelessWidget { - const ViewReactionsUserList({ +class ViewReactionsUserListSliver extends StatelessWidget { + const ViewReactionsUserListSliver({ super.key, required this.messageId, required this.reactionType, @@ -977,9 +963,9 @@ class ViewReactionsUserList extends StatelessWidget { }); final int messageId; - final ReactionType reactionType; - final String emojiCode; - final String emojiName; + final ReactionType? reactionType; + final String? emojiCode; + final String? emojiName; static const semanticsIdentifier = 'view-reactions-user-list'; @@ -987,7 +973,15 @@ class ViewReactionsUserList extends StatelessWidget { Widget build(BuildContext context) { final zulipLocalizations = ZulipLocalizations.of(context); final store = PerAccountStoreWidget.of(context); - final designVariables = DesignVariables.of(context); + + if (reactionType == null || emojiCode == null) { + // The emoji selection was cleared, + // which happens when the message is deleted or loses all its reactions. + // The sheet's header will have a message like + // "This message has no reactions." + return SliverPadding(padding: EdgeInsets.zero); + } + assert(emojiName != null); final message = store.messages[messageId]; @@ -999,39 +993,22 @@ class ViewReactionsUserList extends StatelessWidget { // Muted users will be shown as muted.) if (userIds == null) { - // This reaction lost all its votes, or the message was deleted. - return SizedBox.shrink(); + // The selected emoji lost all its votes. This won't show long if at all; + // a different emoji will be automatically selected if there is one. + return SliverPadding(padding: EdgeInsets.zero); } - Widget result = SizedBox( - height: 400, // TODO(design) tune - child: InsetShadowBox( - top: 8, - bottom: 8, - color: designVariables.bgContextMenu, - // TODO(upstream) we want to pass excludeFromSemantics: true - // to the underlying Scrollable to remove an unwanted node - // in accessibility focus traversal when there are many items. - child: ListView.builder( - padding: EdgeInsets.only( - // The Figma excludes the 8px top padding, which is unusual with the - // shadow effect (our InsetShadowBox). We include it so that the - // first item's touch feedback is shadow-free in the item's initial/ - // scrolled-to-top position. - top: 8, - bottom: 8, - ), - itemCount: userIds.length, - itemBuilder: (_, index) => - ViewReactionsUserItem(userId: userIds[index])))); + Widget result = SliverList.builder( + itemCount: userIds.length, + itemBuilder: (_, index) => ViewReactionsUserItem(userId: userIds[index])); - return Semantics( + return SliverSemantics( identifier: semanticsIdentifier, // See note on `controlsNodes` on the tab. - label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName, userIds.length), + label: zulipLocalizations.seeWhoReactedSheetUserListLabel(emojiName!, userIds.length), role: SemanticsRole.tabPanel, container: true, explicitChildNodes: true, - child: result); + sliver: result); } }