From d79bcb74dce484a087f206f863c260ad123c4d9d Mon Sep 17 00:00:00 2001 From: chungwwei Date: Sat, 11 Oct 2025 04:52:51 -0400 Subject: [PATCH 1/4] all_channels: Remove three dot menu and add gesture controls Make the All Channels Page consistent with the Channel Page, as navigating to channel feed and opening the bottom sheet currently uses the three-dot menu icon, which differs from the gesture pattern used in the Channel Page. Fixes part of #1914 --- lib/widgets/all_channels.dart | 46 +++++++++++++++-------------- test/widgets/all_channels_test.dart | 23 ++++++++++++--- 2 files changed, 43 insertions(+), 26 deletions(-) diff --git a/lib/widgets/all_channels.dart b/lib/widgets/all_channels.dart index 3aaf0d8baf..4041a47cad 100644 --- a/lib/widgets/all_channels.dart +++ b/lib/widgets/all_channels.dart @@ -5,11 +5,13 @@ import '../api/route/channels.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../log.dart'; import '../model/channel.dart'; +import '../model/narrow.dart'; import 'action_sheet.dart'; import 'actions.dart'; import 'app_bar.dart'; import 'button.dart'; import 'icons.dart'; +import 'message_list.dart'; import 'page.dart'; import 'remote_settings.dart'; import 'store.dart'; @@ -96,28 +98,28 @@ class AllChannelsListEntry extends StatelessWidget { final Subscription? subscription = channel is Subscription ? channel : null; final hasContentAccess = store.selfHasContentAccess(channel); - return Padding( - padding: EdgeInsetsDirectional.only(start: 8, end: 4), - child: Row(spacing: 6, children: [ - Icon( - size: 20, - color: colorSwatchFor(context, subscription).iconOnPlainBackground, - iconDataForStream(channel)), - Expanded( - child: Text( - style: TextStyle( - color: designVariables.textMessage, - fontSize: 17, - height: 20 / 17, - ).merge(weightVariableTextStyle(context, wght: 600)), - channel.name)), - if (hasContentAccess) _SubscribeToggle(channel: channel), - ZulipIconButton( - icon: ZulipIcons.more_horizontal, - onPressed: () { - showChannelActionSheet(context, channelId: channel.streamId); - }), - ])); + return InkWell( + onTap: !hasContentAccess ? null : () => Navigator.push(context, + MessageListPage.buildRoute(context: context, + narrow: ChannelNarrow(channel.streamId))), + onLongPress: () => showChannelActionSheet(context, channelId: channel.streamId), + child: Padding( + padding: EdgeInsetsDirectional.only(start: 8, end: 12), + child: Row(spacing: 6, children: [ + Icon( + size: 20, + color: colorSwatchFor(context, subscription).iconOnPlainBackground, + iconDataForStream(channel)), + Expanded( + child: Text( + style: TextStyle( + color: designVariables.textMessage, + fontSize: 17, + height: 20 / 17, + ).merge(weightVariableTextStyle(context, wght: 600)), + channel.name)), + if (hasContentAccess) _SubscribeToggle(channel: channel), + ]))); } } diff --git a/test/widgets/all_channels_test.dart b/test/widgets/all_channels_test.dart index b47546be40..e50c1dafd5 100644 --- a/test/widgets/all_channels_test.dart +++ b/test/widgets/all_channels_test.dart @@ -12,6 +12,7 @@ import 'package:zulip/widgets/app_bar.dart'; import 'package:zulip/widgets/button.dart'; import 'package:zulip/widgets/home.dart'; import 'package:zulip/widgets/icons.dart'; +import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/remote_settings.dart'; import 'package:zulip/widgets/theme.dart'; @@ -202,21 +203,35 @@ void main() { } else { check(maybeToggle).isNull(); } - - check(findInRow(find.byIcon(ZulipIcons.more_horizontal))).findsOne(); } }); - testWidgets('tapping three-dots button opens channel action sheet', (tester) async { + testWidgets('open channel action sheet on long press', (tester) async { await setupAllChannelsPage(tester, channels: [eg.stream()]); - await tester.tap(find.byIcon(ZulipIcons.more_horizontal)); + await tester.longPress(find.byType(AllChannelsListEntry)); await tester.pump(); await transitionDurationObserver.pumpPastTransition(tester); check(find.byType(BottomSheet)).findsOne(); }); + testWidgets('navigate to channel feed on tap', (tester) async { + final channel = eg.stream(name: 'some-channel'); + await setupAllChannelsPage(tester, channels: [channel]); + + connection.prepare(json: eg.newestGetMessagesResult( + foundOldest: true, messages: [eg.streamMessage(stream: channel)]).toJson()); + await tester.tap(find.byType(AllChannelsListEntry)); + await tester.pump(); + await transitionDurationObserver.pumpPastTransition(tester); + + check(find.descendant( + of: find.byType(MessageListPage), + matching: find.text('some-channel')), + ).findsOne(); + }); + testWidgets('use toggle switch to subscribe/unsubscribe', (tester) async { final channel = eg.stream(); await setupAllChannelsPage(tester, channels: [channel]); From 7e3f92eb2d14f92de337cd1a87041da32d2fcbd8 Mon Sep 17 00:00:00 2001 From: chungwwei Date: Sun, 26 Oct 2025 00:40:44 -0400 Subject: [PATCH 2/4] all_channels: Restrict channel name to oneline Previously, long name could span multiple lines when the system font size was large, causing the row height to become insconsistent with other items. Limiting the name to a single line ensures consistent row height, similar to how it's done on the Channel Page. Fixes part of #1914 --- lib/widgets/all_channels.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/widgets/all_channels.dart b/lib/widgets/all_channels.dart index 4041a47cad..9234441b2a 100644 --- a/lib/widgets/all_channels.dart +++ b/lib/widgets/all_channels.dart @@ -112,6 +112,8 @@ class AllChannelsListEntry extends StatelessWidget { iconDataForStream(channel)), Expanded( child: Text( + maxLines: 1, + overflow: TextOverflow.ellipsis, style: TextStyle( color: designVariables.textMessage, fontSize: 17, From baf929f27bbd6c240efef603ca20f7dc79bc8e91 Mon Sep 17 00:00:00 2001 From: chungwwei Date: Sat, 25 Oct 2025 21:07:57 -0400 Subject: [PATCH 3/4] all_channels: Make row item at least 44px tall with ConstrainedBox Make sure every row is at least 44px in height by specifying BoxConstraints(minHeight: 44) to meet touch target requirement. --- lib/widgets/all_channels.dart | 39 +++++++++++++++-------------- test/widgets/all_channels_test.dart | 3 +++ 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/lib/widgets/all_channels.dart b/lib/widgets/all_channels.dart index 9234441b2a..17d03ca30b 100644 --- a/lib/widgets/all_channels.dart +++ b/lib/widgets/all_channels.dart @@ -103,25 +103,26 @@ class AllChannelsListEntry extends StatelessWidget { MessageListPage.buildRoute(context: context, narrow: ChannelNarrow(channel.streamId))), onLongPress: () => showChannelActionSheet(context, channelId: channel.streamId), - child: Padding( - padding: EdgeInsetsDirectional.only(start: 8, end: 12), - child: Row(spacing: 6, children: [ - Icon( - size: 20, - color: colorSwatchFor(context, subscription).iconOnPlainBackground, - iconDataForStream(channel)), - Expanded( - child: Text( - maxLines: 1, - overflow: TextOverflow.ellipsis, - style: TextStyle( - color: designVariables.textMessage, - fontSize: 17, - height: 20 / 17, - ).merge(weightVariableTextStyle(context, wght: 600)), - channel.name)), - if (hasContentAccess) _SubscribeToggle(channel: channel), - ]))); + child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 44), + child: Padding( + padding: EdgeInsetsDirectional.only(start: 8, end: 12), + child: Row(spacing: 6, children: [ + Icon( + size: 20, + color: colorSwatchFor(context, subscription).iconOnPlainBackground, + iconDataForStream(channel)), + Expanded( + child: Text( + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: TextStyle( + color: designVariables.textMessage, + fontSize: 17, + height: 20 / 17, + ).merge(weightVariableTextStyle(context, wght: 600)), + channel.name)), + if (hasContentAccess) _SubscribeToggle(channel: channel) + ])))); } } diff --git a/test/widgets/all_channels_test.dart b/test/widgets/all_channels_test.dart index e50c1dafd5..f6e1092781 100644 --- a/test/widgets/all_channels_test.dart +++ b/test/widgets/all_channels_test.dart @@ -203,6 +203,9 @@ void main() { } else { check(maybeToggle).isNull(); } + + final touchTargetSize = tester.getSize(findElement); + check(touchTargetSize.height).equals(44); } }); From 5a601451fa4a2f72548fae7e95685b75ed19cb55 Mon Sep 17 00:00:00 2001 From: chungwwei Date: Mon, 27 Oct 2025 22:23:42 -0400 Subject: [PATCH 4/4] all_channels: Make toggle transparent and non-interactive when noContentAccess If the toggle's height exceeds 44px, unsubsribing from a private channel shrinks the row to 44px, causing a ui jump. If the private channel name is too long, unsubscribing casues the previously ellipsis text to stretch to the right edge, casuing a ui jump. --- lib/widgets/all_channels.dart | 5 ++++- test/widgets/all_channels_test.dart | 12 ++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/widgets/all_channels.dart b/lib/widgets/all_channels.dart index 17d03ca30b..442806d560 100644 --- a/lib/widgets/all_channels.dart +++ b/lib/widgets/all_channels.dart @@ -121,7 +121,10 @@ class AllChannelsListEntry extends StatelessWidget { height: 20 / 17, ).merge(weightVariableTextStyle(context, wght: 600)), channel.name)), - if (hasContentAccess) _SubscribeToggle(channel: channel) + IgnorePointer(ignoring: !hasContentAccess, + child: AnimatedOpacity(opacity: !hasContentAccess ? 0 : 1, + duration: Duration(milliseconds: 200), + child: _SubscribeToggle(channel: channel))) ])))); } } diff --git a/test/widgets/all_channels_test.dart b/test/widgets/all_channels_test.dart index f6e1092781..1beaa6ba79 100644 --- a/test/widgets/all_channels_test.dart +++ b/test/widgets/all_channels_test.dart @@ -195,13 +195,17 @@ void main() { check(findInRow(find.text(channel.name))).findsOne(); - final maybeToggle = tester.widgetList( - findInRow(find.byType(Toggle))).singleOrNull; + final toggle = tester.widget(findInRow(find.byType(Toggle))); + final opacityWidget = tester.widget(findInRow(find.byType(AnimatedOpacity))); + final ignoreWidget = tester.widget(findInRow(find.byType(IgnorePointer))); if (store.selfHasContentAccess(channel)) { final isSubscribed = channel is Subscription; - check(maybeToggle).isNotNull().value.equals(isSubscribed); + check(toggle).value.equals(isSubscribed); + check(opacityWidget.opacity).equals(1); + check(ignoreWidget.ignoring).isFalse(); } else { - check(maybeToggle).isNull(); + check(opacityWidget.opacity).equals(0); + check(ignoreWidget.ignoring).isTrue(); } final touchTargetSize = tester.getSize(findElement);