Skip to content

Commit 6e5a272

Browse files
committed
inbox: Fix collapsing a section from its sticky header
Before this fix, when scrolled down through the inbox page, collapsing a section (either the all-DMs section or a stream section) through its sticky header would work but cause an abnormal behavior, pushing the current section header and some of the following sections off the screen by the amount of space the current section occupied. After this fix, collapsing a section through a sticky header would work as expected, without pushing the header and the following sections off the screen. Fixes: #391.
1 parent 46b4577 commit 6e5a272

File tree

2 files changed

+86
-2
lines changed

2 files changed

+86
-2
lines changed

lib/widgets/inbox.dart

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,17 @@ abstract class _HeaderItem extends StatelessWidget {
208208
final _InboxPageState pageState;
209209
final int count;
210210
final bool hasMention;
211+
/// This context is used to ensure the [_StreamSection] or [_AllDmsSection]
212+
/// that encloses the current [_HeaderItem] is visible after being collapsed
213+
/// through this [_HeaderItem] being used as a sticky header.
214+
final BuildContext sectionContext;
211215

212216
const _HeaderItem({
213217
required this.collapsed,
214218
required this.pageState,
215219
required this.count,
216220
required this.hasMention,
221+
required this.sectionContext,
217222
});
218223

219224
String get title;
@@ -271,6 +276,7 @@ class _AllDmsHeaderItem extends _HeaderItem {
271276
required super.pageState,
272277
required super.count,
273278
required super.hasMention,
279+
required super.sectionContext,
274280
});
275281

