search

Ashok-Varma / BottomNavigation

This Library helps users to use Bottom Navigation Bar (A new pattern from google) with ease and allows ton of customizations
Apache License 2.0
4.35k stars 689 forks source link

Toggle hide breaks scrolling behavior #45

Open jt-gilkeson opened 8 years ago

jt-gilkeson commented 8 years ago

Scroll sample app down so that bottom bar disappears, click toggle hide (bar re-appears), scroll down again - bar does not disappear. If you scroll up and then down again it fixes itself.

When you unhide the bar you should be able to scroll down and have it hide again.

Seems like the bar gets into the wrong state in BottomVerticalScrollBehavior (Appears calling hide and unhide doesn't update the current state of the behavior?)

When I look at how other behaviors are implemented (like FAB) they check the current visibility of the childview - which gets rid of the need to try to track the state of hidden vs showing and trying to keep that in sync.

Use Case: User scrolls to the bottom of the screen (bar is hidden), then does something that programatically scrolls to the top of the screen - programmatic scrolling doesn't affect coordinator layout so you call unhide to show the bottom bar (and call setexpanded() on appbarlayout). Now the user scrolls down - the appbarlayout correctly scrolls to hidden, and the bottom bar should also scroll to hidden again.

jt-gilkeson commented 8 years ago

Created pull request to fix.

Ashok-Varma commented 8 years ago

i will check that and merge that this weekend

jt-gilkeson commented 8 years ago

This is still broken in 1.3.0 - If you scroll upwards (so bar hides), toggle hide (twice - due to bug) (to show bar), then scroll upwards again, it does not hide the bar.

onNestedVerticalScrollConsumed doesn't get called in BottomVertcialScrollbarBehavior in this state.

Also, Sample App toggle hide is still broken - instead of using a variable (that gets out of sync) it should BottomNavBar.isHidden() to determine whether to hide or show when you toggle.

Ashok-Varma commented 8 years ago

@jt-gilkeson This is only broken in Sample app because sample app was not updated. i need to update it. But the library bug was fixed

jt-gilkeson commented 8 years ago

The library is NOT fixed.

Sorry in my steps I said scroll down - but what I meant is scroll up (slide the text upwards - the action to hide the bottom bar).

Our app calls show() and then the bar does not hide when scrolling upwards. If you follow the steps I provided you will see that the scrolling behavior is not working correctly after calling show.

Again to test - scroll up (in any app), call show(), then scroll up again - the bar does not hide like it should. I believe the other unrelated changes that were made to the nestedscroll handling broke this. As I mentioned, once the bar has been shown using show(), the onNestedVerticalScrollConsumed (or more importantly handleDirection() stops being called when scrolling up.

Ashok-Varma commented 8 years ago

@jt-gilkeson thanks i checked it now. i will push a fix mostly today/ this weekend.

MajaPamukovic commented 6 years ago

Hi, this issue is still present in library versions 2.0.5 and 2.1.0. Any chance this will be fixed in the near future?