Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: greta <gretadoci@gmail.com>
GretaD
force-pushed
the
feat/sort-favorite-contacts
branch
from
658fb4b to
693cb61
Compare
GretaD
added
3. to review
|
Cleaned-up the PR description so it's clear that we can not merge this before the cdav merge, release and update here |
Signed-off-by: greta <gretadoci@gmail.com>
GVodyanov
left a comment
GVodyanov
left a comment
There was a problem hiding this comment.
Tested it and works great!
Just a few styling comments, will approve to not block.
I also noted that there is the age old issue representing itself (at least to me in firefox)
Right after favouriting u get the star stuck on the side of the component until you hover over it, but that's almost more of a vue or browser bug probably, I had this on other PRs too and wasn't able to fix it.
DerDreschner
left a comment
DerDreschner
left a comment
There was a problem hiding this comment.
For me, the sorting is... strange?
simplescreenrecorder-2026-05-06_15.14.52.mp4
And when I open a contact, the favorite status is being lost as it's only included in the PROPFIND for the address book by the cdav-library right now, not the contact itself. Could you have a look?
If necessary, I can modify the cdav-library to retrieve the favorite property as well. But as there is no star indicator on the contact itself, only the contacts list, I see no reason for retrieving it there as well.
Signed-off-by: greta <gretadoci@gmail.com>
DerDreschner
left a comment
DerDreschner
left a comment
There was a problem hiding this comment.
Retested and the sorting is now correct! 😄 Only two things that can be fixed in follow-ups:
- When selecting a contact marked as favorite, the contact list jumps unexpectedly so the selected contact is on the top for some reason:
simplescreenrecorder-2026-05-07_14.55.00.mp4
- Hiding the star for address books that don't have the
{DAV:}write-propertiesright (see nextcloud/server#60183 (comment))
| try { | ||
| await this.$store.dispatch('toggleFavorite', contact) | ||
| } catch (error) { | ||
| console.error('Could not toggle favorite state', error) |
There was a problem hiding this comment.
Should have user feedback too, showError
| return | ||
| } | ||
| if (!contact.dav) { | ||
| console.error('Missing DAV object for contact', contact.key) |
There was a problem hiding this comment.
use logger instead of console
| */ | ||
| async toggleFavorite(context, contact) { | ||
| if (!contact.dav) { | ||
| console.error('Missing DAV object for contact', contact.key) |
There was a problem hiding this comment.
Maybe throw exceptions instead, so the user is aware
| context.commit('updateContactFavorite', contact) | ||
| await contact.dav.updateProperties() |
There was a problem hiding this comment.
These 2 have to be flipped, so the star is not added when the change is not saved
|
Read-only is fine. We want the feature to be usable for the SAB and for read-only shared ABs. It should only not be usable for the recently contacted, because that data is volatile. |
So it's supposed to work for SAB cards ? |
Signed-off-by: greta <gretadoci@gmail.com>
5 participants
Fixes #105
favoriteproperty for each vCard cdav-library#1029 merged, and releasedHow to test
You need to link cdav-library with contacts while in this pr: nextcloud/cdav-library#1029