Skip to content
Open
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
94 changes: 48 additions & 46 deletions lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -269,52 +269,54 @@ abstract class _HeaderItem extends StatelessWidget {
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
final designVariables = DesignVariables.of(context);
return Material(
color: collapsed
? designVariables.background // TODO(design) check if this is the right variable
: uncollapsedBackgroundColor(context),
child: InkWell(
// TODO use onRowTap to handle taps that are not on the collapse button.
// Probably we should give the collapse button a 44px or 48px square
// touch target:
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680973>
// But that's in tension with the Figma, which gives these header rows
// 40px min height.
onTap: onCollapseButtonTap,
onLongPress: this is _LongPressable
? (this as _LongPressable).onLongPress
: null,
child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [
Padding(padding: const EdgeInsets.all(10),
child: Icon(size: 20, color: designVariables.sectionCollapseIcon,
collapsed ? ZulipIcons.arrow_right : ZulipIcons.arrow_down)),
Icon(size: 18,
color: collapsed
? collapsedIconColor(context)
: uncollapsedIconColor(context),
icon),
const SizedBox(width: 5),
Expanded(child: Padding(
padding: const EdgeInsets.symmetric(vertical: 4),
child: Text(
style: TextStyle(
fontSize: 17,
height: (20 / 17),
// TODO(design) check if this is the right variable
color: designVariables.labelMenuButton,
).merge(weightVariableTextStyle(context, wght: 600)),
maxLines: 1,
overflow: TextOverflow.ellipsis,
title(zulipLocalizations)))),
const SizedBox(width: 12),
if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign),
Padding(padding: const EdgeInsetsDirectional.only(end: 16),
child: CounterBadge(
// TODO(design) use CounterKind.quantity, following Figma
kind: CounterBadgeKind.unread,
channelIdForBackground: channelId,
count: count)),
])));
return Semantics(
header: true,
child: Material(
color: collapsed
? designVariables.background // TODO(design) check if this is the right variable
: uncollapsedBackgroundColor(context),
child: InkWell(
// TODO use onRowTap to handle taps that are not on the collapse button.
// Probably we should give the collapse button a 44px or 48px square
// touch target:
// <https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680973>
// But that's in tension with the Figma, which gives these header rows
// 40px min height.
onTap: onCollapseButtonTap,
onLongPress: this is _LongPressable
? (this as _LongPressable).onLongPress
: null,
child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [
Padding(padding: const EdgeInsets.all(10),
child: Icon(size: 20, color: designVariables.sectionCollapseIcon,
collapsed ? ZulipIcons.arrow_right : ZulipIcons.arrow_down)),
Icon(size: 18,
color: collapsed
? collapsedIconColor(context)
: uncollapsedIconColor(context),
icon),
const SizedBox(width: 5),
Expanded(child: Padding(
padding: const EdgeInsets.symmetric(vertical: 4),
child: Text(
style: TextStyle(
fontSize: 17,
height: (20 / 17),
// TODO(design) check if this is the right variable
color: designVariables.labelMenuButton,
).merge(weightVariableTextStyle(context, wght: 600)),
maxLines: 1,
overflow: TextOverflow.ellipsis,
title(zulipLocalizations)))),
const SizedBox(width: 12),
if (hasMention) const _IconMarker(icon: ZulipIcons.at_sign),
Padding(padding: const EdgeInsetsDirectional.only(end: 16),
child: CounterBadge(
// TODO(design) use CounterKind.quantity, following Figma
kind: CounterBadgeKind.unread,
channelIdForBackground: channelId,
count: count)),
]))));
}
}

Expand Down
110 changes: 57 additions & 53 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1904,31 +1904,33 @@ class StreamMessageRecipientHeader extends StatelessWidget {
store.topicVisibilityPolicy(streamId, topic))),
]));

return GestureDetector(
// When already in a topic narrow, disable tap interaction that would just
// push a MessageListPage for the same topic narrow.
// TODO(#1039) simplify by removing topic-narrow condition if we remove
// recipient headers in topic narrows
onTap: narrow is TopicNarrow ? null
: () => Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: TopicNarrow.ofMessage(message))),
onLongPress: () => showTopicActionSheet(context,
channelId: streamId,
topic: topic,
someMessageIdInTopic: message.id),
child: ColoredBox(
color: backgroundColor,
child: Row(
crossAxisAlignment: CrossAxisAlignment.center,
children: [
// TODO(#282): Long stream name will break layout; find a fix.
streamWidget,
Expanded(child: topicWidget),
// TODO topic links?
// Then web also has edit/resolve/mute buttons. Skip those for mobile.
RecipientHeaderDate(message: message),
])));
return Semantics(
header: true,
child: GestureDetector(
// When already in a topic narrow, disable tap interaction that would just
// push a MessageListPage for the same topic narrow.
// TODO(#1039) simplify by removing topic-narrow condition if we remove
// recipient headers in topic narrows
onTap: narrow is TopicNarrow ? null
: () => Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: TopicNarrow.ofMessage(message))),
onLongPress: () => showTopicActionSheet(context,
channelId: streamId,
topic: topic,
someMessageIdInTopic: message.id),
child: ColoredBox(
color: backgroundColor,
child: Row(
crossAxisAlignment: CrossAxisAlignment.center,
children: [
// TODO(#282): Long stream name will break layout; find a fix.
streamWidget,
Expanded(child: topicWidget),
// TODO topic links?
// Then web also has edit/resolve/mute buttons. Skip those for mobile.
RecipientHeaderDate(message: message),
]))));
}
}

Expand Down Expand Up @@ -1961,34 +1963,36 @@ class DmRecipientHeader extends StatelessWidget {
final messageListTheme = MessageListTheme.of(context);
final designVariables = DesignVariables.of(context);

return GestureDetector(
// When already in a DM narrow, disable tap interaction that would just
// push a MessageListPage for the same DM narrow.
// TODO(#1244) simplify by removing DM-narrow condition if we remove
// recipient headers in DM narrows
onTap: narrow is DmNarrow ? null
: () => Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId))),
child: ColoredBox(
color: messageListTheme.dmRecipientHeaderBg,
child: Padding(
padding: const EdgeInsets.symmetric(vertical: 11),
child: Row(
crossAxisAlignment: CrossAxisAlignment.center,
children: [
Padding(
padding: const EdgeInsets.symmetric(horizontal: 6),
child: Icon(
color: designVariables.title,
size: 16,
ZulipIcons.two_person)),
Expanded(
child: Text(title,
style: recipientHeaderTextStyle(context),
overflow: TextOverflow.ellipsis)),
RecipientHeaderDate(message: message),
]))));
return Semantics(
header: true,
child: GestureDetector(
// When already in a DM narrow, disable tap interaction that would just
// push a MessageListPage for the same DM narrow.
// TODO(#1244) simplify by removing DM-narrow condition if we remove
// recipient headers in DM narrows
onTap: narrow is DmNarrow ? null
: () => Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: DmNarrow.ofMessage(message, selfUserId: store.selfUserId))),
child: ColoredBox(
color: messageListTheme.dmRecipientHeaderBg,
child: Padding(
padding: const EdgeInsets.symmetric(vertical: 11),
child: Row(
crossAxisAlignment: CrossAxisAlignment.center,
children: [
Padding(
padding: const EdgeInsets.symmetric(horizontal: 6),
child: Icon(
color: designVariables.title,
size: 16,
ZulipIcons.two_person)),
Expanded(
child: Text(title,
style: recipientHeaderTextStyle(context),
overflow: TextOverflow.ellipsis)),
RecipientHeaderDate(message: message),
])))));
}
}

Expand Down
13 changes: 13 additions & 0 deletions test/widgets/inbox_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,19 @@ void main() {
await setupVarious(tester);
});

testWidgets('channel header has header semantics for accessibility', (tester) async {
await setupVarious(tester);

final headerRow = findStreamHeaderRow(tester, 1);
check(headerRow).isNotNull();

final semanticsFinder = find.ancestor(
of: find.byWidget(headerRow!),
matching: find.byWidgetPredicate(
(widget) => widget is Semantics && widget.properties.header == true));
check(semanticsFinder).findsOne();
});

testWidgets('UnreadCountBadge text color for a channel', (tester) async {
// Regression test for a bug where
// DesignVariables.labelCounterUnread was used for the text instead of
Expand Down
26 changes: 26 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,19 @@ void main() {
).evaluate()).length.equals(1);
});

testWidgets('has header semantics for accessibility', (tester) async {
final message = eg.streamMessage();
await setupMessageListPage(tester,
narrow: const CombinedFeedNarrow(),
messages: [message]);
await tester.pump();

final semantics = tester.firstWidget<Semantics>(find.descendant(
of: find.byType(StreamMessageRecipientHeader),
matching: find.byType(Semantics)));
check(semantics.properties.header).equals(true);
});

testWidgets('show stream name from message when stream unknown', (tester) async {
// This can perfectly well happen, because message fetches can race
// with events.
Expand Down Expand Up @@ -1947,6 +1960,19 @@ void main() {
await tester.pump();
check(pushedRoutes).isEmpty();
});

testWidgets('has header semantics for accessibility', (tester) async {
final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]);
await setupMessageListPage(tester,
narrow: const CombinedFeedNarrow(),
messages: [dmMessage]);
await tester.pump();

final semantics = tester.firstWidget<Semantics>(find.descendant(
of: find.byType(DmRecipientHeader),
matching: find.byType(Semantics)));
check(semantics.properties.header).equals(true);
});
});

group('MessageTimestampStyle', () {
Expand Down