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

Quesiton: Option for pre-build call back #25

Closed
chrisabird opened this issue Mar 19, 2018 · 2 comments
Closed

Quesiton: Option for pre-build call back #25

chrisabird opened this issue Mar 19, 2018 · 2 comments

Comments

@chrisabird
Copy link

chrisabird commented Mar 19, 2018

I've been toying with the idea of introducing an additional callback, something like.

typedef OnWillChangeCallback<ViewModel> = bool Function(ViewModel viewModel);

The reason i found i wanted this was because there are times where it's desirable to trigger some form of navigation action as result of state change. You can't do that during the build action (as far as i understand), but you can do it in the IgnoreChangeCallback pre-build. This works, but feels odd for two reasons. One it happens before the distinct filter which sometimes is useful, and two it's passed the State and not the ViewModel so i can end up duplicating some conversion i've done for the viewModel.

Interested in hearing thoughts and opinions on approaches to dealing with this. I've made the above suggested change locally and using it, so i can submit a pull request if deemed a good idea.

@brianegan
Copy link
Owner

brianegan commented Mar 19, 2018

Hey hey :) Thanks for writing in!

Yah, I think it that type of function might be necessary. I was hoping to avoid it, but there are a few imperative APIs in Flutter that are awkward to work with (Navigation, TabController, TextEditingController, etc). The usual way of handling it is to "Send that action to your Middleware and handle it there," but Navigation, etc are all View concerns, and stuffing it into the Middleware feels wrong.

I'd be happy to accept a PR! One thing: Please make sure to add tests for any change :)

Thanks again.

@brianegan
Copy link
Owner

Thanks so much for the contribution. This is now published as 0.3.6. I'll also include it in the upcoming 0.4.0 release that works for Dart 2.0

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

No branches or pull requests

2 participants