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.

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

SliverPersistentHeaderDelegate.build overlapsContent does not work reliably #26667

Open
maxlapides opened this issue Jan 16, 2019 · 11 comments
Open
Labels
customer: flex f: scrolling Viewports, list views, slivers, etc. found in release: 1.21 Found to occur in 1.21 found in release: 2.10 Found to occur in 2.10 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-framework Owned by Framework team triaged-framework Triaged by Framework team

Comments

@maxlapides
Copy link

I'm trying to add a border underneath of a sliver in a CustomScrollView once that sliver is pinned to the top of the scroll container, so I created a SliverPersistentHeader, but overlapsContent in my SliverPersistentHeaderDelegate.build method is always false.

After doing some digging, I think this issue can be reliably reproduced on any SliverPersistentHeader that is the FIRST SliverPersistentHeader in a CustomScrollView.

For example, let's work with the collapsible scrolling list demonstrated at the end of this article: https://medium.com/flutter-io/slivers-demystified-6ff68ab0296f#bde5. Let's modify the _SliverAppBarDelegate.build method as such:

Widget build(
  BuildContext context,
  double shrinkOffset,
  bool overlapsContent,
) {
  return SizedBox.expand(
    child: Container(
      color: overlapsContent ? Colors.red : Colors.lightBlue,
      child: child,
    ),
  );
}

And let's update makeHeader as such:

SliverPersistentHeader makeHeader(String headerText) {
  return SliverPersistentHeader(
    pinned: true,
    delegate: _SliverAppBarDelegate(
      minHeight: 60,
      maxHeight: 200,
      child: Center(child: Text(headerText)),
    ),
  );
}

These changes should make it so when a header becomes pinned, its background turns from blue to red. If you actually test this out, this change works for all of the headers EXCEPT for the first one.

❯ flutter doctor -v
[✓] Flutter (Channel beta, v1.0.0, on Mac OS X 10.14.2 18C54, locale en-US)
    • Flutter version 1.0.0 at /Users/maxlapides/flutter
    • Framework revision 5391447fae (7 weeks ago), 2018-11-29 19:41:26 -0800
    • Engine revision 7375a0f414
    • Dart version 2.1.0 (build 2.1.0-dev.9.4 f9ebf21297)

[✓] Android toolchain - develop for Android devices (Android SDK 27.0.3)
    • Android SDK at /Users/maxlapides/Library/Android/sdk
    • Android NDK location not configured (optional; useful for native profiling support)
    • Platform android-27, build-tools 27.0.3
    • ANDROID_HOME = /Users/maxlapides/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1136-b06)
    • All Android licenses accepted.

[✓] iOS toolchain - develop for iOS devices (Xcode 10.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 10.1, Build version 10B61
    • ios-deploy 1.9.4
    • CocoaPods version 1.6.0.beta.1

[✓] Android Studio (version 3.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 31.3.1
    • Dart plugin version 181.5656
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1136-b06)

[✓] VS Code (version 1.30.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 2.21.1

[✓] Connected device (2 available)
    • Max's iPhone • 65760f04a8cdf8dc55bf798b8cf0ab5cf488674c • ios • iOS 12.1.2
    • iPhone XS    • 9CFC4B23-BE03-4DC9-B1CD-5E1226F5A183     • ios • iOS 12.1 (simulator)

• No issues found!
@maxlapides
Copy link
Author

maxlapides commented Jan 16, 2019

To make it easier to reproduce, here's the complete example:

code sample
import 'package:flutter/material.dart';
import 'dart:math' as math;

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      home: Scaffold(
        appBar: AppBar(title: Text('Collapsing List Demo')),
        body: CollapsingList(),
      ),
    );
  }
}

class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate {
  _SliverAppBarDelegate({
    required this.minHeight,
    required this.maxHeight,
    required this.child,
  });
  final double minHeight;
  final double maxHeight;
  final Widget child;

  @override
  double get minExtent => minHeight;

  @override
  double get maxExtent => math.max(maxHeight, minHeight);

