immich icon indicating copy to clipboard operation
immich copied to clipboard

feat(mobile): allow contact picking for names in mobile app

Open samolego opened this issue 9 months ago β€’ 15 comments
trafficstars

Hi there!

I love immich but thought it could use a minor improvement when naming your persons. If you do it via mobile app, we have access to user's contact list, so we can use it to suggest people names.

I've implemented that, you can see how it looks in the attached screenshots:

Here's a (very blurred) video as well, I had little time to edit but I hope you get the point:

https://github.com/user-attachments/assets/615bea51-11bb-44e9-9e34-f56906da03d4

Edit: better video with some fake contacts:

https://github.com/user-attachments/assets/a211cb81-ae21-4420-b671-8b300db97dc6

I'm opening this as a draft as design (especially of dropdown) can be improved and I'd like to get a feedback on what you think

samolego avatar Jan 31 '25 22:01 samolego

Hello, this is very cool!

alextran1502 avatar Jan 31 '25 22:01 alextran1502

I've since done some other designs but was actually the most satisfied with this one. I'm marking this as open for review.

samolego avatar Feb 02 '25 09:02 samolego

I understand this only checks the Android phone local contacts.

What about:

  1. Checking first if the name already exists on the server and merge the faces, like on the WebUI?
  2. What about the iOS app?

phipz avatar Feb 04 '25 11:02 phipz

This uses https://pub.dev/packages/fast_contacts so it should work on iOS too. I don't have an iPhone, so I couldn't test it. Maybe someone else can?

Merging faces is possible but imho that should be a separate PR?

samolego avatar Feb 04 '25 13:02 samolego

If you deny Contacts permission (people might not want to have immich accessing contacts), it fails to achieve contacts permissions and puts away the keyboard every time, so I have to tap the text box:

https://github.com/user-attachments/assets/242c1597-c3f0-4fc6-ad0b-941811ecb4b1

Regular app: Debug build of your branch (the box is wider, just something to notice):

Also, with Contacts allowed, it works great (tested on Android), shows the contact names as you type and when you tap on one it fills the name in immich for you. Maybe someone can add a feature to set the contact photo right then and there (similar to Samsung Gallery (and maybe GPhotos)).

NicholasFlamy avatar Feb 05 '25 00:02 NicholasFlamy

Thanks for spotting! I fixed the keyboard issue, also set the dialog width to MediaQuery.of(context).size.width * 0.6, which looks similar to before.

After first time (after asking for the permission) keyboard can still be hidden, though it works fine after that. I couldn't get it to show right after clicking on the permission dialog though.

samolego avatar Feb 05 '25 09:02 samolego

After first time (after asking for the permission) keyboard can still be hidden, though it works fine after that. I couldn't get it to show right after clicking on the permission dialog though.

To be clear, it works fine for me if I allowed the Contacts permission. The video is after I denied Contacts permission and Android doesn't allow the app to ask again (cause I denied at least twice in a row).

Edit: I will test this again when I get home.

NicholasFlamy avatar Feb 05 '25 14:02 NicholasFlamy

To be clear, it works fine for me if I allowed the Contacts permission. The video is after I denied Contacts permission and Android doesn't allow the app to ask again

Yeah, got that - it should be fixed with the above commits :).

samolego avatar Feb 05 '25 16:02 samolego

To be clear, it works fine for me if I allowed the Contacts permission. The video is after I denied Contacts permission and Android doesn't allow the app to ask again

Yeah, got that - it should be fixed with the above commits :).

It behaves correctly now. There is a very slight stutter while it opens so I need to build a release apk and test that to confirm if it's this PR or if it's just the debug build being JIT or wtv.

NicholasFlamy avatar Feb 06 '25 05:02 NicholasFlamy

Good job! It works great! No issues with the permission allowed or denied.

NicholasFlamy avatar Feb 08 '25 01:02 NicholasFlamy

Could someone with permissions approve the workflow runs, so I know if I need to do anything more 😊? Thanks!

samolego avatar Feb 25 '25 10:02 samolego

@samolego You don't need to do anything more, just need some more actual testings

alextran1502 avatar Feb 25 '25 15:02 alextran1502

Does anyone have a way to build an IPA of this? I don't have a real Mac (only a VM) so it'd be hard for me to do it, but I can sideload an IPA to test this on an iPhone.

Edit: I'll try and make a GitHub Workflow to build an IPA on my fork.

NicholasFlamy avatar Feb 25 '25 15:02 NicholasFlamy

@samolego You don't need to do anything more, just need some more actual testings

Ah, thanks!

samolego avatar Feb 25 '25 16:02 samolego

I feel like this could have a psychological regression down the road. :)

a) The reason for the permission prompt is not explained before prompting. There's a weak connection to the activity (renaming a face) β€” an unrelated ask at first, the kind which basic security training warns to steer clear and uninstall the app when encountered (in a calculator).

b) Some apps (ex. Meta) aggressively break when denied non-critical invasive perms, it might not be obvious it's safe to reject. Putting it out there with the pre-prompt may be smart.

c) When permission is already granted either by the person who set up the app, or just not remembering, it may be alarming to see Immich 'imported' all user's contacts full details (not the case) with seemingly no easy way to go back. The guess can be fueled with a pre-personalized image from the contacts book, it may be smarter to consciously show phone contacts without an image, and instead with a '+' β€” showing the name could be added as a suggestion, and not already present. Listing local contacts last (may push down more relevant results) affirms its position as a suggestion. It could (adding complexity) be paired with a small header text 'Contacts from this phone'.

The changes would improve on the feel, that the app takes care of you (on-brand) with informed consent, ft. changing your mind is non-consequential. The changes may be instead pushed to a follow-up as well.

Edit: Sorry to OP for shutting this down :( lmk if I can do something for you.

jtagcat avatar Feb 27 '25 00:02 jtagcat

I'm going to close this PR, it's a cool idea, but my reasons for closing are these:

  • It requires an extra permission that feels intrusive to user privacy
  • Realistically it saves maybe a couple of seconds for an operation you only do once per person you're adding
  • It's just generally very niche and probably not something most people care about, but is more maintenance in the mobile code. It's a nice-to-have, but given it requires the permission, I don't think that's worth it.

zackpollard avatar Mar 03 '25 12:03 zackpollard

I'm disappointed but understand the reasoning.

Would an opt-in in the settings page for this feature be reconsidered? (Still adds mobile code to maintain but not as worrisome for users.)

NicholasFlamy avatar Mar 03 '25 13:03 NicholasFlamy

We don't want everything to be a settings

alextran1502 avatar Mar 03 '25 13:03 alextran1502

We don't want everything to be a settings

Yes, Symfonium and some other apps are a perfect example of why you don't want that. They have so many settings that it's overwhelming and you can never find what you need and they even have to implement a settings search. Simply put, I agree. πŸ™‚

NicholasFlamy avatar Mar 03 '25 13:03 NicholasFlamy

We don't want everything to be a settings

Yes, Symfonium and some other apps are a perfect example of why you don't want that. They have so many settings that it's overwhelming and you can never find what you need and they even have to implement a settings search. Simply put, I agree. πŸ™‚

It's overwhelming for the user and also makes the codebase awful, we already have too many settings :sweat_smile:

zackpollard avatar Mar 03 '25 14:03 zackpollard

Ah, that's kinda sad, though I understand the reasoning with user-privacy-permission. I'd suggest adding opt-in button to show contacts, but that doesn't change the permission requirement.

samolego avatar Mar 03 '25 14:03 samolego