SliverResizingHeader #143325
SliverResizingHeader #143325
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
github-actions
bot
added
framework
HansMuller
force-pushed
the
resizing_header_sliver
branch
from
c64e382 to
da54325
Compare
There was a problem hiding this comment.
Still carefully reviewing all the related PRs🙂. Just FTR, here we probably need to use a ExcludeFocus to wrap the minExtentPrototype and maxExtentPrototype; Otherwise, if we put some tappable widgets here, when we do focus traversal by just using tab key, these two would also be traversed, even though they are not really painted. Probably we don't need IgnorePointer here because hitTest doesn't include them:)
There was a problem hiding this comment.
Good point, I will fix this stuff and add some tests to check.
There was a problem hiding this comment.
This feels odd, should it fallback on the min extent if it is provided? If I provided a minExtentPrototype, but not a max, what will the layout be?
There was a problem hiding this comment.
Good point. If a max isn't provided it wouldn't be surprising if you got a sliver whose sized matched the min prototype.
There was a problem hiding this comment.
Should this included Pinned in the name? I did not realize it would be until I read the docs. This made me think, why not merge this with the PinnedHeaderSliver? Where if the prototypes are unset, it just uses the child size?
Actually, more simply put, when the prototypes are not provided, does this act exactly like PinnedHeaderSliver?
There was a problem hiding this comment.
Oh also same comment re: Sliver as a prefix.
There was a problem hiding this comment.
If the minimum size defaulted to the child's size (instead of 0) then having this sliver effectively be a PinnedHeaderSliver would work. I'd still prefer to have a PinnedHeaderSliver because it's nice if the class says what you mean, although it could be implemented as a trivial wrapper for this one, or this one's implementation.
There was a problem hiding this comment.
I've made the minimum default 0 and the maximum default the intrinsic size of the child. So if you just specify a child you get a header that begins with the child's intrinsic size and shrinks to zero. If the same widget is provided for the minimum and maximum prototypes you get a pinned header.
There was a problem hiding this comment.
Should the docs on these prototypes say that these widgets will not be made visible?
There was a problem hiding this comment.
Looking at this, is zero not true then, when the prototype is null?
There was a problem hiding this comment.
The default for maxExtent is now computed differently, see #143325 (comment)
There was a problem hiding this comment.
It looks like this should not happen, should this assert the box is not null then?
There was a problem hiding this comment.
I've made the parameter non-null.
There was a problem hiding this comment.
Can we add a todo to add See alsos to all these new slivers that can be combined together after they all land?
There was a problem hiding this comment.
Similar comment to the others, there should be a test for each permutation of [vertical, horizontal] : [reversed, not].
HansMuller
force-pushed
the
resizing_header_sliver
branch
from
402608f to
0356054
Compare
HansMuller
force-pushed
the
resizing_header_sliver
branch
from
0356054 to
ce75256
Compare
…l notification (#146654) The scroll notification events reported for a press-drag-release gesture within a scrollable on a touch screen device begin with a `ScrollStartNotification`, followed by a series of `ScrollUpdateNotifications`, and conclude with a `ScrollEndNotification`. This protocol can be used to defer work until an interactive scroll gesture ends. For example, you might defer updating a scrollable's contents via network requests until the scroll has ended, or you might want to automatically auto-scroll at that time. In the example that follows the CustomScrollView automatically scrolls so that the last partially visible fixed-height item is completely visible when the scroll gesture ends. Many iOS applications do this kind of thing. It only makes sense to auto-scroll when the user isn't actively dragging the scrollable around. It's easy enough to do this by reacting to a ScrollEndNotifcation by auto-scrolling to align the last fixed-height list item ([source code](https://gist.github.com/HansMuller/13e2a7adadc9afb3803ba7848b20c410)). https://github.com/flutter/flutter/assets/1377460/a6e6fc77-6742-4f98-81ba-446536535f73 Dragging the scrollbar thumb in a desktop application is a similar user gesture. Currently it's not possible to defer work or auto-scroll (or whatever) while the scrollable is actively being dragged via the scrollbar thumb because each scrollbar thumb motion is mapped to a scroll start - scroll update - scroll end series of notifications. On a desktop platform, the same code behaves quite differently when the scrollbar thumb is dragged. https://github.com/flutter/flutter/assets/1377460/2593d8a3-639c-407f-80c1-6e6f67fb8c5f The stream of scroll-end events triggers auto-scrolling every time the thumb moves. From the user's perspective this feels like a losing struggle. One can also detect the beginning and end of a touch-drag by listening to the value of a ScrollPosition's `isScrollingNotifier`. This approach suffers from a similar problem: during a scrollbar thumb-drag, the `isScrollingNotifier` value isn't updated at all. This PR refactors the RawScrollbar implementation to effectively use a ScrollDragController to manage scrolls caused by dragging the scrollbar's thumb. Doing so means that dragging the thumb will produce the same notifications as dragging the scrollable on a touch device. Now desktop applications can choose to respond to scrollbar thumb drags in the same that they respond to drag scrolling on a touch screen. With the changes included here, the desktop or web version of the app works as expected, whether you're listing to scroll notifications or the scroll position's `isScrollingNotifier`. https://github.com/flutter/flutter/assets/1377460/67435c40-a866-4735-a19b-e3d68eac8139 This PR also makes the second [ScrollPosition API doc example](https://api.flutter.dev/flutter/widgets/ScrollPosition-class.html#cupertino.ScrollPosition.2) work as expected when used with the DartPad that's part of API doc page. Desktop applications also see scroll start-update-end notifications due to the mouse wheel. There is no touch screen analog for the mouse wheel, so an application that wanted to enable this kind of auto-scrolling alignment would have to include a heuristic that dealt with the sequence of small scrolls triggered by the mouse wheel. Here's an example of that: [source code](https://gist.github.com/HansMuller/ce5c474a458f5f4bcc07b0d621843165). This version of the app does not auto-align in response to small changes, wether they're triggered by dragging the scrollbar thumb of the mouse wheel. Related sliver utility PRs: #143538, #143196, #143325.
|
Pining into my inbox/review queue :) |
| /// ** See code in examples/api/lib/widgets/sliver/sliver_resizing_header.0.dart ** | ||
| /// {@end-tool} | ||
| // TODO(hansmuller): add See also links to PersistentHeaderSliver, SliverFloatingHeader, etc | ||
| class SliverResizingHeader extends StatelessWidget { |
There was a problem hiding this comment.
Since this is a pinned sliver, and there is another PR of this set that is pinned and includes Pinned, I still feel like it should have Pinned in the title, but I'll defer to your judgement.
There was a problem hiding this comment.
Just to support the idea a bit further, folks searching for Pinned in the API docs will find the other one, and not this option.
There was a problem hiding this comment.
Header also means "appears at the top" and I'd really prefer to avoid long compound names. In this case "resizing" emphasizes what's special about this header, as pinned does for PinnedHeaderSliver. I'll update the see also links in both classes to make sure that if someone finds the wrong header it will be a short trip to find the correct one.
HansMuller
added
the
autosubmit
Manual roll Flutter from 4608a89 to fc19ecf (38 revisions) Manual roll requested by [email protected] flutter/flutter@4608a89...fc19ecf 2024-06-07 [email protected] Fix `SegmentedButton` clipping when drawing segments (flutter/flutter#149739) 2024-06-07 [email protected] Shift consistently passing tests on mokey to prod (flutter/flutter#149902) 2024-06-07 [email protected] Prepare images for tests individually to enable clean up of cache. (flutter/flutter#149693) 2024-06-07 [email protected] Roll Packages from 586faa6 to 8a2c4e4 (8 revisions) (flutter/flutter#149900) 2024-06-07 [email protected] Roll Flutter Engine from 98e9393bf2e9 to 1cdbebee1901 (4 revisions) (flutter/flutter#149899) 2024-06-07 [email protected] Roll Flutter Engine from 087feab9922b to 98e9393bf2e9 (1 revision) (flutter/flutter#149875) 2024-06-07 [email protected] Refactor `widget_inspector_test.dart` (flutter/flutter#149850) 2024-06-07 [email protected] Add test for dropdown_menu.1.dart (flutter/flutter#149547) 2024-06-07 [email protected] Add test for standard_fab_location.0.dart (flutter/flutter#149225) 2024-06-07 [email protected] [native_assets] Fix framework name deduplication (flutter/flutter#149761) 2024-06-07 [email protected] Roll Flutter Engine from 47b15dfba1da to 087feab9922b (1 revision) (flutter/flutter#149871) 2024-06-07 [email protected] Roll Flutter Engine from 5d5e0ee9102c to 47b15dfba1da (2 revisions) (flutter/flutter#149870) 2024-06-07 [email protected] Roll Flutter Engine from fea8e51b0280 to 5d5e0ee9102c (2 revisions) (flutter/flutter#149867) 2024-06-07 [email protected] Roll Flutter Engine from 5a0a49939c64 to fea8e51b0280 (2 revisions) (flutter/flutter#149865) 2024-06-07 [email protected] Roll pub packages (flutter/flutter#149864) 2024-06-06 [email protected] Roll Flutter Engine from bf1a4877da0b to 5a0a49939c64 (1 revision) (flutter/flutter#149861) 2024-06-06 [email protected] Remove some vestigial /*!*/ comments (flutter/flutter#149361) 2024-06-06 [email protected] Attempt to fix some web test flakiness. (flutter/flutter#149828) 2024-06-06 [email protected] Remove abi key permanently from golden file testing (flutter/flutter#149858) 2024-06-06 [email protected] [CupertinoActionSheet] Add sliding tap gesture (flutter/flutter#149471) 2024-06-06 [email protected] Roll pub packages (flutter/flutter#149852) 2024-06-06 [email protected] SliverResizingHeader (flutter/flutter#143325) 2024-06-06 [email protected] Run tests on macOS 13 or 14 (flutter/flutter#149827) 2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149823) 2024-06-06 [email protected] Roll Flutter Engine from 31804a222f97 to bf1a4877da0b (3 revisions) (flutter/flutter#149848) 2024-06-06 [email protected] Add support for setting the heading level for web semantics (#97894) (flutter/flutter#125771) 2024-06-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Introduce `ChipAnimationStyle` to override default chips animations durations (#149245)" (flutter/flutter#149847) 2024-06-06 [email protected] 3.22.2 release notes (flutter/flutter#149818) 2024-06-06 [email protected] Roll Flutter Engine from 25bcbb888939 to 31804a222f97 (3 revisions) (flutter/flutter#149837) 2024-06-06 [email protected] Introduce `ChipAnimationStyle` to override default chips animations durations (flutter/flutter#149245) 2024-06-06 [email protected] Roll Flutter Engine from f5a9690c6061 to 25bcbb888939 (1 revision) (flutter/flutter#149835) 2024-06-06 [email protected] Fix leaky test. (flutter/flutter#149822) 2024-06-06 [email protected] Unpin archive package from tool, update to version without security advisories (flutter/flutter#149430) 2024-06-06 [email protected] Roll pub packages (flutter/flutter#149630) 2024-06-06 [email protected] Roll Flutter Engine from d9684a37af06 to f5a9690c6061 (7 revisions) (flutter/flutter#149832) 2024-06-06 [email protected] Roll Packages from 11e192a to 586faa6 (13 revisions) (flutter/flutter#149820) 2024-06-06 [email protected] Fix some links in the "Handling breaking change" section (flutter/flutter#149821) 2024-06-06 [email protected] Roll Flutter Engine from 92d0cd9370f7 to d9684a37af06 (1 revision) (flutter/flutter#149793) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...
A sliver that remains �pinned� to the top of the scroll view. Subsequent slivers scroll behind it. Typically the sliver is created as the first item in the list however it can be inserted anywhere and it will always stop at the top of the scroll view. When the scrolling axis is vertical, the PinnedHeaderSliver�s height is defined by its widget child. Multiple PinnedHeaderSlivers will layout one after the other, once they've scrolled to the top. This sliver is preferable to the general purpose SliverPersistentHeader - for its relatively narrow use case - because there's no need to create a [SliverPersistentHeaderDelegate] or to predict the header's size. Here's a [working demo in DartPad](https://dartpad.dev/?id=3b3f24c14fa201f752407a21ca9c9456). https://github.com/flutter/flutter/assets/1377460/943f2e02-8e73-48b7-90be-61168978ff71 Related sliver utility PRs: #143538, #143325, #127340.
3 participants
A sliver that is pinned to the start of its
CustomScrollViewand reacts to scrolling by resizing between the intrinsic sizes of its min and max extent prototypes.The minimum and maximum sizes of this sliver are defined by
minExtentPrototypeandmaxExtentPrototype, a pair of widgets that are laid out once. You can useSizedBoxwidgets to define the size limits.This sliver is preferable to the general purpose
SliverPersistentHeaderfor its relatively narrow use case because there's no need to create aSliverPersistentHeaderDelegateor to predict the header's minimum or maximum size.The sample shows how this sliver's two extent prototype properties can be used to create a resizing header whose minimum and maximum sizes match small and large configurations of the same header widget.
Screen.Recording.2024-02-12.at.6.05.27.PM.mov
Related sliver utility PRs: #143538, #143196, #127340.