  @override
  Widget build(
    BuildContext context,
    double shrinkOffset,
    bool overlapsContent,
  ) {
    return SizedBox.expand(
      child: Container(
        color: overlapsContent ? Colors.red : Colors.lightBlue,
        child: child,
      ),
    );
  }

  @override
  bool shouldRebuild(_SliverAppBarDelegate oldDelegate) {
    return maxHeight != oldDelegate.maxHeight ||
        minHeight != oldDelegate.minHeight ||
        child != oldDelegate.child;
  }
}

class CollapsingList extends StatelessWidget {
  SliverPersistentHeader makeHeader(String headerText) {
    return SliverPersistentHeader(
      pinned: true,
      delegate: _SliverAppBarDelegate(
        minHeight: 60,
        maxHeight: 200,
        child: Center(child: Text(headerText)),
      ),
    );
  }

  @override
  Widget build(BuildContext context) {
    return CustomScrollView(
      slivers: <Widget>[
        makeHeader('Header Section 1'),
        SliverGrid.count(
          crossAxisCount: 3,
          children: [
            Container(color: Colors.red, height: 150.0),
            Container(color: Colors.purple, height: 150.0),
            Container(color: Colors.green, height: 150.0),
            Container(color: Colors.orange, height: 150.0),
            Container(color: Colors.yellow, height: 150.0),
            Container(color: Colors.pink, height: 150.0),
            Container(color: Colors.cyan, height: 150.0),
            Container(color: Colors.indigo, height: 150.0),
            Container(color: Colors.blue, height: 150.0),
          ],
        ),
        makeHeader('Header Section 2'),
        SliverFixedExtentList(
          itemExtent: 150.0,
          delegate: SliverChildListDelegate(
            [
              Container(color: Colors.red),
              Container(color: Colors.purple),
              Container(color: Colors.green),
              Container(color: Colors.orange),
              Container(color: Colors.yellow),
            ],
          ),
        ),
        makeHeader('Header Section 3'),
        SliverGrid(
          gridDelegate: new SliverGridDelegateWithMaxCrossAxisExtent(
            maxCrossAxisExtent: 200.0,
            mainAxisSpacing: 10.0,
            crossAxisSpacing: 10.0,
            childAspectRatio: 4.0,
          ),
          delegate: new SliverChildBuilderDelegate(
            (BuildContext context, int index) {
              return new Container(
                alignment: Alignment.center,
                color: Colors.teal[100 * (index % 9)],
                child: new Text('grid item $index'),
              );
            },
            childCount: 20,
          ),
        ),
        makeHeader('Header Section 4'),
        // Yes, this could also be a SliverFixedExtentList. Writing
        // this way just for an example of SliverList construction.
        SliverList(
          delegate: SliverChildListDelegate(
            [
              Container(color: Colors.pink, height: 150.0),
              Container(color: Colors.cyan, height: 150.0),
              Container(color: Colors.indigo, height: 150.0),
              Container(color: Colors.blue, height: 150.0),
            ],
          ),
        ),
      ],
    );
  }
}

@maxlapides
Copy link
Author

maxlapides commented Jan 16, 2019

@zoechi zoechi added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Jan 17, 2019
@zoechi zoechi added this to the Goals milestone Jan 17, 2019
@maxlapides
Copy link
Author

Just thought I'd loop back here. I still believe this is an issue, and we continue to have to work around this. Has anyone had a chance to look into this?

@VladyslavBondarenko
Copy link

VladyslavBondarenko commented Apr 3, 2020

Reproduced with dev v1.16.4-pre.83

Edit:
Persists with current master 1.21.0-6.0.pre.41

flutter doctor -v
[✓] Flutter (Channel master, 1.21.0-6.0.pre.41, on Mac OS X 10.15.6 19G73, locale en-GB)
    • Flutter version 1.21.0-6.0.pre.41 at /Users/nevercode/dev/flutter
    • Framework revision 39fa00201d (3 hours ago), 2020-07-27 21:11:43 -0700
    • Engine revision f27729e97b
    • Dart version 2.10.0 (build 2.10.0-0.0.dev 24c7666def)
    • Pub download mirror https://pub.flutter-io.cn
    • Flutter download mirror https://storage.flutter-io.cn

 
