torbenkeller / expandable_bottom_sheet

MIT License
33 stars 23 forks source link

Use ExpandableBottomSheetController instead of Key #15

Closed alestiago closed 2 years ago

alestiago commented 2 years ago

Status

DEVELOPING

Breaking Changes

YES

Description

Includes an ExpandableBottomSheetController to control the state of the ExpandableBottomSheet.

Attempt to resolve issues #14.

Type of Change

alestiago commented 2 years ago

@torbenkeller this is a breaking change. This PR is still under development.

Would like to get some feedback from you and developers before updating the documentation. What do we think of this change?

torbenkeller commented 2 years ago

Hey @alestiago, at first, thank you for the PR and I'm sorry for not reviewing it the last two weeks! I hope I can find more time in the future to do this kind of things.

Now to your PR: Usually in Flutter controllers are implemented in a different way than you did. They implement Listenable (usually by extending ChangeNotifier or ValueNotifier) and the widget itself just listens to the changes of the controller and updates itself on a change. The whole logic of the value manipulation lies then in the controller.

I think a very fancy approach would be to let the ExpandedBottomSheetController extend AnimationController. The value of the animation would then be the _positionOffset. Then we could also move all the animation behaviour properties into the controller.

alestiago commented 2 years ago

Thanks @torbenkeller for the feedback!

I was mimicking the implementation of the controller based on another popular package, named "Sliding up Panel".

I was thinking on extending ChangeNotifier or ValueNotifier. However I agree that, extending AnimationController could be interesting, since moving the logic would make the code more maintainable. I think I can give this a try. Although, I'm not sure when since I'm quite busy with work to; but expect some activity from my side.

alestiago commented 2 years ago

@torbenkeller I'm quite busy during the holiday season, hence I might be somewhat inactive. I don't know if you will be able to continue developing from the current changes.

Hopefully, we can get this done soon.