Cloudkibo / Android

0 stars 0 forks source link

Show Avitar in Chat list, Contact list & Inviate list #307

Closed jekram closed 7 years ago

jekram commented 7 years ago

Like WhatsApp we should show the Avitar in Chat list, Contact List and Invite list

dayemsiddiqui commented 7 years ago

I am taking up this issue

dayemsiddiqui commented 7 years ago

@jekram can you further elaborate on this issue, images are already being displayed in chatlist

screenshot_2016-11-23-10-07-03

jekram commented 7 years ago

The Avitar should come from the contact / address book.

This is what we show:

image

This is what we should show: image

dayemsiddiqui commented 7 years ago

Ok Thanks.

dayemsiddiqui commented 7 years ago

@jekram I have implemented the functionality: screenshot_2016-11-24-21-44-31

However, I am facing a performance issue, I am loading all the images from contacts when you press the contact button, however this is causing a lag, and it causes a delay of 3 to 5 seconds when we open the contact fragment.

Also, I noted that whatsapp does not fetch image from our contacts rather it gives the user an option to upload their image and fetches it from server. I am trying to figure out how can I improve the performance.

Heres the example of whatsapp:

screenshot_2016-11-24-21-53-26

jekram commented 7 years ago

Great job.

One small thing when we do not have the Avatar let's show it gender neutral. We are showing a male face.

dayemsiddiqui commented 7 years ago

@jekram ok will make it gender neutral :smile:

dayemsiddiqui commented 7 years ago

@jekram @sojharo I figured out the reason for performance issue, basically after reading photo uri from contacts, I need to decode it into bitmap, the default android method setImageUri does this on the UI thread which causes latency issues, people on stackoverflow are recommending to use a background service for it. However there are a couple of libraries that make this task easier. I am thinking of using one of those libraries.

dayemsiddiqui commented 7 years ago

@jekram @sojharo I have found this really good library for media management, it caches the images, load images asychronously and provide other means to effective manipulate images while giving significant performance boosts.

dayemsiddiqui commented 7 years ago

@jekram how about this?

screenshot_2016-11-25-01-49-06

This is still little bit of performance lag because of the query to fetch image uri from contact. For now I am pushing this code, I will further discuss this with @sojharo that how can we improve the performance.

jekram commented 7 years ago

Thanks for the update.

Talk to @sojharo for performance.

When you say "query to fetch image uri from contact"? If that local or over the network. I am assuming it is local. Also, we should only fetch it one time and only get back to contact if there was any change in contact.

jekram commented 7 years ago

Build 80 @dayemsiddiqui

This is not working:

Several things are wrong:

  1. I all seeing the number of names on my "Chat list" that I have never chatted .
  2. The "Contacts" and "Invite" nothings happen. A dump was sent to the service.

1 (Contacts) list is correct

image

@sojharo FYI

dayemsiddiqui commented 7 years ago

@jekram let me take a look

dayemsiddiqui commented 7 years ago

@jekram can you please tell me the reproducible steps which caused the bug, I tested the app both by compiling code from my machine and by downloading the app from playstore. It seems to work fine for me. I also tested it using a different number and I was unable to reproduce the bug.

screenshot_2016-11-26-17-10-11

dayemsiddiqui commented 7 years ago

@jekram I figured out the solution, the app is correctly showing all the groups, actually when I was implementing group member add functionality, I added you to these groups, so you are a part of these groups that's why the app is showing the groups, try using the app for a different number, you will find the results as expected.

sojharo commented 7 years ago

It is showing all of your groups because the group sync was implemented by me couple of days ago.

sojharo commented 7 years ago

@jekram @dayemsiddiqui I don't think we need invite screen any more as all of the contacts who are on CloudKibo and who are not on cloudkibo are shown on contacts screen. The people who are not on cloudkibo are shown with invite button. In invite page all the contacts who are not on cloudkibo are shown with invite button. so we should remove invite screen. If we want to keep invite screen, then we should not show people who are not on cloudkibo in contacts screen.

jekram commented 7 years ago

@dayemsiddiqui

This only explains part of the problem.

Today when I hit of the "Chat" button that applications hangs. They were two dumps sent. The same thing was happening when clicking on "Contacts" & "Invite" button.

We have a serious performance issue with the latest change, to the extent that it crashes. The app is not usable.

This is why I have marked this as High.

@sojharo

Please open a task for Group invitation. In WhatsApp when you are invited to the group you are notified. I am not getting any notification when I am getting added to the group. Please follow the same workflow as Whatsapp for adding people to the group.

dayemsiddiqui commented 7 years ago

@jekram you are correct there is a performance issue, right now the app fetches the images everytime when you click the chat button, this can be prevented by loading the relevant imageUri at app install, or running this fetching as a background service, I am trying to implement the first approach as it is better.

jekram commented 7 years ago

Thanks for your quick response.

jekram commented 7 years ago

Also, agree with @sojharo on removing the "Invite Screen"

sojharo commented 7 years ago

@dayemsiddiqui please look into the following Android documentation. Here is the way to put files inside cache. I am not sure but I think if you put some contacts images in cache and then load them from there it would be faster.

https://developer.android.com/training/basics/data-storage/files.html#GetWritePermission

