Skip to content
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.

Sign up for GitHub

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

anchors 5/n: Control scroll position #1436

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

anchors 5/n: Control scroll position #1436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
09d7a57
msglist [nfc]: Cut already-broken logic for slightly less padding at …
gnprice Mar 25, 2025
50b0a75
msglist [nfc]: Introduce MessageListScrollView, not yet doing anythin…
gnprice Mar 23, 2025
b49b3a3
scroll [nfc]: Copy RenderViewport.performLayout and friends from upst…
gnprice Mar 23, 2025
9eefc2b
msglist [nfc]: Introduce MessageListScrollPosition
gnprice Mar 23, 2025
4efbb12
msglist [nfc]: Force anchor to 1.0
gnprice Mar 24, 2025
2541a10
scroll [nfc]: Introduce applyContentDimensionsRaw on scroll position
gnprice Mar 24, 2025
1667c6e
scroll: Start out scrolled to bottom of list
gnprice Apr 3, 2025
07a3905
scroll: Keep short list pinned at bottom of viewport, not past bottom
gnprice Mar 24, 2025
0f7bc7a
scroll: Stay at end once there
gnprice Mar 24, 2025
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
11 changes: 5 additions & 6 deletions lib/widgets/message_list.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ class MessageList extends StatefulWidget {

class _MessageListState extends State<MessageList> with PerAccountStoreAwareStateMixin<MessageList> {
MessageListView? model;
final ScrollController scrollController = ScrollController();
final ScrollController scrollController = MessageListScrollController();
final ValueNotifier<bool> _scrollToBottomVisibleValue = ValueNotifier<bool>(false);

@override
Expand Down Expand Up @@ -622,7 +622,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
if (i == 2) return TypingStatusWidget(narrow: widget.narrow);

final data = model!.items[length - 1 - (i - 3)];
return _buildItem(zulipLocalizations, data, i);
return _buildItem(zulipLocalizations, data);
}));

if (!ComposeBox.hasComposeBox(widget.narrow)) {
Expand All @@ -631,7 +631,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
sliver = SliverSafeArea(sliver: sliver);
}

return CustomPaintOrderScrollView(
return MessageListScrollView(
// TODO: Offer `ScrollViewKeyboardDismissBehavior.interactive` (or
// similar) if that is ever offered:
// https://github.com/flutter/flutter/issues/57609#issuecomment-1355340849
Expand All @@ -645,7 +645,6 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat

controller: scrollController,
semanticChildCount: length + 2,
anchor: 1.0,
Copy link
Collaborator

@chrisbobbe chrisbobbe Apr 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

msglist [nfc]: Force anchor to 1.0

I'm trying to understand anchor; could you confirm that this part of RenderViewport.anchor's dartdoc is correct?

  /// […] If the [anchor] is 1.0, and the
  /// [axisDirection] is [AxisDirection.right], then the zero scroll offset is
  /// on the left edge of the viewport.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a code example at the doc link; here's what it looks like with the code as-is, with AxisDirection.right and anchor 0.5:

image

And if I just change anchor to 1.0:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds backward to me — I'd say that's true if you replace either "1.0" with "0.0", or ".right" with ".left", or "left edge" with "right edge". (Or all three.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's helpful, thanks. I'm curious if this also confused you when you were first learning it :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I did run across that bit of docs recently and puzzled for a bit over whether there was a way to interpret it that made that description correct. (The wording "the zero scroll offset is on the left edge of the viewport" is itself a bit vague.)

I don't recall when I was first figuring out what anchor means. It appears in
bdac26f and d4490b1 from last January, so I must have had at least an empirical understanding by then.

center: centerSliverKey,
paintOrder: SliverPaintOrder.firstIsTop,

Expand All @@ -659,7 +658,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
]);
}

Widget _buildItem(ZulipLocalizations zulipLocalizations, MessageListItem data, int i) {
Widget _buildItem(ZulipLocalizations zulipLocalizations, MessageListItem data) {
switch (data) {
case MessageListHistoryStartItem():
return Center(
Expand All @@ -685,7 +684,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
return MessageItem(
key: ValueKey(data.message.id),
header: header,
trailingWhitespace: i == 1 ? 8 : 11,
trailingWhitespace: 11,
item: data);
}
}
Expand Down
Loading