Skip to content

msglist: When initial message fetch comes up empty, auto-focus compose box #1544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
18 changes: 18 additions & 0 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,15 @@ sealed class ComposeBoxController {
final content = ComposeContentController();
final contentFocusNode = FocusNode();

/// If no input is focused, requests focus on the appropriate input.
///
/// A convenience method to encapsulate choosing the topic or content input
/// when both exist (see [StreamComposeBoxController.requestFocusIfUnfocused]).
void requestFocusIfUnfocused() {
if (contentFocusNode.hasFocus) return;
contentFocusNode.requestFocus();
}

@mustCallSuper
void dispose() {
content.dispose();
Expand Down Expand Up @@ -1609,6 +1618,15 @@ class StreamComposeBoxController extends ComposeBoxController {
final ValueNotifier<ComposeTopicInteractionStatus> topicInteractionStatus =
ValueNotifier(ComposeTopicInteractionStatus.notEditingNotChosen);

@override void requestFocusIfUnfocused() {
if (topicFocusNode.hasFocus || contentFocusNode.hasFocus) return;
if (topicInteractionStatus.value == ComposeTopicInteractionStatus.notEditingNotChosen) {
topicFocusNode.requestFocus();
} else {
contentFocusNode.requestFocus();
}
}
Comment on lines +1621 to +1628
Copy link
Collaborator Author

@chrisbobbe chrisbobbe Jun 6, 2025

Choose a reason for hiding this comment

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

This function could also be useful for #1448.


@override
void dispose() {
topic.dispose();
Expand Down
11 changes: 11 additions & 0 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,8 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
model.fetchInitial();
}

bool _prevFetched = false;

void _modelChanged() {
if (model.narrow != widget.narrow) {
// Either:
Expand All @@ -535,6 +537,15 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
// The actual state lives in the [MessageListView] model.
// This method was called because that just changed.
});

if (!_prevFetched && model.fetched && model.messages.isEmpty) {
// If the fetch came up empty, there's nothing to read,
// so opening the keyboard won't be bothersome and could be helpful.
// It's definitely helpful if we got here from the new-DM page.
MessageListPage.ancestorOf(context)
.composeBoxState?.controller.requestFocusIfUnfocused();
}
_prevFetched = model.fetched;
}

void _handleScrollMetrics(ScrollMetrics scrollMetrics) {
Expand Down
5 changes: 5 additions & 0 deletions test/widgets/compose_box_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ extension ComposeBoxControllerChecks on Subject<ComposeBoxController> {
Subject<FocusNode> get contentFocusNode => has((c) => c.contentFocusNode, 'contentFocusNode');
}

extension StreamComposeBoxControllerChecks on Subject<StreamComposeBoxController> {
Subject<ComposeTopicController> get topic => has((c) => c.topic, 'topic');
Subject<FocusNode> get topicFocusNode => has((c) => c.topicFocusNode, 'topicFocusNode');
}

extension EditMessageComposeBoxControllerChecks on Subject<EditMessageComposeBoxController> {
Subject<int> get messageId => has((c) => c.messageId, 'messageId');
Subject<String?> get originalRawContent => has((c) => c.originalRawContent, 'originalRawContent');
Expand Down
73 changes: 71 additions & 2 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import 'dart:convert';
import 'dart:io';

import 'package:checks/checks.dart';
import 'package:collection/collection.dart';
import 'package:crypto/crypto.dart';
import 'package:file_picker/file_picker.dart';
import 'package:flutter_checks/flutter_checks.dart';
Expand Down Expand Up @@ -56,14 +57,23 @@ void main() {
User? selfUser,
List<User> otherUsers = const [],
List<ZulipStream> streams = const [],
List<Message>? messages,
bool? mandatoryTopics,
int? zulipFeatureLevel,
}) async {
if (narrow case ChannelNarrow(:var streamId) || TopicNarrow(: var streamId)) {
assert(streams.any((stream) => stream.streamId == streamId),
final channel = streams.firstWhereOrNull((s) => s.streamId == streamId);
assert(channel != null,
'Add a channel with "streamId" the same as of $narrow.streamId to the store.');
if (narrow is ChannelNarrow) {
// By default, bypass the complexity where the topic input is autofocused
// on an empty fetch, by making the fetch not empty. (In particular that
// complexity includes a getStreamTopics fetch for topic autocomplete.)
messages ??= [eg.streamMessage(stream: channel)];
}
}
addTearDown(testBinding.reset);
messages ??= [];
selfUser ??= eg.selfUser;
zulipFeatureLevel ??= eg.futureZulipFeatureLevel;
final selfAccount = eg.account(user: selfUser, zulipFeatureLevel: zulipFeatureLevel);
Expand All @@ -81,7 +91,11 @@ void main() {
connection = store.connection as FakeApiConnection;

connection.prepare(json:
eg.newestGetMessagesResult(foundOldest: true, messages: []).toJson());
eg.newestGetMessagesResult(foundOldest: true, messages: messages).toJson());
if (narrow is ChannelNarrow && messages.isEmpty) {
// The topic input will autofocus, triggering a getStreamTopics request.
connection.prepare(json: GetStreamTopicsResult(topics: []).toJson());
}
await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id,
child: MessageListPage(initNarrow: narrow)));
await tester.pumpAndSettle();
Expand Down Expand Up @@ -134,6 +148,61 @@ void main() {
await tester.pump(Duration.zero);
}

group('auto focus', () {
testWidgets('ChannelNarrow, non-empty fetch', (tester) async {
final channel = eg.stream();
await prepareComposeBox(tester,
narrow: ChannelNarrow(channel.streamId),
streams: [channel],
messages: [eg.streamMessage(stream: channel)]);
check(controller).isA<StreamComposeBoxController>()
.topicFocusNode.hasFocus.isFalse();
});

testWidgets('ChannelNarrow, empty fetch', (tester) async {
final channel = eg.stream();
await prepareComposeBox(tester,
narrow: ChannelNarrow(channel.streamId),
streams: [channel],
messages: []);
check(controller).isA<StreamComposeBoxController>()
.topicFocusNode.hasFocus.isTrue();
});

testWidgets('TopicNarrow, non-empty fetch', (tester) async {
final channel = eg.stream();
await prepareComposeBox(tester,
narrow: TopicNarrow(channel.streamId, eg.t('topic')),
streams: [channel],
messages: [eg.streamMessage(stream: channel, topic: 'topic')]);
check(controller).isNotNull().contentFocusNode.hasFocus.isFalse();
});

testWidgets('TopicNarrow, empty fetch', (tester) async {
final channel = eg.stream();
await prepareComposeBox(tester,
narrow: TopicNarrow(channel.streamId, eg.t('topic')),
streams: [channel],
messages: []);
check(controller).isNotNull().contentFocusNode.hasFocus.isTrue();
});

testWidgets('DmNarrow, non-empty fetch', (tester) async {
final user = eg.user();
await prepareComposeBox(tester,
narrow: DmNarrow.withUser(user.userId, selfUserId: store.selfUserId),
messages: [eg.dmMessage(from: user, to: [store.selfUser])]);
check(controller).isNotNull().contentFocusNode.hasFocus.isFalse();
});

testWidgets('DmNarrow, empty fetch', (tester) async {
await prepareComposeBox(tester,
narrow: DmNarrow.withUser(eg.user().userId, selfUserId: store.selfUserId),
messages: []);
check(controller).isNotNull().contentFocusNode.hasFocus.isTrue();
});
});

group('ComposeBoxTheme', () {
test('lerp light to dark, no crash', () {
final a = ComposeBoxTheme.light;
Expand Down
Loading