Issue#344 Snackbar added for user awareness resolved

[harshitagupta30]

Fixes issue #344 @kunall17 Please review!

[smarx]

Automated message from Dropbox CLA bot

@harshitagupta30, it looks like you've already signed the Dropbox CLA. Thanks!

[kunall17]

Snackbar overlaps the FAB can you fix this?

[harshitagupta30]

didn't notice that! Yeah, I'll fix this.

[harshitagupta30]

Hi @kunall17! the best solution to this problem I could find is to hide the fab whenever snackbar appears and as soon as snackbar disappears fab will appear which doesn't overlap at all and looks good!

[kunall17]

How about using the animations which are currently being used rather than this fading?

[harshitagupta30]

I didn't get your point. Can you please point out where its been used?

[kunall17]

https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/activities/ZulipActivity.java#L1100

[harshitagupta30]

@kunall17 I earlier tried to use these animations but they resulted into unusual behaviour of the fab! Sometimes it appears and sometimes it doesn't! It also happened that it was slightly visible at the bottom but not complete. That is why I opted out for adding the new behavior.

[kunall17]

Weird this method is being used at many places for animation hiding and stuff The point is two animations for hiding the FAB isn't looking good one fadeout and one sliding!

Any possibility for sliding this FAB out?

[harshitagupta30]

@kunall17 According to the documentation everywhere whenever we have snackbar along with the fab in coordinator layout then when the snackbar appears the fab will itself move up and tried that approach but not got that result. Then use the animation which was resulting into the fab being half hidden and half visible and then I came out with this i.e. hiding the whenever the snackbar pops out.

[kunall17]

@harshitagupta30 yeah it would have if we were using the default behavior of the fab and the coordinator layouts! Here we use custom one's hence this does not works!

[harshitagupta30]

@kunall17 Can you please review this and finalise the changes in this PR so that this issue can be resolved? Thanks!

[kunall17]

Still overlapping

[kunall17]

This is still overlapping because i think your behaviour is not being used now, because behaviour is set here by java https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/activities/ZulipActivity.java#L1674

[harshitagupta30]

Hi @kunall17! I've made the changes as told by you and yes that behavior was not working! I've added the required code and I guess its working fine now! Can you please review it? :)

[brainwane]

@harshitagupta30 Thanks for your pull request, and thanks for writing a descriptive commit message. Here's a guide to help you write commit messages that are more consistent with our style, which makes it easier for us in the future -- when someone's trying to solve a new problem, if commit messages are all in the same format, it's easier to skim them in logs and quickly understand what's happening.

Here are some tips on tidying your commit history, including changing your commit messages. And when you update your code or your commit message, you can still of course update your pull request and the changes will appear here. :)

Thanks!

[harshitagupta30]

@kunall17 Can you please review it? :+1:

[kunall17]

This removes the sync b/w removing toolbar and FAB which was there before, and sometimes the FAB automatically starts fading (tried on emulator don't know about a physical device)

You should not remove the old Behaviour which was being used, edit the current behaviour i found some good article which might help you achieving this https://www.bignerdranch.com/blog/customizing-coordinatorlayouts-behavior/

Otherwise the PR is good for a merge! 👍

[harshitagupta30]

@kunall17 I am referring to the same article! I tried to apply the same thing here :D But alas it's not working here! Now, what should I do? any suggestions?

[kunall17]

What changes you tried to make to things working on the old behaviour file?

[harshitagupta30]

I added these 2 lines in MessageListFragment

CoordinatorLayout.LayoutParams params = (CoordinatorLayout.LayoutParams) floatingActionButton.getLayoutParams();

params.setBehavior(new ShrinkBehaviorOfFab());

after you said that the behavior is not set.

[kunall17]

No, i'm talking about changes in https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/util/RemoveViewsOnScroll.java

[harshitagupta30]

I've made no changes in this file instead I've added a new class ShrinkBehaviorOfFab and point the fab behavior to this class

[kunall17]

So creating a new behaviour removes the old behaviour as well, which are to supported now as well, so you need to edit the old behaviour!

[harshitagupta30]

No, I haven't changed the old behavior.
This(https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/util/RemoveViewsOnScroll.java) class is having behavior for both app layout and floating button so I added a new class which are keeping track of both old and new behavior of the fab

[kunall17]

So that's what I'm saying so far you need to edit this file https://github.com/zulip/zulip-android/blob/master/app/src/main/java/com/zulip/android/util/RemoveViewsOnScroll.java use conditions and sync up with the snackbar/coordinator layout! So as to have the new beviour and the old one's as well!

[harshitagupta30]

I guess the issue may be caused because of using two behaviors coz in ZulipActivity we are using RemoveViewsOnScroll and in MessageListFragment we are using ShrinkBehaviorOfFab which might be the reason. Let me know if I am thinking right.

[harshitagupta30]

Yeah I got it know! I'll make those changes and update the PR. One more thing instead of updating RemoveViewsOnScroll.java can I update all the changes related to fab in ShrinkBehaviorOfFab.java and keep RemoveViewsOnScroll.java for shrinking AppBarLayout?

[kunall17]

Your work will increase way more then, It would be better to have the RemoveViewsOnScroll!

[harshitagupta30]

Hi @kunall17! I tried to merge the changes in RemoveViewsOnScroll but it is not producing the desired output! So I just wrote the methods hideView() and showView() in ShrinkBehaviorOfFab and its working fine. Can you please review it? Thanks :+1: