exprollable_page_view icon indicating copy to clipboard operation
exprollable_page_view copied to clipboard

showModalExprollable doesn't grow correctly when using a mouse to scroll

Open joseph-skyclover opened this issue 1 year ago • 9 comments

I noticed that when you are using the showModalExprollable function, the "grow on scroll" animation snaps to ViewportInset.expanded when on a device without touch or on web.

This has been confirmed to be reproducible in the examples.

joseph-skyclover avatar Jun 06 '23 19:06 joseph-skyclover

Thank you for your reporting! I cannot reproduce the problem. Could you let me know on which device are you reproducing? Videos and snapshots would also be helpful.

fujidaiti avatar Jun 07 '23 03:06 fujidaiti

Oh, sorry, I missed the title. Do you mean the problem that occurs on the desktop or on a desktop browser?

fujidaiti avatar Jun 07 '23 03:06 fujidaiti

Take a look at these videos

The macOS one demonstrates how it snaps to the top. The chrome one also demonstrates that it works in touch mode (at the end of the video)

To answer your question, it happens on both. When there is a touch screen in use it works correctly (or when the touch screen is emulated by chrome).

joseph-skyclover avatar Jun 07 '23 14:06 joseph-skyclover

I have watched the video and confirmed that I can reproduce the problem. The cause of the problem is that when scrolling with the mouse wheel, ScrollPosition.setPixels is not called. There is a subclass of ScrollPosition called AbsorbScrollPosition that plays an important role in this package and it overrides the setPixels method to do some things related to the grow on scroll animation. A ScrollPosition is attached to a ListView and then setPixels is called whenever the scroll position changes. But when scrolling with the mouse wheel, I don't know why, setPixels is never called at all.

I created a minimum project to reproduce this problem:

The code
import 'package:flutter/material.dart';

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

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

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

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

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  late final ScrollController controller;

  @override
  void initState() {
    super.initState();
    controller = MyScrollController()
      ..addListener(() {
        debugPrint("[ScrollController] offset = ${controller.offset}");
      });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: ListView.builder(
        controller: controller,
        itemBuilder: (_, index) => ListTile(
          title: Text("Item#$index"),
        ),
      ),
    );
  }
}

class MyScrollPosition extends ScrollPositionWithSingleContext {
  MyScrollPosition({
    required super.physics,
    required super.context,
    super.oldPosition,
    super.initialPixels,
    super.keepScrollOffset,
  });

  @override
  double setPixels(double newPixels) {
    debugPrint("[ScrollPosition] setPixels($newPixels)");
    return super.setPixels(newPixels);
  }
}

class MyScrollController extends ScrollController {
  @override
  ScrollPosition createScrollPosition(ScrollPhysics physics,
      ScrollContext context, ScrollPosition? oldPosition) {
    return MyScrollPosition(
      physics: physics,
      context: context,
      initialPixels: initialScrollOffset,
      oldPosition: oldPosition,
      keepScrollOffset: keepScrollOffset,
    );
  }
}

And the results (Ran on macbook air) :

https://github.com/fujidaiti/exprollable_page_view/assets/68946713/7f9d67d7-0289-45e5-883a-6bdfcad19bc5

https://github.com/fujidaiti/exprollable_page_view/assets/68946713/59fc092c-c290-4296-8a57-87fb56df508c

In the first video, I am scrolling the ListView with the trackpad, and you can see that setPixels is called each time the scroll position changes (see console log on the left). The bouncing physics also works as well as on touch devices like the iPhones. On the other hand, in the second video, I'm scrolling the ListView with the mouse scroll wheel, but you can see that sePixels is not called at all. Also, the bouncing physics by the scroll wheel is not working. I googled this problem and found a related issue: https://github.com/flutter/flutter/issues/91507#issuecomment-940061563 (which says that the bouncing physics not working with the scroll wheel is the intended behavior).

If I can find a reason why setPixels is not called, or another way to detect changes in scroll position in the ScrollPosition class, this bug might be fixed. I will try to find that, but it will take some time to fix.

fujidaiti avatar Jun 07 '23 19:06 fujidaiti

It seems that when scrolling the list view with a mouse, setPixels is not called, instead pointerScroll and eventually forcePixels is called to change the current scroll position.

fujidaiti avatar Jun 08 '23 02:06 fujidaiti

I noticed that when you are using the showModalExprollable function, the "grow on scroll" animation snaps to ViewportInset.expanded

This is because the pointerScroll method tells the scroll physics to start a ballistic animation that snaps the viewport inset to specified snap positions (maybe, not confirmed).

fujidaiti avatar Jun 08 '23 02:06 fujidaiti

Adding the following code to the AbsorbScrollPosition class partially solved this issue. But there is still another problem related to the snapping behavior that the page view rattles as it tries to snap to a nearby snapping position each time the mouse wheel is slightly rotated (see the video below). The PageView has a similar problem discussed in here and it is not clear how exactly this should behave in such a situation. One possible solution, I think, is to disable the snapping effects on the desktop platforms and web, but it would be far from a smooth, comfortable UI (see the second video, the result of commenting out the line of goBallistic(0.0); in pointerScroll).

  @override
  // The code was mostly borrowed from [super.pointerScroll]:
  void pointerScroll(double delta) {
    if (delta == 0.0) {
      goBallistic(0.0);
      return;
    }

    final double targetPixels =
        (impliedPixels + delta).clamp(impliedMinScrollExtent, maxScrollExtent);
    if (targetPixels != impliedPixels) {
      goIdle();
      updateUserScrollDirection(
        -delta > 0.0 ? ScrollDirection.forward : ScrollDirection.reverse,
      );
      final double oldPixels = pixels;
      isScrollingNotifier.value = true;
      forcePixels(targetPixels);
      didStartScroll();
      didUpdateScrollPositionBy(pixels - oldPixels);
      didEndScroll();
      goBallistic(0.0);
    }
  }

https://github.com/fujidaiti/exprollable_page_view/assets/68946713/ecbd51ae-4760-45bf-8ace-cd4d762d00cb

https://github.com/fujidaiti/exprollable_page_view/assets/68946713/e89e880a-d6d6-482e-b5dd-9530d06148ce

fujidaiti avatar Jun 12 '23 13:06 fujidaiti

I decided to implement the landscape mode that is mentioned in #47. This will disable the expanding/shrinking page animation in landscape mode, which won't fix the problem, but it will workaround it. In addition, this behavior is also implemented in the original UI of Apple Books app which this package is trying to emulate:

https://github.com/fujidaiti/exprollable_page_view/assets/68946713/f094f6f1-9dd0-4c32-a406-e2367b03a324

fujidaiti avatar Aug 06 '23 14:08 fujidaiti

i think this is a great decision to support more than portrait view on mobile. let me know if you want any testing/review

jtkeyva avatar Aug 09 '23 06:08 jtkeyva