Kavantix / sliver_tools

A set of useful sliver tools that are missing from the flutter framework
MIT License
652 stars 64 forks source link

MultiSliver with several pinned headers in NestScrollView headerSliverBuilder need overscrolling to overlap #56

Closed manu-sncf closed 2 years ago

manu-sncf commented 2 years ago

When I put MultiSliver in NestedScrollView.headerSliverBuilder with several pinned headers inside, I need to "overscroll" to begin overlapping the body. The problem doesn't occur with one pinned header.

In the demo, look at the cursor and the shadow beneath the TabBar.

demo

The code :

import 'package:flutter/material.dart';
import 'package:sliver_tools/sliver_tools.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  static const String _title = 'Flutter Code Sample';

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      title: _title,
      home: MyStatelessWidget(),
    );
  }
}

class MyStatelessWidget extends StatelessWidget {
  const MyStatelessWidget({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final List<String> tabs = <String>['Tab 1', 'Tab 2'];
    return DefaultTabController(
      length: tabs.length, // This is the number of tabs.
      child: Scaffold(
        body: NestedScrollView(
          headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) {
            return <Widget>[
              SliverOverlapAbsorber(
                handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
                sliver: MediaQuery.removePadding(
                  context: context,
                  removeBottom: true,
                  child: MultiSliver(
                    children: [
                      const SliverPinnedHeader(
                          child: SafeArea(child: Padding(padding: EdgeInsets.all(16), child: Text('Test')))),
                      SliverAppBar(
                        title: const Text('Books'), // This is the title in the app bar.
                        pinned: true,
                        expandedHeight: 150.0,
                        forceElevated: innerBoxIsScrolled,
                        bottom: TabBar(
                          tabs: tabs.map((String name) => Tab(text: name)).toList(),
                        ),
                      ),
                    ],
                  ),
                ),
              ),
            ];
          },
          body: TabBarView(
            // These are the contents of the tab views, below the tabs.
            children: tabs.map((String name) {
              return SafeArea(
                top: false,
                bottom: false,
                child: Builder(
                  builder: (BuildContext context) {
                    return CustomScrollView(
                      key: PageStorageKey<String>(name),
                      slivers: <Widget>[
                        SliverOverlapInjector(
                          // This is the flip side of the SliverOverlapAbsorber
                          // above.
                          handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
                        ),
                        SliverPadding(
                          padding: const EdgeInsets.all(8.0),
                          sliver: SliverFixedExtentList(
                            itemExtent: 48.0,
                            delegate: SliverChildBuilderDelegate(
                              (BuildContext context, int index) {
                                return ListTile(
                                  title: Text('Item $index'),
                                );
                              },
                              childCount: 30,
                            ),
                          ),
                        ),
                      ],
                    );
                  },
                ),
              );
            }).toList(),
          ),
        ),
      ),
    );
  }
}

The problem is also on iOS and Web.

Flutter doctor :

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.0.4, on macOS 12.5 21G72 darwin-arm, locale fr-FR)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
[✓] Xcode - develop for iOS and macOS (Xcode 13.2.1)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2021.2)
[✓] IntelliJ IDEA Ultimate Edition (version 2022.1.3)
[✓] VS Code (version 1.70.0)
[✓] Connected device (3 available)
[✓] HTTP Host Availability

Thanks for your help.

manu-sncf commented 2 years ago

The bug seems to be fixed with this patch :

index 02a3394..2aa7405 100644
--- a/lib/src/rendering/multi_sliver.dart
+++ b/lib/src/rendering/multi_sliver.dart
@@ -213,7 +213,7 @@ class RenderMultiSliver extends RenderSliver
       layoutOffset += childParentData.geometry.layoutExtent;
       hasVisualOverflow =
           hasVisualOverflow || childParentData.geometry.hasVisualOverflow;
-      maxScrollObstructionExtent =
+      maxScrollObstructionExtent +=
           childParentData.geometry.maxScrollObstructionExtent;
       visible = visible || childParentData.geometry.visible;
       if (childParentData.geometry.cacheExtent != 0.0) {
Kavantix commented 2 years ago

Sorry for the late response but good catch! I believe that solution is indeed correct but will have to do a little more testing in the weekend.

Kavantix commented 2 years ago

From my testing the change does seem to improve some scenarios, however your test scenario provided in the original comment still behaves weirdly. However, that is probably just because of the NestedScrollView.

Kavantix commented 2 years ago

Ok the weird behaviour that is left is because SliverPersistentHeader doesnt handle overlap correctly (see flutter/flutter#91256)

manu-sncf commented 2 years ago

You are right, in fact, the fix work if SliverAppBar is first. I inverted the SliverAppBar with SliverPinnedHeader, my mistake

Here is the code that reproduce the problem and the fix of the PR which works fine without weird behavior.

import 'package:flutter/material.dart';
import 'package:sliver_tools/sliver_tools.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  static const String _title = 'Flutter Code Sample';

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      title: _title,
      home: MyStatelessWidget(),
    );
  }
}

class MyStatelessWidget extends StatelessWidget {
  const MyStatelessWidget({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final List<String> tabs = <String>['Tab 1', 'Tab 2'];
    return DefaultTabController(
      length: tabs.length, // This is the number of tabs.
      child: Scaffold(
        body: NestedScrollView(
          headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) {
            return <Widget>[
              SliverOverlapAbsorber(
                handle:
                    NestedScrollView.sliverOverlapAbsorberHandleFor(context),
                sliver: MediaQuery.removePadding(
                  context: context,
                  removeBottom: true,
                  child: MultiSliver(
                    children: [
                      SliverAppBar(
                        title: const Text('Books'),
                        pinned: true,
                        expandedHeight: 150.0,
                        forceElevated: innerBoxIsScrolled,
                        bottom: TabBar(
                          tabs: tabs
                              .map((String name) => Tab(text: name))
                              .toList(),
                        ),
                      ),
                      const SliverPinnedHeader(
                          child: SafeArea(
                              child: Padding(
                                  padding: EdgeInsets.all(16),
                                  child: Text('Test')))),
                    ],
                  ),
                ),
              ),
            ];
          },
          body: TabBarView(
            // These are the contents of the tab views, below the tabs.
            children: tabs.map((String name) {
              return SafeArea(
                top: false,
                bottom: false,
                child: Builder(
                  builder: (BuildContext context) {
                    return CustomScrollView(
                      key: PageStorageKey<String>(name),
                      slivers: <Widget>[
                        SliverOverlapInjector(
                          // This is the flip side of the SliverOverlapAbsorber
                          // above.
                          handle:
                              NestedScrollView.sliverOverlapAbsorberHandleFor(
                                  context),
                        ),
                        SliverPadding(
                          padding: const EdgeInsets.all(8.0),
                          sliver: SliverFixedExtentList(
                            itemExtent: 48.0,
                            delegate: SliverChildBuilderDelegate(
                              (BuildContext context, int index) {
                                return ListTile(
                                  title: Text('Item $index'),
                                );
                              },
                              childCount: 30,
                            ),
                          ),
                        ),
                      ],
                    );
                  },
                ),
              );
            }).toList(),
          ),
        ),
      ),
    );
  }
}

The bug https://github.com/flutter/flutter/issues/91256 as you mentioned is confirmed if SliverAppBar is not first in the list.