joanpablo / reactive_forms

This is a model-driven approach to handling form inputs and validations, heavily inspired in Angular's Reactive Forms
MIT License
469 stars 86 forks source link

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

Open vasilich6107 opened 2 years ago

vasilich6107 commented 2 years ago

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

ChiPhanTlm commented 2 years ago

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

vasilich6107 commented 2 years ago

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

joanpablo commented 2 years ago

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

vasilich6107 commented 2 years ago

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

In my implementation it works image

vasilich6107 commented 2 years ago

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

joanpablo commented 2 years ago

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 commented 2 years ago

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.

vasilich6107 commented 2 years ago

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 commented 2 years ago

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.

joanpablo commented 2 years ago

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 commented 2 years ago

What do you think?

vasilich6107 commented 2 years ago

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

joanpablo commented 2 years ago

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.

codakkk commented 6 months ago
  • ReactiveStatusListenableBuilder

Any update?