276282
@override get title => 'Direct messages'; // TODO(i18n)
@@ -280,7 +286,13 @@ class _AllDmsHeaderItem extends _HeaderItem {
280286
@override get uncollapsedBackgroundColor => const Color(0xFFF3F0E7);
281287
@override get unreadCountBadgeBackgroundColor => null;
282288

283-
@override get onCollapseButtonTap => () {
289+
@override get onCollapseButtonTap => () async{
290+
if (!collapsed) {
291+
await Scrollable.ensureVisible(
292+
sectionContext,
293+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
294+
);
295+
}
284296
pageState.allDmsCollapsed = !collapsed;
285297
};
286298
@override get onRowTap => onCollapseButtonTap; // TODO open all-DMs narrow?
@@ -304,6 +316,7 @@ class _AllDmsSection extends StatelessWidget {
304316
hasMention: data.hasMention,
305317
collapsed: collapsed,
306318
pageState: pageState,
319+
sectionContext: context,
307320
);
308321
return StickyHeaderItem(
309322
header: header,
@@ -386,6 +399,7 @@ class _StreamHeaderItem extends _HeaderItem {
386399
required super.pageState,
387400
required super.count,
388401
required super.hasMention,
402+
required super.sectionContext,
389403
});
390404

391405
@override get title => subscription.name;
@@ -397,10 +411,14 @@ class _StreamHeaderItem extends _HeaderItem {
397411
@override get unreadCountBadgeBackgroundColor =>
398412
subscription.colorSwatch().unreadCountBadgeBackground;
399413

400-
@override get onCollapseButtonTap => () {
414+
@override get onCollapseButtonTap => () async {
401415
if (collapsed) {
402416
pageState.uncollapseStream(subscription.streamId);
403417
} else {
418+
await Scrollable.ensureVisible(
419+
sectionContext,
420+
alignmentPolicy: ScrollPositionAlignmentPolicy.keepVisibleAtStart,
421+
);
404422
pageState.collapseStream(subscription.streamId);
405423
}
406424
};
@@ -427,6 +445,7 @@ class _StreamSection extends StatelessWidget {
427445
hasMention: data.hasMention,
428446
collapsed: collapsed,
429447
pageState: pageState,
448+
sectionContext: context,
430449
);
431450
return StickyHeaderItem(
432451
header: header,

test/widgets/inbox_test.dart

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,21 @@ void main() {
5151
await tester.pumpAndSettle();
5252
}
5353

54+
List<StreamMessage> generateStreamMessages(
55+
ZulipStream stream,
56+
int count,
57+
) {
58+
return List.generate(
59+
count,
60+
(index) {
61+
return eg.streamMessage(
62+
stream: stream,
63+
topic: '${stream.name} topic $index',
64+
);
65+
},
66+
);
67+
}
68+
5469
/// Set up an inbox view with lots of interesting content.
5570
Future<void> setupVarious(WidgetTester tester) async {
5671
final stream1 = eg.stream(streamId: 1, name: 'stream 1');
@@ -64,7 +79,9 @@ void main() {
6479
users: [eg.selfUser, eg.otherUser, eg.thirdUser],
6580
unreadMessages: [
6681
eg.streamMessage(stream: stream1, topic: 'specific topic', flags: []),
82+
...generateStreamMessages(stream1, 10),
6783
eg.streamMessage(stream: stream2, flags: []),
84+
...generateStreamMessages(stream2, 40),
6885
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], flags: []),
6986
eg.dmMessage(from: eg.otherUser, to: [eg.selfUser, eg.thirdUser], flags: []),
7087
]);
@@ -310,6 +327,30 @@ void main() {
310327
checkAppearsUncollapsed(tester, findSectionContent);
311328
});
312329

330+
testWidgets('collapse all-DMs section after scroll', (tester) async {
331+
await setupVarious(tester);
332+
333+
final listFinder = find.byType(Scrollable);
334+
// Scroll the [StickyHeaderListView] enough so that
335+
// the [_AllDmsSection] shows a sticky header
336+
await tester.drag(listFinder, const Offset(0, -50));
337+
338+
final headerRow = findAllDmsHeaderRow(tester);
339+
// Check that the sticky header is present
340+
check(headerRow).isNotNull();
341+
342+
final findSectionContent = find.text(eg.otherUser.fullName);
343+
344+
checkAppearsUncollapsed(tester, findSectionContent);
345+
await tapCollapseIcon(tester);
346+
// Check that the header is still visible even after
347+
// collapsing the section
348+
check(headerRow).isNotNull();
349+
checkAppearsCollapsed(tester, findSectionContent);
350+
await tapCollapseIcon(tester);
351+
checkAppearsUncollapsed(tester, findSectionContent);
352+
});
353+
313354
// TODO check it remains collapsed even if you scroll far away and back
314355

315356
// TODO check that it's always uncollapsed when it appears after being
@@ -385,6 +426,30 @@ void main() {
385426
checkAppearsUncollapsed(tester, 1, findSectionContent);
386427
});
387428

429+
testWidgets('collapse stream section after scroll', (tester) async {
430+
await setupVarious(tester);
431+
432+
final listFinder = find.byType(Scrollable);
433+
// Scroll the [StickyHeaderListView] enough so that
434+
// the [_StreamSection] shows a sticky header
435+
await tester.drag(listFinder, const Offset(0, -200));
436+
437+
final headerRow = findStreamHeaderRow(tester, 1);
438+
// Check that the sticky header is present
439+
check(headerRow).isNotNull();
440+
441+
final findSectionContent = find.text('specific topic');
442+
443+
checkAppearsUncollapsed(tester, 1, findSectionContent);
444+
await tapCollapseIcon(tester, 1);
445+
// Check that the header is still visible even after
446+
// collapsing the section
447+
check(headerRow).isNotNull();
448+
checkAppearsCollapsed(tester, 1, findSectionContent);
449+
await tapCollapseIcon(tester, 1);
450+
checkAppearsUncollapsed(tester, 1, findSectionContent);
451+
});
452+
388453
// TODO check it remains collapsed even if you scroll far away and back
389454

390455
// TODO check that it's always uncollapsed when it appears after being

0 commit comments

Comments
 (0)