Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 26 additions & 22 deletions lib/widgets/all_channels.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -96,28 +98,30 @@ 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: ConstrainedBox(constraints: const BoxConstraints(minHeight: 44),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit-sequence nit:

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.

Did the removal of the three-dots button make these rows shorter than they had been?

If so, then this commit adding the 44px min-height should go before the commit that removes the three-dots button. That way we don't have an intermediate state where the UI is less good in some ways than it was before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, reordering the commits.
Private channel row with no content access is shorter (20px high) without the three-dots button.

child: Padding(padding: const 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),
]))));
}
}

Expand Down
24 changes: 21 additions & 3 deletions test/widgets/all_channels_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -203,20 +204,37 @@ void main() {
check(maybeToggle).isNull();
}

check(findInRow(find.byIcon(ZulipIcons.more_horizontal))).findsOne();
final touchTargetSize = tester.getSize(findElement);
check(touchTargetSize.height).equals(44);
}
});

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]);
Expand Down