championswimmer / yt_dlp_gui

38 stars 10 forks source link

[GUI] lifting the state up and avoiding `setState` #7

Open thisisamank opened 1 year ago

thisisamank commented 1 year ago

I was trying to disable download button if no links are given but the state seems to be very tightly coupled with UI and the screen gets refreshed only when some checkbox is clicked so I couldn't do it. IMO we should use something like ValueNotifier and put all the business logic there if we don't want to use any state management packages. I have a basic draft ready with "separated concerns", LMK if this would be a valuable addition at this moment.

championswimmer commented 1 year ago

Would probably prefer having a proper state management solution only, instead of just value notifiers.

thisisamank commented 1 year ago

We can use Riverpod as you said in your stream too. These days ValueNotifiers + sealed classes can do a lot of things that riverpod can do. That's why I used it there. Let me create a new PR with riverpod.

thisisamank commented 1 year ago

In my apps I generally use get_it + riverpod, plus I have seen this pattern in a lot of apps. get_it for dependency injection and riverpod for state management. People do use riverpod for both, personally, I am not a fan of it as your dependency will require the reference of ref to be accessible which is only available at UI layer. So in case some of our repository class needs access to other repository class it would be difficult.