reactive_forms icon indicating copy to clipboard operation
reactive_forms copied to clipboard

What do you think about creating ReactiveFormControlListener, ReactiveFormControlBuilder and ReactiveFormControlConsumer?

Open vasilich6107 opened this issue 2 years ago • 14 comments

Hi. Managing of field subscriptions in a right way is pretty complicated. Here is an example of managing of subscription in bloc lib https://github.com/felangel/bloc/blob/master/packages/flutter_bloc/lib/src/bloc_listener.dart

What do you think about creating a set of widgets in the same concept as bloc lib

  • ReactiveFormControlListener
  • ReactiveFormControlBuilder
  • ReactiveFormControlConsumer

vasilich6107 avatar Jun 23 '22 06:06 vasilich6107

It would be nice to have one of these instead of listen to the stream manually. It can work like BlocListener

ChiPhanTlm avatar Jun 29 '22 03:06 ChiPhanTlm

@ChiPhanTlm @evanholt1 @krista30 I released https://pub.dev/packages/reactive_forms_lbc Cause sometimes it takes eternity to get response and next eternity take the PR to be merged. Only listener is currently available cause I'm interested in listener and do not need other for now) May be add them later.

vasilich6107 avatar Jul 07 '22 16:07 vasilich6107

Hi @vasilich6107

Do you have an example of how this new implementation is better than the current implementation? I would like to have a use case implemented with both variants, so I can really understand where is the different.

Thanks in advance

joanpablo avatar Jul 07 '22 17:07 joanpablo

Hi @joanpablo Here is an example https://github.com/artflutter/reactive_forms_widgets/blob/master/packages/reactive_forms_lbc/example/lib/main.dart#L104

Despite the difference is not huge my copy/paste implementation has some benefits

  • familiar approach popularised by bloc library, almost everyone is familiar with Listener/Builder/Consumer paradigm
  • you do not have to bother with child in listener
  • typing (ReactiveStatusListenableBuilder is missing one despite it purposed for status we pass control into builder without ability to add types)
  • listenWhen callback with access to prev and curr values
  • i can't do this with regular implementation image

In my implementation it works image

vasilich6107 avatar Jul 08 '22 05:07 vasilich6107

  • added ReactiveFormControlFocusListener
  • added ReactiveFormControlStatusListener
  • added ReactiveFormControlTouchListener

Would be great to hear feedback from you @ChiPhanTlm @evanholt1 @krista30 @joanpablo

vasilich6107 avatar Jul 08 '22 15:07 vasilich6107

Hi @vasilich6107,

It is normal that the example you presented above fails, because you are blending the declarative programming of Flutter (when creating widgets) with the imperative nature of the Flash package. You are executing the showFlash method inside a build method, this will always fail, even if you are not using ReactiveForms. To make it works you will need to use a SchedulerBinding.instance.addPostFrameCallback or something similar. Or simply subscribe to value changes in the initState of your widget.

You only show an example with the current implementation of Reactive Form, could you please show me an implementation with the proposed implementation using the ReactiveFormControlStatusListener?

I still need to understand the issue.

joanpablo avatar Jul 11 '22 10:07 joanpablo

Regarding the feature about getting the actual and current values of a Control int this thread we discuss how to implement this behavior using Reactive Forms and rxdart. Of course, if it is the desire of the community we could improve our Listenable builders to contain previous values as well.

joanpablo avatar Jul 11 '22 10:07 joanpablo

It is normal that the example you presented above fails

yes, I know

SchedulerBinding.instance.addPostFrameCallback or something similar

I know this lifehack. But I do not want to use it as far as there is more clean way borrowed from bloc library

Or simply subscribe to value changes in the initState of your widget

We have slightly different approaches. I do not like to code things manually) This is boring to crate state full widget, add init state, subscribe and then unsubscribe manually) I prefer to do not repeat myself. This is the reason I’ve created those widgets. I’m just dropping the widget and boom - everything works out of the box.

we discuss how to implement this behavior using Reactive Forms and rxdart

thanks, will take a look

I still need to understand the issue.

there is no big issue in current implementation. I think that my approach is something like syntactic sugar or shortcut to make my(and others) life easier

vasilich6107 avatar Jul 12 '22 04:07 vasilich6107

could you please show me an implementation with the proposed implementation using the ReactiveFormControlStatusListener?

I’m not sure that I clearly understand what you mean.

vasilich6107 avatar Jul 12 '22 05:07 vasilich6107

Hi @vasilich6107

I have seen that you have recently modified the implementation of your ReactiveFormControlValueListener, you have included a ReactiveFormControlValueBuilder. I understand that it is a prototype and still a work in progress.

But right now I see two widgets with a lot of duplicated code (both widgets do a lot of common stuff) the only difference is that one uses some kind of callback to notify when the value change and the other one use the same callback to trigger the rebuild by refreshing the state of the stateful widget. Also you are including dependencies on third-party libraries that in my opinion could bring maintain complexity and that it really doesn't bring many advantages.

As far as I understand (I'm still trying to understand the issue) if you want to solve the error in the previous example (the one that uses the imperative showFlash() method), we can just add a callback to the ReactiveValueListenableBuilder (+- the same idea you are using) and if you want to access prev values we can also improve the ReactiveValueListenableBuilder so that we can configure it to hold the previous value if the developer wants to do it.

Something similar to (this is just like pseudo code):

ReactiveValueListenableBuilder(
   formControlName: '....',
   builder: (context, control, child) => ...,
   onValueChanged: (control, previousValue) {  ... showFlash... }
)

joanpablo avatar Jul 12 '22 13:07 joanpablo

What do you think?

joanpablo avatar Jul 12 '22 13:07 joanpablo

Also you are including dependencies on third-party libraries

It is not a big issue if dependency does it's job. I'm not forcing anyone to include my implementation)

Something similar to (this is just like pseudo code) What do you think?

It's up to you. I wanted this type of widgets for a long time but i did not have spare time to implement them(fooly me, it was so easy). Now I have this type of widgets) Will finish with Builders and Consumers and that's it.

If you want to implement something similar out of the box it could help developers. For me one of the advantages of my implementation is that it is familiar to me and teammates by the same helper widget split as utilized in bloc. So it makes flow across my project identical.

There is also listenWhen and buildWhen methods. So it's like native experience to me) Other implementations would disturb me with lack of similarity

vasilich6107 avatar Jul 12 '22 16:07 vasilich6107

Thanks for your comments @vasilich6107 they are very important to me. I got your point. Anyway I will implement the

ReactiveValueListenableBuilder(
   formControlName: '....',
   builder: (context, control, child) => ...,
   onValueChanged: (control, previousValue) {  ... showFlash... }
) 

but first I will test if it really solves the issue of the imperative and declarative stuff (I'm not sure yet, but based on your comments it seems to work). That way if you or any other developer want to use it they can do it.

joanpablo avatar Jul 12 '22 16:07 joanpablo

  • ReactiveStatusListenableBuilder

Any update?

codakkk avatar Apr 16 '24 19:04 codakkk