Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Issue#344 Snackbar added for user awareness resolved #358

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue#344 Snackbar added for user awareness resolved #358

wants to merge 1 commit into from

Conversation

harshitagupta30
Copy link
Collaborator

Fixes issue #344
@kunall17 Please review!

@smarx
Copy link

smarx commented Feb 1, 2017

Automated message from Dropbox CLA bot

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

@kunall17
Copy link
Collaborator

kunall17 commented Feb 1, 2017

Snackbar overlaps the FAB can you fix this?

snackbar fab

@harshitagupta30
Copy link
Collaborator Author

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

@harshitagupta30
Copy link
Collaborator Author

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!

videotogif_2017 02 02_16 12 51

@kunall17
Copy link
Collaborator

kunall17 commented Feb 2, 2017

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

@harshitagupta30
Copy link
Collaborator Author

harshitagupta30 commented Feb 2, 2017

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

@harshitagupta30
Copy link
Collaborator Author

@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
Copy link
Collaborator

kunall17 commented Feb 3, 2017

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
Copy link
Collaborator Author

@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
Copy link
Collaborator

kunall17 commented Feb 4, 2017

@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
Copy link
Collaborator Author

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

@kunall17
Copy link
Collaborator

kunall17 commented Feb 6, 2017

Still overlapping
device-2017-02-07-030901


import java.util.List;

/**
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this author details, the git history has the author details!

public void onViewCreated(View view, @Nullable Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
CoordinatorLayout coordinatorLayout = (CoordinatorLayout) getActivity().findViewById(R.id.coordinatorLayout);
snackbar = Snackbar.make(coordinatorLayout,R.string.no_new_messages,Snackbar.LENGTH_SHORT)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format the whole code (ctrl + alt + L)

@kunall17
Copy link
Collaborator

kunall17 commented Feb 6, 2017

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
Copy link
Collaborator Author

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
Copy link
Contributor

@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
Copy link
Collaborator Author

@kunall17 Can you please review it? 👍

@kunall17
Copy link
Collaborator

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
Copy link
Collaborator Author

harshitagupta30 commented Feb 14, 2017

@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
Copy link
Collaborator

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

@harshitagupta30
Copy link
Collaborator Author

harshitagupta30 commented Feb 14, 2017

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
Copy link
Collaborator

@harshitagupta30
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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

@harshitagupta30
Copy link
Collaborator Author

harshitagupta30 commented Feb 21, 2017

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 👍

@harshitagupta30
Copy link
Collaborator Author

@kunall17 Can you please review it? 👍

@kunall17
Copy link
Collaborator

kunall17 commented Mar 5, 2017

@harshitagupta30
The custom behaviour you made is nice, but the problem is the perfect sync which was there between the toolbar and the FAB has gone now
screenshot_20170305-110243

@harshitagupta30
Copy link
Collaborator Author

@kunall17 Any suggestions to make it sync again? :)

@kunall17
Copy link
Collaborator

kunall17 commented Mar 5, 2017

Yeah use the same behaviour in both AppBarLayout and FAB!

@zulipbot
Copy link
Member

zulipbot commented May 4, 2017

@harshitagupta30, your pull request has developed a merge conflict! Please review the most recent commit (f791888) for conflicting changes and resolve your pull request's merge conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants