Skip to content

feat: sort favorite contact first in list#5251

Merged
GretaD merged 1 commit into
mainfrom
feat/sort-favorite-contacts
May 8, 2026
Merged

feat: sort favorite contact first in list#5251
GretaD merged 1 commit into
mainfrom
feat/sort-favorite-contacts

Conversation

@GretaD
Copy link
Copy Markdown
Contributor

@GretaD GretaD commented Apr 10, 2026

Fixes #105

Screenshot from 2026-04-22 11-25-30

How to test

You need to link cdav-library with contacts while in this pr: nextcloud/cdav-library#1029

@GretaD GretaD self-assigned this Apr 10, 2026
@GretaD GretaD added enhancement New feature or request 2. developing Work in progress labels Apr 10, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 10, 2026

Codecov Report

❌ Patch coverage is 0% with 105 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/store/contacts.js 0.00% 46 Missing and 22 partials ⚠️
src/components/ContactsList/ContactsListItem.vue 0.00% 30 Missing ⚠️
src/models/contact.js 0.00% 4 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@GretaD

This comment was marked as outdated.

@GretaD GretaD force-pushed the feat/sort-favorite-contacts branch from 658fb4b to 693cb61 Compare May 4, 2026 09:23
@GretaD GretaD marked this pull request as ready for review May 4, 2026 09:25
@GretaD GretaD requested review from GVodyanov and hamza221 as code owners May 4, 2026 09:25
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 4, 2026
@DerDreschner DerDreschner self-requested a review May 4, 2026 10:03
Comment thread src/store/contacts.js Outdated
Comment thread src/store/contacts.js Outdated
Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/store/contacts.js
@ChristophWurst
Copy link
Copy Markdown
Member

Cleaned-up the PR description so it's clear that we can not merge this before the cdav merge, release and update here

Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Copy link
Copy Markdown
Contributor

@GVodyanov GVodyanov left a comment

Choose a reason for hiding this comment

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

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)

Image

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.

Copy link
Copy Markdown
Contributor

@DerDreschner DerDreschner left a comment

Choose a reason for hiding this comment

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

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.

@GretaD GretaD requested a review from DerDreschner May 7, 2026 12:37
Copy link
Copy Markdown
Contributor

@DerDreschner DerDreschner left a comment

Choose a reason for hiding this comment

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

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

Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/components/ContactsList/ContactsListItem.vue Outdated
Comment thread src/store/contacts.js Outdated
Comment thread src/store/contacts.js Outdated
Copy link
Copy Markdown
Contributor

@hamza221 hamza221 left a comment

Choose a reason for hiding this comment

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

Image IMO the button shouldn't show for readonly contacts

@ChristophWurst
Copy link
Copy Markdown
Member

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.

Comment thread src/store/contacts.js Outdated
Comment thread src/store/contacts.js Outdated
Comment thread src/store/contacts.js Outdated
@hamza221
Copy link
Copy Markdown
Contributor

hamza221 commented May 7, 2026

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 ?
I'll test again cause it didn't work when testing

async toggleFavorite() {
const contact = this.$store.getters.getContact(this.source.key)
if (!contact) {
console.error('Could not find contact in store', this.source.key)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

all console.error should be logger.error instead

@hamza221
Copy link
Copy Markdown
Contributor

hamza221 commented May 8, 2026

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 ? I'll test again cause it didn't work when testing

Works fine btw 👏🏼

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds “favorite contact” support to the Contacts frontend so favorites can be toggled from the list UI and are sorted to the top of contact lists.

Changes:

  • Add favorite-aware sorting and a toggleFavorite Vuex action/mutation flow in the contacts store.
  • Add a favorite getter/setter on the Contact model backed by contact.dav.favorite.
  • Add a favorites toggle control (star) to each contact list item, plus an action menu entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File Description
src/store/contacts.js Introduces favorite-aware sorting, store mutation/action to toggle favorites, and preserves favorite metadata across fetch/update flows.
src/models/contact.js Adds favorite getter/setter that proxies to the DAV layer.
src/components/ContactsList/ContactsListItem.vue Adds UI controls (star + action button) to toggle favorite state from the contact list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/store/contacts.js
Comment on lines 155 to 173
for (let i = 0, len = state.sortedContacts.length; i < len; i++) {
if (sortData(state.sortedContacts[i], sortedContact) >= 0) {
state.sortedContacts.splice(i, 0, sortedContact)
break
} else if (i + 1 === len) {
// we reached the end insert it now
const other = state.sortedContacts[i]

// favorite comes before non-favorite
const differentFavStatus = other.favorite !== sortedContact.favorite
const otherShouldComeFirst = differentFavStatus && other.favorite
const sameFavAndSortedFirst = !differentFavStatus && sortData(other, sortedContact) >= 0

if (otherShouldComeFirst || sameFavAndSortedFirst) {
continue
}

if (i + 1 === len) {
state.sortedContacts.push(sortedContact)
} else {
state.sortedContacts.splice(i, 0, sortedContact)
}
break
}
Comment thread src/store/contacts.js
Comment on lines +50 to +60
// alphabetical within each group
if (!a.value && !b.value) {
return 0
}
if (!a.value) {
return 1
}
if (!b.value) {
return -1
}
return a.value.localeCompare(b.value)
Comment thread src/store/contacts.js Outdated
.sort(sortData)
.map((contact) => ({
key: contact.key,
value: (contact[state.orderKey] || '').toString().toLowerCase(),
Comment thread src/store/contacts.js
Comment thread src/store/contacts.js
Comment thread src/components/ContactsList/ContactsListItem.vue
Comment thread src/components/ContactsList/ContactsListItem.vue
Comment on lines +39 to +47
<div
class="favorite-star"
:class="{ favorite: isFavorite }"
@click.stop="toggleFavorite">
<StarIcon
v-if="isFavorite"
:size="16"
class="favorite-icon" />
</div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@DerDreschner
Copy link
Copy Markdown
Contributor

Works fine btw 👏🏼

Thanks for the feedback! Makes my blood pressure going down a lot 😄

@hamza221
Copy link
Copy Markdown
Contributor

hamza221 commented May 8, 2026

requires #5310

@ChristophWurst
Copy link
Copy Markdown
Member

requires #5310

Showtime

@GretaD GretaD force-pushed the feat/sort-favorite-contacts branch from b4000c1 to eaca005 Compare May 8, 2026 11:47
@GretaD GretaD requested a review from hamza221 May 8, 2026 12:02
@GretaD GretaD force-pushed the feat/sort-favorite-contacts branch 3 times, most recently from 2ca5373 to d874b8d Compare May 8, 2026 12:18
Signed-off-by: greta <gretadoci@gmail.com>
@hamza221 hamza221 force-pushed the feat/sort-favorite-contacts branch from d874b8d to bf76118 Compare May 8, 2026 14:21
@GretaD GretaD merged commit 1b82f16 into main May 8, 2026
35 of 36 checks passed
@GretaD GretaD deleted the feat/sort-favorite-contacts branch May 8, 2026 15:09
Copy link
Copy Markdown
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Very nice, thank you! :)

@hamza221
Copy link
Copy Markdown
Contributor

/backport to stable8.5 please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sort favorite contacts first in list

7 participants