miguelhincapie / CustomBottomSheetBehavior

Custom BottomSheetBehavior for Android that mimic Google Maps behavior
Apache License 2.0
915 stars 184 forks source link

mLastStableState was not set on setState #43

Open cesardeazevedo opened 7 years ago

cesardeazevedo commented 7 years ago

Hi, thank you so much for this project.

I had a issue when setting state directly with setState, and it seems the mLastStableState was not set on this line: https://github.com/miguelhincapie/CustomBottomSheetBehavior/blob/master/app/src/main/java/co/com/parsoniisolutions/custombottomsheetbehavior/lib/BottomSheetBehaviorGoogleMapsLike.java#L563

What happens is when we set the state directly from expand to collapse, and starting dragging again, the bottom sheet makes a "hard jump" expanding like crazy, this happens on the sample project when we press the navigation close icon.

I've solved by moving the assignment out of the if (mViewRef == null) statement, but i'm still not sure about that, i just would like to point that, and hear this from you, and others who having this issue.

i am currently integrating this awesome project with react-native (react-native-bottom-sheet-behavior), but this isn't related though.

Thank you.

michael-rapp commented 6 years ago

I encountered the same issue and the change proposed by @cesardeazevedo solved the problem. I did not notice any negative side effects so far.

miguelhincapie commented 6 years ago

ok, so I think we can close this issue right?

michael-rapp commented 6 years ago

The fix works, yes. But it is not part of this repository, is it?

cesardeazevedo commented 6 years ago

I think this should be definitely part of this repository, i just didn't created a pull request earlier because i wasn't sure of any side effects, but i haven't seen so far.

@michael-rapp would you mind to create a pull request?

michael-rapp commented 6 years ago

I have created a pull request (https://github.com/miguelhincapie/CustomBottomSheetBehavior/pull/59) including the changes I used to solve this issue. My solution is slightly different from the one suggested by @cesardeazevedo as I think that the variables mState and mLastStableState should always be set together. As mentioned before, I did not encounter any regressions after applying this patch.