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

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

Open
joseph-skyclover opened this issue Jun 6, 2023 · 9 comments
Assignees
Labels
Milestone

Comments

@joseph-skyclover
Copy link

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
Copy link
Owner

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
Copy link
Owner

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

@joseph-skyclover
Copy link
Author

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
Copy link
Owner

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

scroll-by-trackpad-420p.mov
scroll-by-mouse-420p.mov

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: flutter/flutter#91507 (comment) (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 fujidaiti self-assigned this Jun 7, 2023
@fujidaiti
Copy link
Owner

fujidaiti commented Jun 8, 2023

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
Copy link
Owner

fujidaiti commented Jun 8, 2023

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
Copy link
Owner

fujidaiti commented Jun 12, 2023

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);
    }
  }
2023-06-12.22.03.22.mov
2023-06-13.11.03.49.mov

@fujidaiti fujidaiti removed their assignment Jun 12, 2023
fujidaiti added a commit that referenced this issue Jun 12, 2023
This was referenced Aug 1, 2023
@fujidaiti
Copy link
Owner

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:

2023-08-06.23.09.16.mov

@fujidaiti fujidaiti self-assigned this Aug 6, 2023
@fujidaiti fujidaiti added this to the v1.0.0 milestone Aug 6, 2023
@jtkeyva
Copy link

jtkeyva commented Aug 9, 2023

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

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

When branches are created from issues, their pull requests are automatically linked.

3 participants