Contacts icon indicating copy to clipboard operation
Contacts copied to clipboard

Add option to edit structured addresses (new)

Open stephanritscher opened this issue 1 year ago • 14 comments

What is it?

  • [ ] Bugfix
  • [x] Feature
  • [ ] Codebase improvement

Description of the changes in your PR

  • Introduce option to edit structured addresses as requested.
  • Based on this fork,
  • In the current state, the PR changes the dependency to my fork of Commons. This needs to be revisited before merging.

Before/After Screenshots/Screen Record

  • Before:
  • After:

Fixes the following issue(s)

  • Fixes #30.

Relies on the following changes

  • https://github.com/FossifyOrg/Commons/pull/32

Acknowledgement

stephanritscher avatar Jan 28 '24 21:01 stephanritscher

This PR would make Fossify Contacts so much better. I've stayed with Simple Contacts Pro SE, as the structured address feature allows my contacts to integrate very well into my Nextcloud instance.

raycadle avatar Feb 10 '24 14:02 raycadle

@naveensingh What do you think about the feature respectively pull request?

stephanritscher avatar Feb 25 '24 22:02 stephanritscher

@stephanritscher Sounds like a good idea but I'm curious, why wasn't this merged in the original Simple Contacts?

Note: I'm yet to review this or any other PR.

naveensingh avatar Feb 26 '24 03:02 naveensingh

Tibbi didn't like having additional options and didn't regard this as important feature.

stephanritscher avatar Feb 26 '24 09:02 stephanritscher

Note: I'm yet to review this or any other PR.

This is implemented and works well in Simple Contacts Pro SE. Probably the sooner it is merged, the less issues there would be in the process. It is a great feature. I've been fixing addresses for the last 30 years and if it is only because someone didn't find it important enough than it is just plain sad. Please!

alensiljak avatar Mar 19 '24 17:03 alensiljak

@stephanritscher

I did some testing and found a few issues:

  • When the address fields are empty and I disable structured addresses, the structured address fields are still visible.

  • When the address fields are filled and I disable structured addresses, the app crashes:

    Process: org.fossify.contacts, PID: 21824
    java.lang.NullPointerException: Missing required view with ID: org.fossify.contacts:id/contact_city
        at org.fossify.contacts.databinding.ItemEditStructuredAddressBinding.bind(ItemEditStructuredAddressBinding.java:173)
        at org.fossify.contacts.activities.EditContactActivity.setupAddresses(EditContactActivity.kt:621)
        at org.fossify.contacts.activities.EditContactActivity.setupEditContact(EditContactActivity.kt:496)
        at org.fossify.contacts.activities.EditContactActivity.gotContact(EditContactActivity.kt:178)
        at org.fossify.contacts.activities.EditContactActivity.access$startChoosePhotoIntent(EditContactActivity.kt:55)
        at org.fossify.contacts.activities.EditContactActivity.access$gotContact(EditContactActivity.kt:55)
        at org.fossify.contacts.activities.EditContactActivity$initContact$1.invoke$lambda$0(EditContactActivity.kt:164)
        at android.os.Handler.handleCallback(Handler.java:984)
      ...
    
    
  • When I enable/disable structured addresses and press back, the app crashes. Logs:

    Process: org.fossify.contacts, PID: 32013
    java.lang.NullPointerException: Missing required view with ID: org.fossify.contacts:id/contact_city
        at org.fossify.contacts.databinding.ItemEditStructuredAddressBinding.bind(ItemEditStructuredAddressBinding.java:173)
        at org.fossify.contacts.activities.EditContactActivity.getFilledAddresses(EditContactActivity.kt:1212)
        at org.fossify.contacts.activities.EditContactActivity.fillContactValues(EditContactActivity.kt:1136)
        at org.fossify.contacts.activities.EditContactActivity.hasContactChanged(EditContactActivity.kt:367)
        at org.fossify.contacts.activities.EditContactActivity.onBackPressed(EditContactActivity.kt:299)
        at android.app.Activity.onKeyUp(Activity.java:4058)
    ...
    
    Process: org.fossify.contacts, PID: 31881
    java.lang.NullPointerException: Missing required view with ID: org.fossify.contacts:id/contact_address
        at org.fossify.contacts.databinding.ItemEditAddressBinding.bind(ItemEditAddressBinding.java:86)
        at org.fossify.contacts.activities.EditContactActivity.getFilledAddresses(EditContactActivity.kt:1238)
        at org.fossify.contacts.activities.EditContactActivity.fillContactValues(EditContactActivity.kt:1136)
        at org.fossify.contacts.activities.EditContactActivity.hasContactChanged(EditContactActivity.kt:367)
        at org.fossify.contacts.activities.EditContactActivity.onBackPressed(EditContactActivity.kt:299)
        at android.app.Activity.onKeyUp(Activity.java:4046)
        at android.view.KeyEvent.dispatch(KeyEvent.java:2931)
        at android.app.Activity.dispatchKeyEvent(Activity.java:4421)
    ...
    
  • Not sure if intended but when I enable "Structured addresses (edit mode)" and disable the Addresses field, structured addresses fields are not shown.