I am not saying this is solution. Just have a look and give your thoughts. Also remember caches have limited storage of 1 MB per application. Also if storage is low on device then Android can remove apps cache without informing.

In whatsapp, I have noticed it doesn't load the images of all contacts at once. It loads them at the time when you scroll down to them.

sojharo commented 7 years ago

I have opened the task for group invitation https://github.com/Cloudkibo/Android/issues/317

dayemsiddiqui commented 7 years ago

@jekram @sojharo thanks I will look into it. I have achieved a significant performance boost by implementing the following:

I will further use @sojharo advice to optimize the performance

jekram commented 7 years ago

Ok.

dayemsiddiqui commented 7 years ago

@jekram @sojharo should I hide the invite tab from the main menu?

jekram commented 7 years ago

Yes. Please. It is redundant.

sojharo commented 7 years ago

@dayemsiddiqui please see if this can help in performance : https://github.com/nostra13/Android-Universal-Image-Loader

jekram commented 7 years ago

Thanks. Sojharo

dayemsiddiqui commented 7 years ago

@sojharo I am already using glide which uses automatically handles caching, I am removing the invite button and pushing the code, you can then merge the changes so you and @jekram can test performance on their end.

dayemsiddiqui commented 7 years ago

@jekram @sojharo I found out a very interesting thing regarding listview to improve performance:

https://developer.android.com/training/improving-layouts/smooth-scrolling.html

The official documentation says that frequently calling the findViewbyId can degrade the performance of listview for that the documentation suggests to implement the viewHolder pattern, secondly it says that we should do all database querying as background async tasks, so now I am going to implement the following two points which will hopefully boost the performance even further:

I will let you the results in terms of performance when I am done with this.

sojharo commented 7 years ago

I had also read somewhere about Android's loader API to load data from database and put in list view. It automatically updates list too. Also please look into this if it helps in performance.

dayemsiddiqui commented 7 years ago

@sojharo thanks

dayemsiddiqui commented 7 years ago

@jekram @sojharo ok so I implemented the changes which I mentioned above in the contact list tab, while keeping the chat list tab unchanged. This made the contact list very responsive. Then I used Android Monitor to analyse the performance of the app, here are the results:

performance_analysis

Ok so after running the app, I performed the following steps to generate this graph:

In contact list I have implemented the following two points which has made it very responsive:

However I think @sojharo 's advice to use Android load api is better as it will be a much cleaner solution.

For now I am implementing the above two points in chat list and pushing the code, however I will study the android's load api and implement it for a final solution before closing this issue.

jekram commented 7 years ago

Thanks for the detailed analysis. I also, agree with the approach. Look forward to the final solution.

dayemsiddiqui commented 7 years ago

@sojharo @jekram I have studied the adnroid's load api I have started implementing it. I will create a pull request once it is done

jekram commented 7 years ago

OK. Thanks

sojharo commented 7 years ago

@dayemsiddiqui I had read some where that android team has made the loader api recommended way to load the lists in android. If you have understood it then let's discuss. I am in office till 9 pm today. So whenever you are free you can come by. It would be nice if we slowly implement loader API in all of our lists i.e. chat list, contact list, conversations, calls log and others.

jekram commented 7 years ago

I installed version 82.

  1. The Chat list came out OK first time

  2. Then I went to Contact list

  3. Then I went back to Chat list I got this blank screen for 3-4 minutes

  4. Eventually, I saw the Chat List

image

image

dayemsiddiqui commented 7 years ago

@jekram yes I am implementing cursor loader after this it will be fixed right now it loads the entire data then shows the results what the cursor loader does it shows data chunk by chunk which gives better response time I am also studying caching to improve performance even further I will try to implement the changes by tonight. Right now the delay time increases with the number of contacts

dayemsiddiqui commented 7 years ago

@jekram @sojharo I have made some changes and created a pull request. Basically now the app fetches the images only once at the start, this should remove the delay. I am sorry that this issue is taking so much time to resolve. Also instead of manually fetching the image I am using @sojharo method of android contactService to fetch images only when the contacts are changed

dayemsiddiqui commented 7 years ago

@jekram @sojharo

After testing with these two, the app is running fine for me without any glitches, I commited the changes, hopefully there wont be anymore issue now.

jekram commented 7 years ago

Thanks. I will test this one the code has merged and published in the market place

dayemsiddiqui commented 7 years ago

@jekram can you please test this issue on your end and verify if the problem still persists or it has been solved?

jekram commented 7 years ago

The performance problem has been fixed and I can switch back and forth and it is working. However, now tall the avatars are generic avatars and it is not showing me real avatar s that was the objective of this task.

jekram commented 7 years ago

image

dayemsiddiqui commented 7 years ago

@jekram at my end the performance is ok and the avatars and images were also showing correctly, the code might not have been integrated properly. No worries I will create the pull request with the this change implemented, hopefully after that this issue can be closed.

dayemsiddiqui commented 7 years ago

@jekram I have checked the code is fine, can you try and check after reinstalling the app( first uninstall the current version otherwise the database wont get updated).

jekram commented 7 years ago

Looks good to me. Should we close this ? Is something else is pending on this task?

dayemsiddiqui commented 7 years ago

Yes I think we can close this now.