Skip to content

anchors 10/n: Open /near/ links at message #1566

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

Merged
merged 8 commits into from
Jun 12, 2025
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
55 changes: 42 additions & 13 deletions lib/model/internal_link.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,43 @@ Uri narrowLink(PerAccountStore store, Narrow narrow, {int? nearMessageId}) {
return result;
}

/// A [Narrow] from a given URL, on `store`'s realm.
/// The result of parsing some URL within a Zulip realm,
/// when the URL corresponds to some page in this app.
sealed class InternalLink {
InternalLink({required this.realmUrl});

final Uri realmUrl;
}

/// The result of parsing some URL that points to a narrow on a Zulip realm,
/// when the narrow is of a type that this app understands.
class NarrowLink extends InternalLink {
NarrowLink(this.narrow, this.nearMessageId, {required super.realmUrl});

final Narrow narrow;
final int? nearMessageId;
}

/// Try to parse the given URL as a page in this app, on `store`'s realm.
///
/// `url` must already be a result from [PerAccountStore.tryResolveUrl]
/// on `store`.
///
/// Returns `null` if any of the operator/operand pairs are invalid.
/// Returns null if the URL isn't on this realm,
/// or isn't a valid Zulip URL,
/// or isn't currently supported as leading to a page in this app.
///
/// In particular this will return null if `url` is a `/#narrow/…` URL
/// and any of the operator/operand pairs are invalid.
/// Since narrow links can combine operators in ways our [Narrow] type can't
/// represent, this can also return null for valid narrow links.
///
/// This can also return null for some valid narrow links that our Narrow
/// type *could* accurately represent. We should try to understand these
/// better, but some kinds will be rare, even unheard-of:
/// better, but some kinds will be rare, even unheard-of. For example:
/// #narrow/stream/1-announce/stream/1-announce (duplicated operator)
// TODO(#252): handle all valid narrow links, returning a search narrow
Narrow? parseInternalLink(Uri url, PerAccountStore store) {
InternalLink? parseInternalLink(Uri url, PerAccountStore store) {
if (!_isInternalLink(url, store.realmUrl)) return null;

final (category, segments) = _getCategoryAndSegmentsFromFragment(url.fragment);
Expand Down Expand Up @@ -155,7 +176,7 @@ bool _isInternalLink(Uri url, Uri realmUrl) {
return (category, segments);
}

Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
NarrowLink? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
assert(segments.isNotEmpty);
assert(segments.length.isEven);

Expand All @@ -164,6 +185,7 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
ApiNarrowDm? dmElement;
ApiNarrowWith? withElement;
Set<IsOperand> isElementOperands = {};
int? nearMessageId;

for (var i = 0; i < segments.length; i += 2) {
final (operator, negated) = _parseOperator(segments[i]);
Expand Down Expand Up @@ -201,24 +223,28 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
// It is fine to have duplicates of the same [IsOperand].
isElementOperands.add(IsOperand.fromRawString(operand));

case _NarrowOperator.near: // TODO(#82): support for near
continue;
case _NarrowOperator.near:
if (nearMessageId != null) return null;
final messageId = int.tryParse(operand, radix: 10);
if (messageId == null) return null;
nearMessageId = messageId;

case _NarrowOperator.unknown:
return null;
}
}

final Narrow? narrow;
if (isElementOperands.isNotEmpty) {
if (streamElement != null || topicElement != null || dmElement != null || withElement != null) {
return null;
}
if (isElementOperands.length > 1) return null;
switch (isElementOperands.single) {
case IsOperand.mentioned:
return const MentionsNarrow();
narrow = const MentionsNarrow();
case IsOperand.starred:
return const StarredMessagesNarrow();
narrow = const StarredMessagesNarrow();
case IsOperand.dm:
case IsOperand.private:
case IsOperand.alerted:
Expand All @@ -230,17 +256,20 @@ Narrow? _interpretNarrowSegments(List<String> segments, PerAccountStore store) {
}
} else if (dmElement != null) {
if (streamElement != null || topicElement != null || withElement != null) return null;
return DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId);
narrow = DmNarrow.withUsers(dmElement.operand, selfUserId: store.selfUserId);
} else if (streamElement != null) {
final streamId = streamElement.operand;
if (topicElement != null) {
return TopicNarrow(streamId, topicElement.operand, with_: withElement?.operand);
narrow = TopicNarrow(streamId, topicElement.operand, with_: withElement?.operand);
} else {
if (withElement != null) return null;
return ChannelNarrow(streamId);
narrow = ChannelNarrow(streamId);
}
} else {
return null;
}
return null;

return NarrowLink(narrow, nearMessageId, realmUrl: store.realmUrl);
}