[✓] Android toolchain - develop for Android devices (Android SDK version 29.0.3)
    • Android SDK at /Users/nevercode/Library/Android/sdk
    • Platform android-29, build-tools 29.0.3
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.6)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.6, Build version 11E708
    • CocoaPods version 1.9.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin version 47.1.2
    • Dart plugin version 193.7361
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)

[✓] VS Code (version 1.47.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.12.2

 
[✓] Connected device (5 available)            
    • Redmi Note 7 (mobile)       • 6345c469                                 • android-arm64  • Android 9 (API 28)
    • Nevercode’s iPhone (mobile) • b668e524315069f3db3661ac11ff1f66afafebdb • ios            • iOS 13.6
    • macOS (desktop)             • macos                                    • darwin-x64     • Mac OS X 10.15.6 19G73
    • Web Server (web)            • web-server                               • web-javascript • Flutter Tools
    • Chrome (web)                • chrome                                   • web-javascript • Google Chrome 84.0.4147.105

• No issues found!

@VladyslavBondarenko VladyslavBondarenko added the has reproducible steps The issue has been confirmed reproducible and is ready to work on label Apr 3, 2020
@NaikSoftware
Copy link

NaikSoftware commented May 15, 2020

Same issue with flutter 1.17.1
Solved this by using shrinkOffest, which > 0 when content overlaps in my case

@kf6gpe kf6gpe added the P2 Important issues not at the top of the work list label May 29, 2020
@VladyslavBondarenko VladyslavBondarenko added the found in release: 1.21 Found to occur in 1.21 label Jul 28, 2020
@Hixie Hixie removed this from the None. milestone Aug 17, 2020
@Oleh-Sv
Copy link
Contributor

Oleh-Sv commented Oct 7, 2021

overlapsContent works only when pinned == false. If pinned == true you should use shrinkOffset > maxExtent - minExtent.

In general case:
final bool isScrolledUnder = overlapsContent || (pinned && shrinkOffset > maxExtent - minExtent);

@Oleh-Sv
Copy link
Contributor

Oleh-Sv commented Oct 7, 2021

@Hixie I think you need to close this bug.

@maheshmnj
Copy link
Member

Reproduces as of stable 2.10.3

@maheshmnj maheshmnj added the found in release: 2.10 Found to occur in 2.10 label Mar 11, 2022
@delizondo
Copy link

This bug is still present in the latest stable Flutter version.
See the report here

Dartpad example here

@Piinks Piinks added P3 Issues that are less important to the Flutter project P2 Important issues not at the top of the work list and removed P2 Important issues not at the top of the work list P3 Issues that are less important to the Flutter project labels May 30, 2023
@Piinks Piinks self-assigned this May 30, 2023
@flutter-triage-bot flutter-triage-bot bot added team-framework Owned by Framework team triaged-framework Triaged by Framework team labels Jul 8, 2023
@flutter-triage-bot flutter-triage-bot bot added the Bot is counting down the days until it unassigns the issue label Oct 18, 2023
@flutter-triage-bot
Copy link

This issue is assigned to @Piinks but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!

@Piinks
Copy link
Contributor

Piinks commented Oct 18, 2023

Thanks bot! This is part of a much larger refactor that is actively being explored. :)

@flutter-triage-bot flutter-triage-bot bot removed the Bot is counting down the days until it unassigns the issue label Oct 18, 2023
@Piinks Piinks removed their assignment Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer: flex f: scrolling Viewports, list views, slivers, etc. found in release: 1.21 Found to occur in 1.21 found in release: 2.10 Found to occur in 2.10 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-framework Owned by Framework team triaged-framework Triaged by Framework team
Projects
None yet
Development

No branches or pull requests