search

owncloud-archive / maps

:globe_with_meridians: Maps app for ownCloud
GNU Affero General Public License v3.0
42 stars 20 forks source link

fix contact display #53

Closed v1r0x closed 9 years ago

v1r0x commented 9 years ago

check if address is set for current user and only display it if an address exists.

v1r0x commented 9 years ago

should fix #14 and #37

Henni commented 9 years ago

Doesn't fix #14 Instead I now see: image

edit: i tested with two contacts, only one of them has an address.

v1r0x commented 9 years ago

Yes, https://github.com/v1r0x/maps/blob/fix-contacts/js/script.js#L733 doesn't get updated, but all contacts are shown correctly.

Henni commented 9 years ago

@v1r0x It only fixes #37 then? I didn't have a closer look at the source of maps yet, so maybe you can describe what this has to do with https://github.com/v1r0x/maps/blob/fix-contacts/js/script.js#L733.

Do you have an idea how to fix #14?

v1r0x commented 9 years ago

It basically fixes both. The only thing which is currently broken is the progress bar. I'm not a javascript programmer and don't get the problem (should work as it did before).

The link I provided is the line where the total number of contacts is set. In my PR this is set to 0 (and I don't know why). This is why it displays the "x of 1" and infinty%

v1r0x commented 9 years ago

Is it possible to check if the address is undefined and only do the lookup if it is not?

Henni commented 9 years ago

@v1r0x how about checking if the addresse is undefined in the function toolKit.adresLookup()?

v1r0x commented 9 years ago

Yes, I had a look at the code and this seems good. Thanks for the help! Should work now

Henni commented 9 years ago

@jancborchardt could you please take a look and confirm that this fixes your issue #14

@v1r0x It would be great if you could fix a small beauty issue. The loading bar should be reset at the beginning of loading the contacts. Otherwise you see the completed loading bar for a short time.

jancborchardt commented 9 years ago

WOW! This looks great! And fixes #14 for me. The only issue is that the actual avatars are not shown, just these dark silhouettes for everyone.

But I suppose adding that functionality is for a separate pull request?

v1r0x commented 9 years ago

Yes, I think this is another problem (hopefully^^). Another problem: If a contact has more than one address, only one is shown

Henni commented 9 years ago

@jancborchardt yes this should be a different pull request Thanks for the feedback!

@v1r0x I think it is ok to just show one address per contact, but it is important that the 'preferred' address is chosen.

jancborchardt commented 9 years ago

Should we merge it then? :+1: from me. @Henni if you think it’s good press the button. :)

Henni commented 9 years ago

Let's fix the loading bar in another PR.