@JsonEnum(fieldRename: FieldRename.kebab, alwaysCreate: true)
Expand Down
16 changes: 15 additions & 1 deletion lib/model/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
/// which might be made internally by this class in order to
/// fetch the messages from scratch, e.g. after certain events.
Anchor get anchor => _anchor;
final Anchor _anchor;
Anchor _anchor;

void _register() {
store.registerMessageList(this);
Expand Down Expand Up @@ -756,6 +756,20 @@ class MessageListView with ChangeNotifier, _MessageSequence {
}
}

/// Reset this view to start from the newest messages.
///
/// This will set [anchor] to [AnchorCode.newest],
/// and cause messages to be re-fetched from scratch.
void jumpToEnd() {
assert(fetched);
assert(!haveNewest);
assert(anchor != AnchorCode.newest);
_anchor = AnchorCode.newest;
_reset();
notifyListeners();
fetchInitial();
}

/// Add [outboxMessage] if it belongs to the view.
void addOutboxMessage(OutboxMessage outboxMessage) {
// TODO(#1441) implement this
Expand Down
2 changes: 1 addition & 1 deletion lib/notifications/display.dart
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ class NotificationDisplayManager {

return MessageListPage.buildRoute(
accountId: account.id,
// TODO(#82): Open at specific message, not just conversation
// TODO(#1565): Open at specific message, not just conversation
narrow: payload.narrow);
}

Expand Down
19 changes: 11 additions & 8 deletions lib/widgets/content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1536,15 +1536,18 @@ void _launchUrl(BuildContext context, String urlString) async {
return;
}

final internalNarrow = parseInternalLink(url, store);
if (internalNarrow != null) {
unawaited(Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: internalNarrow)));
return;
final internalLink = parseInternalLink(url, store);
assert(internalLink == null || internalLink.realmUrl == store.realmUrl);
switch (internalLink) {
case NarrowLink():
unawaited(Navigator.push(context,
MessageListPage.buildRoute(context: context,
narrow: internalLink.narrow,
initAnchorMessageId: internalLink.nearMessageId)));

case null:
await PlatformActions.launchUrl(context, url);
}

await PlatformActions.launchUrl(context, url);
}

/// Like [Image.network], but includes [authHeader] if [src] is on-realm.
Expand Down
99 changes: 78 additions & 21 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,17 @@ abstract class MessageListPageState {
}

class MessageListPage extends StatefulWidget {
const MessageListPage({super.key, required this.initNarrow});
const MessageListPage({
super.key,
required this.initNarrow,
this.initAnchorMessageId,
});

static AccountRoute<void> buildRoute({int? accountId, BuildContext? context,
required Narrow narrow}) {
required Narrow narrow, int? initAnchorMessageId}) {
return MaterialAccountWidgetRoute(accountId: accountId, context: context,
page: MessageListPage(initNarrow: narrow));
page: MessageListPage(
initNarrow: narrow, initAnchorMessageId: initAnchorMessageId));
}

/// The [MessageListPageState] above this context in the tree.
Expand All @@ -162,6 +167,7 @@ class MessageListPage extends StatefulWidget {
}

final Narrow initNarrow;
final int? initAnchorMessageId; // TODO(#1564) highlight target upon load

@override
State<MessageListPage> createState() => _MessageListPageState();
Expand Down Expand Up @@ -240,6 +246,10 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
actions.add(_TopicListButton(streamId: streamId));
}

// TODO(#80): default to anchor firstUnread, instead of newest
final initAnchor = widget.initAnchorMessageId == null
? AnchorCode.newest : NumericAnchor(widget.initAnchorMessageId!);

// Insert a PageRoot here, to provide a context that can be used for
// MessageListPage.ancestorOf.
return PageRoot(child: Scaffold(
Expand All @@ -259,7 +269,8 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
// we matched to the Figma in 21dbae120. See another frame, which uses that:
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=147%3A9088&mode=dev
body: Builder(
builder: (BuildContext context) => Column(
builder: (BuildContext context) {
return Column(
// Children are expected to take the full horizontal space
// and handle the horizontal device insets.
// The bottom inset should be handled by the last child only.
Expand All @@ -279,11 +290,13 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
child: MessageList(
key: _messageListKey,
narrow: narrow,
initAnchor: initAnchor,
onNarrowChanged: _narrowChanged,
))),
if (ComposeBox.hasComposeBox(narrow))
ComposeBox(key: _composeBoxKey, narrow: narrow)
]))));
]);
})));
}
}

