-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Add support for pointer scrolling to trigger floats & snaps #76145
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
Conversation
|
This pr does not seem to fix the repro in the original issue import 'package:flutter/material.dart';
void main() {
runApp(SliverPesistentHeaderScrollUpMouse());
}
class SliverPesistentHeaderScrollUpMouse extends StatelessWidget {
@override
Widget build(BuildContext context) {
List<Widget> _widgets = [];
for (int i = 0; i < 200; i++) {
_widgets.add(
Container(
height: 10,
color: Colors.blue,
child: Text('$i'),
)
);
}
return MaterialApp(
title: 'Default label',
home: Scaffold(
body: CustomScrollView(
slivers: [
SliverPersistentHeader(
floating: true,
delegate: Header(),
),
SliverList(
delegate: SliverChildListDelegate(
_widgets
),
)
],
)
)
);
}
}
class Header extends SliverPersistentHeaderDelegate {
@override
Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
return Container(
height: 50,
color: Colors.red,
);
}
@override
double get maxExtent => 50;
@override
double get minExtent => 50;
@override
bool shouldRebuild(SliverPersistentHeaderDelegate oldDelegate) {
return false;
}
}
I have tried on macos and flutter for web |
Oh wow that's bizarre, sorry about that. Let me see what is going on. |
|
Aah I see, I forgot |
|
All set now I think. :) PTAL |
| } | ||
|
|
||
| void _isScrollingListener() { | ||
| if (_position == null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert instead?
| } | ||
| } | ||
|
|
||
| class _FloatingHeader extends StatefulWidget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this merge into the _SliverFloatingPersistentHeader and _SliverFloatingPinnedPersistentHeader, probably as mixin? you may need to change the two widgets to statefulwidget or somehow inject the logic into _SliverPersistentHeaderElement.
It feels a bit convoluted that how it is inserted into the widget tree and can only work with floating persistent header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I tried that first, but then I could not find the RenderSliverFloatingPersistentHeader as an ancestor from that point. I'll look again and see if there is another way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can do something like context.findRenderObject to get the renderobject (which should be RenderSliverFloatingPersistentHeader if there is no additional renderobject in between _SliverFloatingPersistentHeader and RenderSliverFloatingPersistentHeader)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so the structure i am imagine is the _SliverFloatingPersistentHeader listens to the scrollable and updates its renderobject(assuming it is RenderSliverFloatingPersistentHeader) based on it.
In terms of how _SliverFloatingPersistentHeader can listen to scrollable, you can either change it to a stateful widget (probably overkill), or makes _SliverFloatingPersistentHeader inject some callback into _SliverPersistentHeaderElement, and figure out how _SliverPersistentHeaderElement can listen to inherited widget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not figures out how to do this yet, not sure if I am understanding completely. _SliverFloatingPersistentHeader is a RenderObjectWidget, and _SliverPersistentHeaderElement is a RenderObjectElement.
My original plan here in introducing _FloatingHeader was to follow to same pattern we use for SliverAppBar, which applies a _FloatingAppBar in order to listen to the scroll events. Should this be rewritten? It would probably be better for maintainability if we used the same logic one way or the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah we have similar situation in the appbar as well. If we have it here, do we still need the logic in _FloatingAppBar? it looks the scrolling maybe listen twice for SliverAppBar?
|
I am going to close this for now while we investigate the issue further. |
|
We've gotten some valid use cases reported on the bug, so I am re-opening this. :) |
|
in case it gets buried in the thread, I replied #76145 (comment) |
|
@Piinks What's the status of this PR? (Desktop Triage) |
|
Thanks @gspencergoog! Just pushed the latest updates, @chunhtai PTAL :) |
chunhtai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This adds support for pointer scrolling to be able to trigger the floating and snapping effects of
SliverPersistentHeaders,SliverAppBars, etc.Fixes #75059
Unblocks #71322 / #75728
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.