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

[Sheet] SheetController.(relative) animateTo is broken #412

Open
vishna opened this issue Jun 28, 2024 · 2 comments
Open

[Sheet] SheetController.(relative) animateTo is broken #412

vishna opened this issue Jun 28, 2024 · 2 comments

Comments

@vishna
Copy link

vishna commented Jun 28, 2024

What's wrong

Calling SheetController.relativeAnimateTo does nothing in conditions where it should expand

Steps to repro

Here's the "minimal" code I managed to put together to illustrate the issue

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

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

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      title: 'Flutter Demo',
      home: HomeAndSheet(),
    );
  }
}

class HomeAndSheet extends StatefulWidget {
  const HomeAndSheet({super.key});

  @override
  State<HomeAndSheet> createState() => _HomeAndSheetState();
}

class _HomeAndSheetState extends State<HomeAndSheet> {
  Widget emptyPage(BuildContext context) => const Placeholder();
  Widget pageWithList(BuildContext context) => const PageWithList();
  List<WidgetBuilder> pages() => [emptyPage, pageWithList]; // BAD CASE
  // List<WidgetBuilder> pages() => [pageWithList, emptyPage]; // GOOD CASE

  late WidgetBuilder pageBuilder;
  final sheetController = SheetController();

  @override
  void initState() {
    super.initState();
    setPage(0);
  }

  void setPage(int pageNumber) {
    setState(() {
      pageBuilder = pages()[pageNumber];
    });
  }

  @override
  Widget build(BuildContext context) {
    final sheetHeight = MediaQuery.sizeOf(context).height - 100;

    return Stack(
      children: [
        HomePage(
          title: 'Flutter Demo Home Page',
          onTap: () async {
            debugPrint("SHOW");
            setPage(1);
            await sheetController.relativeAnimateTo(
              1,
              duration: const Duration(milliseconds: 400),
              curve: Curves.easeOutQuart,
            );
            setPage(0);
          },
        ),
        Sheet(
          controller: sheetController,
          fit: SheetFit.loose,
          initialExtent: 0,
          child: SizedBox(
            height: sheetHeight,
            child: Builder(
              builder: pageBuilder,
            ),
          ),
        )
      ],
    );
  }
}

class HomePage extends StatelessWidget {
  const HomePage({super.key, required this.title, required this.onTap});

  final String title;
  final VoidCallback onTap;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(title),
      ),
      body: const Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            Text(
              'Press the button to reveal the sheet',
            ),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: onTap,
        child: const Icon(Icons.border_bottom),
      ),
    );
  }
}

class PageWithList extends StatelessWidget {
  const PageWithList({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      backgroundColor: Colors.amber,
      appBar: AppBar(
        backgroundColor: Colors.amberAccent,
        title: const Text("Oh Sheet!"),
      ),
      body: ListView(
        children: ListTile.divideTiles(
          context: context,
          tiles: List<Widget>.generate(
              100,
              (int index) => ListTile(
                    title: Text('Item $index'),
                  )),
        ).toList(),
      ),
    );
  }
}

Launch simulator, press FAB button and watch the sheet not appear. If you switch in code from // BAD to // GOOD, hot restart, tap the button again the sheet will appear 😐

What's happening

BAD CASE

SHOW
relativeAnimateTo(1.0, 0:00:00.400000, Cubic(0.17, 0.84, 0.44, 1.00))
pixelsFromRelativeOffset (offset = 1.0, minScrollExtent = 0.0, maxScrollExtent = 832.0)
beginActivity(DrivenScrollActivity#cc5e8(AnimationController#f7883(▶ 0.000; for DrivenScrollActivity)))
setPixels(0.0)
updateRenderObject <--- We put PageWithList widget inside the sheet
performLayout START
beginActivity(IdleScrollActivity#84f3a)
applyContentDimensions(0.0, 832.0)
performLayout END
updateRenderObject <--- We put Placeholder widget inside the sheet
performLayout START
applyContentDimensions(0.0, 832.0)
performLayout END

GOOD

SHOW
relativeAnimateTo(1.0, 0:00:00.400000, Cubic(0.17, 0.84, 0.44, 1.00))
pixelsFromRelativeOffset (offset = 1.0, minScrollExtent = 0.0, maxScrollExtent = 832.0)
beginActivity(DrivenScrollActivity#bc7fc(AnimationController#596a8(▶ 0.000; for DrivenScrollActivity)))
setPixels(0.0)
updateRenderObject <--- We put Placeholder widget inside the sheet
performLayout START
applyContentDimensions(0.0, 832.0)
performLayout END
setPixels(381.951171875)
setPixels(460.16555404663086)
setPixels(527.0387233543396)
setPixels(582.0008573913574)
setPixels(625.2629356384277)
setPixels(694.4747106933594)
setPixels(742.2553861427307)
setPixels(761.226918964386)
setPixels(776.0444121360779)
setPixels(788.8407738685609)
setPixels(799.1720542907715)
setPixels(807.4257688903808)
setPixels(814.3350498962402)
setPixels(819.6619298744201)
setPixels(823.9420788574218)
setPixels(827.145937626362)
setPixels(829.4243977189063)
setPixels(830.9135014343261)
setPixels(831.7354572105407)
setPixels(832.0)
beginActivity(IdleScrollActivity#2b86b)
updateRenderObject <--- We put PageWithList widget inside the sheet
performLayout START
beginActivity(IdleScrollActivity#b08ce)
applyContentDimensions(0.0, 832.0)
performLayout END

What I think is happening

Whenever animateTo is called and a new widget is put at the same time, a performLayout inside RenderSheetViewport will have to be called. In GOOD case this manages to happen before beginActivity(IdleScrollActivity#2b86b) so the sheet "knows" how to expand. In BAD CASE out widget seems to be more complex and the dimensions are not delivered on time and the animation doesn't execute properly.

I managed to workaround this by putting a delay of 150ms before animateTo. This is obviously a non ideal HACK. Other possible hack is to use "Placeholder" widget and only replace the widget once the animation is ongoing. Related -> #360

Investigating a proper fix, if anyone has any pointers - let me know.

@vishna vishna changed the title [sheet] SheetController.(relative) animateTo is broken [Sheet] SheetController.(relative) animateTo is broken Jun 28, 2024
vishna pushed a commit to vishna/modal_bottom_sheet that referenced this issue Jun 28, 2024
Expanding [Sheet] using [SheetController] might not always work, e.g. you are trying to animate the sheet and expand controller at the same time (see: jamesblasco#412) You can use [addPostLayoutCallback] in order to defer your call to the moment [RenderSheetViewport] has finished [RenderSheetViewport.performLayout] Additionally you can provide [timeout] after which this callback should be called anyway incase [RenderSheetViewport.performLayout] doesn't start. Usually (this has not been super benchmarked!) layout starts under 50ms, putting 100ms to be on the safe side - YMMV
@vishna
Copy link
Author

vishna commented Jun 28, 2024

Made a PR that allows firing exactly when the layout is done using Sheet.addPostLayoutCallback:

setPage(1);
Sheet.addPostLayoutCallback(() async {
  await sheetController.relativeAnimateTo(
    1,
    duration: const Duration(milliseconds: 400),
    curve: Curves.easeOutQuart,
  );
  setPage(0);
});

@vishna
Copy link
Author

vishna commented Jun 28, 2024

⚠️

This can still fail if the widget in your sheet causes RenderSheetViewport trigger performLayout more than once. We had this issue in our main app but solved this by optimizing how we build our widget tree.

The best fix would be for the animation to always complete, treat my PR as a workaround that might not work for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant