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

Please use Dart streams for Animation and other listeners #15938

Closed
escamoteur opened this Issue Mar 26, 2018 · 14 comments

Comments

Projects
None yet
4 participants
@escamoteur
Copy link
Contributor

escamoteur commented Mar 26, 2018

Hi,
while working on an Animation want to remove my Listeners at the end of the Animation. Unfortunately you have to pass the Listener function to remove it, which makes it impossible to use with Lambda function.

See like in this example:

    _pageMoveToSubscription = theViewModel.scrollToEvents
         .listen((index) {
           animation.addStatusListener((status) {
             if (status == AnimationStatus.completed) {
               _pageController.animateToPage(index,
                                            curve: Curves.linear,
                                            duration: new Duration(milliseconds: 1));
               controller.reverse();
               animation.removeStatusListener(????) <<<<< see here
             }                                                    
             controller.forward();
           });
         });

It would be way easier if Animations would offer a Stream which you can listen on and which subscriptions you can easily cancel. Additionally you would get all the power of Stream manipulations.

@escamoteur escamoteur changed the title Please use Dart streams for Animation listeners Please use Dart streams for Animation and other listeners Mar 27, 2018

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented May 29, 2018

How about:

    _pageMoveToSubscription = theViewModel.scrollToEvents
         .listen((index) {
           void handler(status) {
             if (status == AnimationStatus.completed) {
               _pageController.animateToPage(index,
                                            curve: Curves.linear,
                                            duration: new Duration(milliseconds: 1));
               controller.reverse();
               animation.removeStatusListener(handler);
             }                                                    
             controller.forward();
           }
           animation.addStatusListener(handler);
         });

I haven't tested it but I'm pretty sure that works.

@escamoteur

This comment has been minimized.

Copy link
Contributor Author

escamoteur commented May 30, 2018

Still, where do you remove the StatusListener. Besides that if you would use Streams listeners we could use all Streams or Rx functions on the Data. Thinking on the proposed BLOCK pattern Streams would also make more sense than methods.
Dart has this beautifully feature, so why not using it?

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Jun 5, 2018

Still, where do you remove the StatusListener.

I'm not sure what you mean. In the snippet I pasted, there's a call to animation.removeStatusListener(handler);, which removes the listener.

At a broader level, though, we mostly don't use Streams in Flutter's framework itself for a few reasons: Streams are quite heavy-weight (the API is very broad, compared to Listenable), the API is hard to extend (it's non-trivial to make your own streams and stream convertors and so on), and Streams are inherently asynchronous, which makes their use for sending data along a little inconvenient in the framework (unlike, say, ValueListenable which has a synchronous value property).

However, you can use StreamBuilder to use Streams with Flutter widgets, and there's no reason I can think of why you couldn't adapt from Stream to Listenable or ValueListenable and then use them throughout the framework.

@no-response

This comment has been minimized.

Copy link

no-response bot commented Aug 19, 2018

Without additional information, we are unfortunately not sure how to resolve this issue. We are therefore reluctantly going to close this bug for now. Please don't hesitate to comment on the bug if you have any more information for us; we will reopen it right away!
Thanks for your contribution.
cc @Hixie

@tudor07

This comment has been minimized.

Copy link

tudor07 commented Oct 23, 2018

Please reconsider this, it is really painful to work with the current animations listeners api.

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Oct 24, 2018

Maybe we could create a ValueListenable<T>-to-Stream<T> adapter... If this is something that interests you, I recommend either writing a package that implements that, or filing a bug suggesting it.

@escamoteur

This comment has been minimized.

Copy link
Contributor Author

escamoteur commented Oct 24, 2018

Still it is pretty sad that a reactive framework doesn't go all the way down and uses Streams inside,

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Oct 25, 2018

We can't use Streams internally for the reasons I described above.

Would the adapters I suggest above satisfy your use cases?

@tudor07

This comment has been minimized.

Copy link

tudor07 commented Oct 26, 2018

Honestly for me it would be great if we would have at least a clear() method on the Animation so we can remove all listeners at once.

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Oct 27, 2018

@tudor07 That seems like a layering violation -- you can't necessarily know what objects are listening to the object, so removing the listeners may mean removing listeners that another object owns.

Can you elaborate on what you are doing that you end up wanting this feature?

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Oct 28, 2018

I'd be curious to hear about your experiences with the valueListenableToStreamAdapter() method from master...Hixie:listenable_stream

@tudor07

This comment has been minimized.

Copy link

tudor07 commented Oct 30, 2018

@Hixie
What I want to do, is everytime I run an animation, it may start/end with different values, so I am creating a new Tween everytime, and these Tweens have the same AnimationController as parent, so I guess the old listeners remain present. So my solution for this is:

  1. Declare a Function on the StatefulWidget which is the listener
  2. Everytime I want to run the animation I have to do:
// field in StatefulWidget class
Function _listener;

...

// in some method where I run the animation
    final tween = Tween<double>(begin: _someValue, end: _someOtherValue)
        .animate(CurvedAnimation(
      parent: _parentAnimController,
      curve: Curves.linear,
    ));

    tween.removeListener(_listener);
    _listener = () {
      // logic from my listener that changes the values so it animates
     setState(() => _val = tween.value);
    };
    tween.addListener(_listener);

It would be cool if there was a solution to not have the top-level Function listener, so I can run my listener only once and the second time I run the animation I don't have to remove and re-add.

But again, maybe I am doing something wrong in the first place, I would be happy to hear about a better solution.

Maybe for my case it would be nice to have a addListenerOnce method that removes the listener after it finished.

@Hixie

This comment has been minimized.

Copy link
Contributor

Hixie commented Dec 15, 2018

@tudor07 Not sure what you mean by "the old listeners remain present". That shouldn't be the case.
But in any case that seems like a question for a different issue. This issue is about streams.

@escamoteur any opinions regarding the valueListenableToStreamAdapter() method from master...Hixie:listenable_stream?

@escamoteur

This comment has been minimized.

Copy link
Contributor Author

escamoteur commented Feb 4, 2019

@Hixie I think that is a good solution!!! Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.