diff --git a/lib/features/chat/views/chat_page.dart b/lib/features/chat/views/chat_page.dart index d9c947e6..60b08d7c 100644 --- a/lib/features/chat/views/chat_page.dart +++ b/lib/features/chat/views/chat_page.dart @@ -251,6 +251,8 @@ class _PinToTopState { final String? userMessageId; final String? streamingMessageId; + _PinToTopState preserveForUserScroll() => this; + _PinToTopState dismiss({bool preserveStreamingId = false}) { if (!preserveStreamingId) { return const _PinToTopState.inactive(); @@ -263,6 +265,42 @@ class _PinToTopState { } } +class _PinToTopScrollPhysics extends ScrollPhysics { + const _PinToTopScrollPhysics({ + required this.maximumScrollOffset, + super.parent, + }); + + final double? Function(ScrollMetrics position) maximumScrollOffset; + + @override + _PinToTopScrollPhysics applyTo(ScrollPhysics? ancestor) { + return _PinToTopScrollPhysics( + maximumScrollOffset: maximumScrollOffset, + parent: buildParent(ancestor), + ); + } + + @override + double applyBoundaryConditions(ScrollMetrics position, double value) { + final configuredMaximum = maximumScrollOffset(position); + if (configuredMaximum != null && configuredMaximum.isFinite) { + final effectiveMaximum = configuredMaximum + .clamp(position.minScrollExtent, position.maxScrollExtent) + .toDouble(); + final overflow = _scrollOverflowBeyondMaximum( + currentOffset: position.pixels, + proposedOffset: value, + maximumOffset: effectiveMaximum, + ); + if (overflow != 0) { + return overflow; + } + } + return super.applyBoundaryConditions(position, value); + } +} + class ChatPage extends ConsumerStatefulWidget { const ChatPage({super.key}); @@ -303,6 +341,7 @@ class _ChatPageState extends ConsumerState { // Pin-to-top: scroll user message to top of viewport when sending _PinToTopState _pinToTopState = const _PinToTopState.inactive(); GlobalKey _pinnedUserMessageKey = GlobalKey(); + double? _pinToTopMaximumScrollOffset; final _stableLayoutCache = _ChatListStableLayoutCache(); _ChatListStableLayoutMetadata? _lastExtentCacheInvalidationMetadata; String? _cachedGreetingName; @@ -322,6 +361,18 @@ class _ChatPageState extends ConsumerState { bool get _wantsPinToTop => _pinToTopState.isActive; String? get _pinnedUserMessageId => _pinToTopState.userMessageId; String? get _pinnedStreamingId => _pinToTopState.streamingMessageId; + double? _activePinToTopMaximumScrollOffset(ScrollMetrics position) { + final pinnedPromptOffset = _pinToTopMaximumScrollOffset; + if (!_wantsPinToTop || pinnedPromptOffset == null) { + return null; + } + return _maximumPinToTopScrollOffset( + pinnedPromptOffset: pinnedPromptOffset, + maxScrollExtent: position.maxScrollExtent, + phantomExtent: _pinToTopPhantomScrollExtent(), + ); + } + bool get _isUserInteractingWithScroll => _bottomAnchorController.isUserInteractingWithScroll; set _isUserInteractingWithScroll(bool value) { @@ -1812,10 +1863,13 @@ class _ChatPageState extends ConsumerState { final targetTop = renderObject.localToGlobal(Offset.zero).dy; final currentOffset = _scrollController.offset; final maxScroll = _scrollController.position.maxScrollExtent; - final targetOffset = (currentOffset + targetTop - topInset).clamp( - 0.0, - maxScroll, - ); + final targetOffset = (currentOffset + targetTop - topInset) + .clamp(0.0, maxScroll) + .toDouble(); + // The phantom sliver makes this offset reachable for short conversations. + // Keep the alignment as the minimum lower boundary while the response is + // short; the boundary expands with real response content as it grows. + _pinToTopMaximumScrollOffset = targetOffset; if ((targetOffset - currentOffset).abs() < 1.0) { return; @@ -1914,50 +1968,12 @@ class _ChatPageState extends ConsumerState { bool _endPinToTopInFlight = false; - void _dismissPinToTop({bool preserveStreamingId = false}) { - if (!_wantsPinToTop || !mounted) { - return; - } - setState(() { - _pinToTopState = _pinToTopState.dismiss( - preserveStreamingId: preserveStreamingId, - ); - }); - } - - bool _canDismissPinToTopWithoutViewportJump() { - if (!_scrollController.hasClients) { - return true; - } - final position = _scrollController.position; - return _canRemovePinToTopPhantomWithoutViewportJump( - currentOffset: position.pixels, - maxScrollExtent: position.maxScrollExtent, - phantomExtent: _pinToTopPhantomScrollExtent(), - epsilon: _scrollCorrectionEpsilon, - ); - } - - void _dismissPinToTopAfterUserScroll({bool preserveStreamingId = false}) { - if (!_wantsPinToTop || !mounted) { - return; - } - if (!_canDismissPinToTopWithoutViewportJump()) { - return; - } - _dismissPinToTop(preserveStreamingId: preserveStreamingId); - } - /// Transitions out of pin-to-top mode. /// /// When [instant] is true, uses jumpTo to avoid competing with streaming /// row-size corrections. void _endPinToTop({bool instant = false, bool preserveStreamingId = false}) { if (!_wantsPinToTop || !mounted || _endPinToTopInFlight) return; - if (_isUserInteractingWithScroll && !instant) { - _dismissPinToTopAfterUserScroll(preserveStreamingId: preserveStreamingId); - return; - } if (!_scrollController.hasClients) { setState(() { _pinToTopState = _pinToTopState.dismiss( @@ -2189,6 +2205,7 @@ class _ChatPageState extends ConsumerState { : null; if (parentUserId != null && _pinnedStreamingId != tailAssistant.id) { // New streaming response detected + _pinToTopMaximumScrollOffset = null; _pinToTopState = _PinToTopState.active( userMessageId: parentUserId, streamingMessageId: tailAssistant.id, @@ -2237,10 +2254,13 @@ class _ChatPageState extends ConsumerState { notification is UserScrollNotification && notification.direction == ScrollDirection.idle; - // User scrolling dismisses pin-to-top once the user takes control. + // User scrolling pauses automatic bottom anchoring but preserves + // pin-to-top. This lets the user browse older messages and return to + // the same pinned lower boundary without losing the prompt anchor. if (isTouchDragStart || isUserScrollUpdate || isUserDirectionalScroll) { + _pinToTopState = _pinToTopState.preserveForUserScroll(); if (!_isUserInteractingWithScroll) { _bottomScrollSettler.cancel(); _cancelPendingInitialBottomSettle(); @@ -2260,22 +2280,11 @@ class _ChatPageState extends ConsumerState { try { ref.read(composerAutofocusEnabledProvider.notifier).set(false); } catch (_) {} - if (_wantsPinToTop) { - _dismissPinToTopAfterUserScroll(preserveStreamingId: true); - } } if (notification is ScrollEndNotification || isUserScrollIdle) { _endScrollProfile(reason: 'idle'); - final wasInteracting = _isUserInteractingWithScroll; _isUserInteractingWithScroll = false; _updateBottomAnchorTracking(); - // Re-check after the final drag update, but keep the same no-jump - // guard used during the gesture. For short conversations, removing - // the phantom spacer here would clamp the second prompt's offset to - // zero and visibly animate back to the first message. - if (wasInteracting && _wantsPinToTop) { - _dismissPinToTopAfterUserScroll(preserveStreamingId: true); - } } return false; // Allow notification to continue bubbling }, @@ -2283,8 +2292,11 @@ class _ChatPageState extends ConsumerState { key: const ValueKey('actual_messages'), controller: _scrollController, keyboardDismissBehavior: ScrollViewKeyboardDismissBehavior.onDrag, - physics: SuperRangeMaintainingScrollPhysics( - parent: platformAlwaysScrollablePhysics(context), + physics: _PinToTopScrollPhysics( + maximumScrollOffset: _activePinToTopMaximumScrollOffset, + parent: SuperRangeMaintainingScrollPhysics( + parent: platformAlwaysScrollablePhysics(context), + ), ), scrollCacheExtent: const ScrollCacheExtent.pixels(600), slivers: [ @@ -3850,6 +3862,44 @@ double _scrollAnimationStartOffset({ .toDouble(); } +double _scrollOverflowBeyondMaximum({ + required double currentOffset, + required double proposedOffset, + required double maximumOffset, +}) { + if (!currentOffset.isFinite || + !proposedOffset.isFinite || + !maximumOffset.isFinite) { + return 0.0; + } + if (maximumOffset <= currentOffset && currentOffset < proposedOffset) { + return proposedOffset - currentOffset; + } + if (currentOffset < maximumOffset && maximumOffset < proposedOffset) { + return proposedOffset - maximumOffset; + } + return 0.0; +} + +double _maximumPinToTopScrollOffset({ + required double pinnedPromptOffset, + required double maxScrollExtent, + required double phantomExtent, +}) { + if (!pinnedPromptOffset.isFinite || + !maxScrollExtent.isFinite || + !phantomExtent.isFinite) { + return pinnedPromptOffset; + } + final maxWithoutPhantom = (maxScrollExtent - phantomExtent) + .clamp(0.0, maxScrollExtent) + .toDouble(); + return math + .max(pinnedPromptOffset, maxWithoutPhantom) + .clamp(0.0, maxScrollExtent) + .toDouble(); +} + bool _shouldKeepConversationBottomAnchoredOnInsetChange({ required double previousBottomInset, required double nextBottomInset, @@ -3881,20 +3931,6 @@ double _scrollOffsetAfterRemovingPinToTopPhantom({ return currentOffset.clamp(0.0, maxWithoutPhantom).toDouble(); } -bool _canRemovePinToTopPhantomWithoutViewportJump({ - required double currentOffset, - required double maxScrollExtent, - required double phantomExtent, - required double epsilon, -}) { - final targetOffset = _scrollOffsetAfterRemovingPinToTopPhantom( - currentOffset: currentOffset, - maxScrollExtent: maxScrollExtent, - phantomExtent: phantomExtent, - ); - return (currentOffset - targetOffset).abs() <= epsilon; -} - double _estimateMessageListExtentForIndex( _ChatListStableLayoutMetadata layoutMetadata, int? index, @@ -4022,6 +4058,52 @@ double debugScrollAnimationStartOffsetForTesting({ ); } +@visibleForTesting +double debugScrollOverflowBeyondMaximumForTesting({ + required double currentOffset, + required double proposedOffset, + required double maximumOffset, +}) { + return _scrollOverflowBeyondMaximum( + currentOffset: currentOffset, + proposedOffset: proposedOffset, + maximumOffset: maximumOffset, + ); +} + +@visibleForTesting +bool debugPinToTopRemainsActiveAfterUserScrollForTesting() { + const state = _PinToTopState.active( + userMessageId: 'user', + streamingMessageId: 'assistant', + ); + return state.preserveForUserScroll().isActive; +} + +@visibleForTesting +ScrollPhysics debugPinToTopScrollPhysicsForTesting({ + required double? Function(ScrollMetrics position) maximumScrollOffset, + ScrollPhysics? parent, +}) { + return _PinToTopScrollPhysics( + maximumScrollOffset: maximumScrollOffset, + parent: parent, + ); +} + +@visibleForTesting +double debugMaximumPinToTopScrollOffsetForTesting({ + required double pinnedPromptOffset, + required double maxScrollExtent, + required double phantomExtent, +}) { + return _maximumPinToTopScrollOffset( + pinnedPromptOffset: pinnedPromptOffset, + maxScrollExtent: maxScrollExtent, + phantomExtent: phantomExtent, + ); +} + @visibleForTesting bool debugShouldKeepConversationBottomAnchoredOnInsetChangeForTesting({ required double previousBottomInset, @@ -4065,21 +4147,6 @@ double debugScrollOffsetAfterRemovingPinToTopPhantomForTesting({ ); } -@visibleForTesting -bool debugCanRemovePinToTopPhantomWithoutViewportJumpForTesting({ - required double currentOffset, - required double maxScrollExtent, - required double phantomExtent, - double epsilon = 1.0, -}) { - return _canRemovePinToTopPhantomWithoutViewportJump( - currentOffset: currentOffset, - maxScrollExtent: maxScrollExtent, - phantomExtent: phantomExtent, - epsilon: epsilon, - ); -} - @visibleForTesting double debugEstimateMessageListExtentForTesting( List messages, { diff --git a/test/features/chat/views/chat_page_layout_metadata_test.dart b/test/features/chat/views/chat_page_layout_metadata_test.dart index 166fa258..b8bba07d 100644 --- a/test/features/chat/views/chat_page_layout_metadata_test.dart +++ b/test/features/chat/views/chat_page_layout_metadata_test.dart @@ -1134,18 +1134,7 @@ void main() { expect(shouldKeepBottomAnchored, isFalse); }); - test('pin-to-top user scroll keeps phantom until removal is stable', () { - // Two short turns can leave the second prompt pinned beyond the real - // (phantom-free) scroll range. Ending the first drag must not force this - // offset to zero, which would jump back to the first message (#560). - expect( - debugCanRemovePinToTopPhantomWithoutViewportJumpForTesting( - currentOffset: 500, - maxScrollExtent: 800, - phantomExtent: 800, - ), - isFalse, - ); + test('pin-to-top removal clamps offsets to the phantom-free range', () { expect( debugScrollOffsetAfterRemovingPinToTopPhantomForTesting( currentOffset: 500, @@ -1155,14 +1144,6 @@ void main() { 0, ); - expect( - debugCanRemovePinToTopPhantomWithoutViewportJumpForTesting( - currentOffset: 920, - maxScrollExtent: 1200, - phantomExtent: 400, - ), - isFalse, - ); expect( debugScrollOffsetAfterRemovingPinToTopPhantomForTesting( currentOffset: 920, @@ -1172,14 +1153,6 @@ void main() { 800, ); - expect( - debugCanRemovePinToTopPhantomWithoutViewportJumpForTesting( - currentOffset: 760, - maxScrollExtent: 1200, - phantomExtent: 400, - ), - isTrue, - ); expect( debugScrollOffsetAfterRemovingPinToTopPhantomForTesting( currentOffset: 760, @@ -1190,6 +1163,137 @@ void main() { ); }); + test('pin-to-top boundary keeps the pinned prompt in view', () { + expect( + debugScrollOverflowBeyondMaximumForTesting( + currentOffset: 480, + proposedOffset: 560, + maximumOffset: 500, + ), + 60, + ); + expect( + debugScrollOverflowBeyondMaximumForTesting( + currentOffset: 500, + proposedOffset: 650, + maximumOffset: 500, + ), + 150, + ); + expect( + debugScrollOverflowBeyondMaximumForTesting( + currentOffset: 560, + proposedOffset: 600, + maximumOffset: 500, + ), + 40, + ); + expect( + debugScrollOverflowBeyondMaximumForTesting( + currentOffset: 560, + proposedOffset: 520, + maximumOffset: 500, + ), + 0, + ); + }); + + testWidgets( + 'pin-to-top physics stops a short response at the pinned prompt', + (tester) async { + final controller = ScrollController(); + addTearDown(controller.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 300, + child: CustomScrollView( + controller: controller, + physics: debugPinToTopScrollPhysicsForTesting( + maximumScrollOffset: (position) => + debugMaximumPinToTopScrollOffsetForTesting( + pinnedPromptOffset: 250, + maxScrollExtent: position.maxScrollExtent, + phantomExtent: 800, + ), + parent: const SuperRangeMaintainingScrollPhysics( + parent: BouncingScrollPhysics( + parent: AlwaysScrollableScrollPhysics(), + ), + ), + ), + slivers: const [ + SliverToBoxAdapter(child: SizedBox(height: 1200)), + ], + ), + ), + ), + ), + ); + + expect(controller.position.maxScrollExtent, 900); + await tester.drag(find.byType(CustomScrollView), const Offset(0, -1000)); + await tester.pumpAndSettle(); + + expect(controller.offset, closeTo(250, 0.01)); + + await tester.drag(find.byType(CustomScrollView), const Offset(0, 100)); + await tester.pumpAndSettle(); + + expect(controller.offset, lessThan(250)); + }, + ); + + testWidgets( + 'pin-to-top physics scrolls through a long response but blocks phantom space', + (tester) async { + final controller = ScrollController(); + addTearDown(controller.dispose); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: SizedBox( + height: 300, + child: CustomScrollView( + controller: controller, + physics: debugPinToTopScrollPhysicsForTesting( + maximumScrollOffset: (position) => + debugMaximumPinToTopScrollOffsetForTesting( + pinnedPromptOffset: 250, + maxScrollExtent: position.maxScrollExtent, + phantomExtent: 300, + ), + parent: const SuperRangeMaintainingScrollPhysics( + parent: BouncingScrollPhysics( + parent: AlwaysScrollableScrollPhysics(), + ), + ), + ), + slivers: const [ + SliverToBoxAdapter(child: SizedBox(height: 1200)), + ], + ), + ), + ), + ), + ); + + expect(controller.position.maxScrollExtent, 900); + await tester.drag(find.byType(CustomScrollView), const Offset(0, -1000)); + await tester.pumpAndSettle(); + + // 900 total extent - 300 phantom extent = 600 real-content extent. + expect(controller.offset, closeTo(600, 0.01)); + }, + ); + + test('manual scrolling preserves active pin-to-top state', () { + expect(debugPinToTopRemainsActiveAfterUserScrollForTesting(), isTrue); + }); + test( 'keyboard inset growth ignores pin-to-top mode and manual scrolling', () {