naveensingh avatar Oct 25 '24 21:10 naveensingh

* When the address fields are empty and I disable structured addresses, the structured address fields are still visible.

* When the address fields are filled and I disable structured addresses, the app crashes:
* When I enable/disable structured addresses and press back, the app crashes.

The same issues are in the app of @stephanritscher . I never came across these crashes because I edited the visible fields only in the app settings. A reasonable solution could be to remove these settings from the edit dialog.

* Not sure if intended but when I enable "Structured addresses (edit mode)" and disable the `Addresses` field, structured addresses fields are not shown.

This makes sense to me. If I don't want to see the addresses it doesn't matter what the setting for structured addresses is. Structured addresses is kind of a sub-setting of addresses. Maybe the UI could reflect that.

Actually a new feature but related to this one: Don't disable the not visible fields completely but hide them behind a "hidden fields" button at the bottom like the "more fields" button in LineageOS contacts app.

bege10 avatar Oct 26 '24 09:10 bege10

@naveensingh @bege10 Thanks a lot for testing. I didn't come across crashes for a while which shows you know the corner cases of the app much better than me. I will have a look and try to fix the issues.

stephanritscher avatar Oct 26 '24 19:10 stephanritscher

Hi,

I think I fixed the issues. I also rebased to fossify/master and updated stephanritscher/FossifyCommons accordingly.

One thing I noticed when testing is that editing a contact present on multiple accounts with merge option enabled behaves differently (and worse in my setup) than SimpleContacts did. Previously it opened the "main" account (probably the one providing most fields or similar), now FossifyContacts seems to open always a "side" account on my device with barely any contact data - might be the first account, but not lexicographically). Before noticing a different account was opened, I thought I introduced a bug, but I verified the behaviour with the app you published in F-Droid. Was there an intentional change related to this?

Best regards, Stephan

stephanritscher avatar Oct 28 '24 20:10 stephanritscher

Might the "main account" be the "standard account for new contacts" (translated from German) as set in the settings of the AOSP/LineageOS contacts app? I don't find this setting this contacts app.

bege10 avatar Oct 29 '24 08:10 bege10

You may have seen this already, but I was looking through the RFC for this partly related feature request, and found that there is mention of address components in the May 2024 extension to the RFC

This specification modifies the definition of the ADR property. It extends its structured value with additional address components to better support the variety of international addresses. It separates the address parts, which currently are typically combined in street address component values, into distinct components.

Perhaps it may be relevant. And, thank you for doing this @stephanritscher . I use Nextcloud too, and had been using your app.

DiagonalArg avatar Nov 17 '24 06:11 DiagonalArg

Note also the related request for structured name fields, which include standard properties ORG and TITLE (which Nextcloud uses), along with NICKNAME, and various extensions as I've posted in #193 , #194 and another poster in #168

DiagonalArg avatar Nov 29 '24 12:11 DiagonalArg

@naveensingh @stephanritscher Hi Guys - maybe I lost the overview - but any progress here? I hesitate switch over to Fossify Contacts (even I think it would be wise to join the "Main Development Stream" again until the changes @stephanritscher made for Simple Contacts Pro SE made it to Fossify Contacts. Or is this done already and I missed it above somewhere?

lintux avatar Feb 04 '25 13:02 lintux

It will be available in the next update of Fossify Contacts.

naveensingh avatar Feb 04 '25 13:02 naveensingh