Expand Down Expand Up @@ -479,9 +492,15 @@ const kFetchMessagesBufferPixels = (kMessageListFetchBatchSize / 2) * _kShortMes
/// When there is no [ComposeBox], also takes responsibility
/// for dealing with the bottom inset.
class MessageList extends StatefulWidget {
const MessageList({super.key, required this.narrow, required this.onNarrowChanged});
const MessageList({
super.key,
required this.narrow,
required this.initAnchor,
required this.onNarrowChanged,
});

final Narrow narrow;
final Anchor initAnchor;
final void Function(Narrow newNarrow) onNarrowChanged;

@override
Expand All @@ -504,8 +523,9 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat

@override
void onNewStore() { // TODO(#464) try to keep using old model until new one gets messages
final anchor = _model == null ? widget.initAnchor : _model!.anchor;
_model?.dispose();
_initModel(PerAccountStoreWidget.of(context));
_initModel(PerAccountStoreWidget.of(context), anchor);
}

@override
Expand All @@ -516,10 +536,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
super.dispose();
}

void _initModel(PerAccountStore store) {
// TODO(#82): get anchor as page/route argument, instead of using newest
// TODO(#80): default to anchor firstUnread, instead of newest
final anchor = AnchorCode.newest;
void _initModel(PerAccountStore store, Anchor anchor) {
_model = MessageListView.init(store: store,
narrow: widget.narrow, anchor: anchor);
model.addListener(_modelChanged);
Expand All @@ -537,6 +554,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
// redirected us to the new location of the operand message ID.
widget.onNarrowChanged(model.narrow);
}
// TODO when model reset, reset scroll
setState(() {
// The actual state lives in the [MessageListView] model.
// This method was called because that just changed.
Expand Down Expand Up @@ -568,6 +586,9 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
// still not yet updated to account for the newly-added messages.
model.fetchOlder();
}
if (scrollMetrics.extentAfter < kFetchMessagesBufferPixels) {
model.fetchNewer();
}
}

void _scrollChanged() {
Expand Down Expand Up @@ -618,6 +639,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
// MessageList's dartdoc.
child: SafeArea(
child: ScrollToBottomButton(
model: model,
scrollController: scrollController,
visible: _scrollToBottomVisible))),
])))));
Expand Down Expand Up @@ -753,13 +775,21 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
}

Widget _buildEndCap() {
return Column(crossAxisAlignment: CrossAxisAlignment.stretch, children: [
TypingStatusWidget(narrow: widget.narrow),
MarkAsReadWidget(narrow: widget.narrow),
// To reinforce that the end of the feed has been reached:
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603
const SizedBox(height: 36),
]);
if (model.haveNewest) {
return Column(crossAxisAlignment: CrossAxisAlignment.stretch, children: [
TypingStatusWidget(narrow: widget.narrow),
// TODO perhaps offer mark-as-read even when not done fetching?
MarkAsReadWidget(narrow: widget.narrow),
// To reinforce that the end of the feed has been reached:
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Mark-as-read/near/1680603
const SizedBox(height: 36),
]);
} else if (model.busyFetchingMore) {
// See [_buildStartCap] for why this condition shows a loading indicator.
return const _MessageListLoadingMore();
} else {
return SizedBox.shrink();
}
}

Widget _buildItem(MessageListItem data) {
Expand Down Expand Up @@ -809,13 +839,40 @@ class _MessageListLoadingMore extends StatelessWidget {
}

class ScrollToBottomButton extends StatelessWidget {
const ScrollToBottomButton({super.key, required this.scrollController, required this.visible});
const ScrollToBottomButton({
super.key,
required this.model,
required this.scrollController,
required this.visible,
});

final ValueNotifier<bool> visible;
final MessageListView model;
final MessageListScrollController scrollController;
final ValueNotifier<bool> visible;

void _scrollToBottom() {
scrollController.position.scrollToEnd();
if (model.haveNewest) {
// Scrolling smoothly from here to the bottom won't require any requests
// to the server.
// It also probably isn't *that* far away: the user must have scrolled
// here from there (or from near enough that a fetch reached there),
// so scrolling back there -- at top speed -- shouldn't take too long.
// Go for it.
scrollController.position.scrollToEnd();
} else {
// This message list doesn't have the messages for the bottom of history.
// There could be quite a lot of history between here and there --
// for example, at first unread in the combined feed or a busy channel,
// for a user who has some old unreads going back months and years.
// In that case trying to scroll smoothly to the bottom is hopeless.
//
// Given that there were at least 100 messages between this message list's
// initial anchor and the end of history (or else `fetchInitial` would
// have reached the end at the outset), that situation is very likely.
// Even if the end is close by, it's at least one fetch away.
// Instead of scrolling, jump to the end, which is always just one fetch.
model.jumpToEnd();
}
}

@override
Expand Down
Loading