fujidaiti / exprollable_page_view

Yet another PageView widget that expands its viewport as it scrolls. Exprollable is a coined word combining the words expandable and scrollable.
https://pub.dev/packages/exprollable_page_view
MIT License
71 stars 5 forks source link

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

Open joseph-skyclover opened 1 year ago

joseph-skyclover commented 1 year ago

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.

fujidaiti commented 1 year ago

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 commented 1 year ago

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

joseph-skyclover commented 1 year ago

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).

fujidaiti commented 1 year ago

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 ```dart 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 createState() => _MyHomePageState(); } class _MyHomePageState extends State { 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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

jtkeyva commented 1 year ago

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