Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic shortcuts for starred contacts #249

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Grafcube
Copy link

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • It uses the custom sorting if set.
  • It uses ACTION_DIAL instead of ACTION_CALL if calling permission is denied.

Fixes the following issue(s)

Acknowledgement

@naveensingh naveensingh added the testers needed We need testers to test changes in this PR. label Sep 28, 2024
@Aga-C
Copy link
Member

Aga-C commented Sep 28, 2024

I think that visible shortcuts should always be the first ones according to current sorting of Favorites tab. For example, if I sort descending by name, I want to see in shortcuts three first items by that sorting, not by the default one; if I use custom sorting, I want to have specific contacts visible at the top, so also in shortcuts.

@Grafcube
Copy link
Author

Done! I recall sorting the list at first but I must have accidentally removed it while refactoring.

@Aga-C
Copy link
Member

Aga-C commented Sep 28, 2024

I've downloaded your latest changes and sorting is still not applied. See screenshots:

obraz

obraz

Also, there are two other bugs:

  • Changing sorting doesn't invoke refreshing shortcuts.
  • After fresh installation of the app, there are no shortcuts at all. I have to add/remove any contact from favorites first.

@Grafcube
Copy link
Author

Grafcube commented Sep 28, 2024

The changes aren't applied immediately. It's called onResume so it'll apply if you close the app and reopen it (you don't have to fully close it, you can open it through recents as well).

I have to add/remove any contact from favorites first.

I didn't need to add/remove anything when I was testing it so I'm not sure why that's happening.

@Aga-C
Copy link
Member

Aga-C commented Sep 28, 2024

I did it, and sorting is still not applied.

@Grafcube
Copy link
Author

Grafcube commented Sep 28, 2024

I just tested it again with a fresh install and it seems to work fine. It only shows up after the second or third time I open the app after a fresh install, but after that it reflects sorting changes relatively quickly.

I'm not sure where the issue is happening.

Edit: Just to clarify, I'm talking about the sorting in the Favourites tab.

@Grafcube
Copy link
Author

Grafcube commented Oct 1, 2024

@Aga-C can you test out the latest commit? It should work now.

@Aga-C
Copy link
Member

Aga-C commented Oct 1, 2024

But you still haven't solved a problem, which is in your code:

                if (order.isEmpty()) {
                    starred.sorted()
                } else {
                    val orderList = Converters().jsonToStringList(order).withIndex().associate { it.value to it.index }
                    starred.sortedBy { orderList[it.contactId.toString()] }
                }

You always assume, that if order is set, that's the only way to sort, while you should check config.isCustomOrderSelected. That's why I haven't seen changes - I had order set, but I was changing sorting to different from custom.

@Grafcube
Copy link
Author

Grafcube commented Oct 1, 2024

I see. Sorry about that, it seems like I fundamentally misunderstood the issue. It should be fixed now.

@Aga-C
Copy link
Member

Aga-C commented Oct 1, 2024

It still doesn't show the first three items from the current sorting. Check the recording below:

qemu-system-x86_64_xeknsQ9wCX.mp4

Also, one of these contacts (Test Hmm) is created as a private contact. It should still display it.

@Grafcube
Copy link
Author

Grafcube commented Oct 1, 2024

At this point, I genuinely have no idea where the problem is. I'm just not encountering that issue so I have no way to debug it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testers needed We need testers to test changes in this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add launcher shortcuts to call contacts
3 participants