Skip to content

Move Address Element autocomplete state & navigation handling into separate element. #11035

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Move Address Element autocomplete state & navigation handling into separate element. #11035

wants to merge 1 commit into from
+423 −445

Conversation

samer-stripe
Copy link
Collaborator

Summary

Move Address Element autocomplete state & navigation handling into separate element.

Motivation

Allows for autocomplete logic to be shared in MPE.

Testing

  • Added tests
  • Modified tests
  • Manually verified

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 30, 2025 •
edited
Loading

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   2.1 MiB │   2.1 MiB │  0 B │   4.3 MiB │   4.3 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 302.9 KiB │ 302.9 KiB │  0 B │   457 KiB │   457 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.7 KiB │   7.7 KiB │  0 B │   7.4 KiB │   7.4 KiB │  0 B 
    other │  95.8 KiB │  95.8 KiB │ +3 B │ 183.5 KiB │ 183.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.8 MiB │   9.8 MiB │ +3 B │  21.8 MiB │  21.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20682 │ 20682 │ 0 (+0 -0) 
   types │  6503 │  6503 │ 0 (+0 -0) 
 classes │  5266 │  5266 │ 0 (+0 -0) 
 methods │ 31511 │ 31511 │ 0 (+0 -0) 
  fields │ 18239 │ 18239 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3646 │ 3646 │  0
APK 
   compressed    │  uncompressed   │                     
──────────┬──────┼──────────┬──────┤                     
 size     │ diff │ size     │ diff │ path                
──────────┼──────┼──────────┼──────┼─────────────────────
 29.2 KiB │ +4 B │ 64.6 KiB │  0 B │ ∆ META-INF/CERT.SF  
  1.2 KiB │ -1 B │  1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA 
──────────┼──────┼──────────┼──────┼─────────────────────
 30.4 KiB │ +3 B │ 65.8 KiB │  0 B │ (total)

@samer-stripe samer-stripe force-pushed the samer/isolate-autocomplete-components branch from c7f966a to b06b6d5 Compare June 30, 2025 14:19
Base automatically changed from samer/isolate-autocomplete-components to master June 30, 2025 15:14
@samer-stripe samer-stripe force-pushed the samer/move-ae-logic-into-separate-element branch from 4522dd3 to baa004b Compare June 30, 2025 15:16
samer-stripe
samer-stripe commented Jun 30, 2025
View reviewed changes
...tsheet/src/main/java/com/stripe/android/paymentsheet/addresselement/AddressFormController.kt
.filter {
it.value.isComplete
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleted FormController and replaced with a simple controller/holder for Address Element functionality. We can probably move this into InputAddressViewModel but also can be kept and tested here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This won't be re-used for MPE.

samer-stripe
samer-stripe commented Jun 30, 2025
View reviewed changes
...pe-ui-core/src/main/java/com/stripe/android/uicore/elements/AutocompleteAddressInteractor.kt
fun interface Factory {
fun create(): AutocompleteAddressInteractor
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The primary interactor for managing autocomplete state. This will be used to navigate to autocomplete and respond to its result. MPE will have a separate implementation.

samer-stripe
samer-stripe commented Jun 30, 2025
View reviewed changes
...pe-ui-core/src/main/java/com/stripe/android/uicore/elements/AutocompleteAddressController.kt
scope = interactor.interactorScope,
started = SharingStarted.Lazily,
initialValue = createAddressElement(addressType.value, state.value),
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Highlighting this comment as to why I'm choosing to use stateIn for this element.

samer-stripe
samer-stripe commented Jun 30, 2025
View reviewed changes
...pe-ui-core/src/main/java/com/stripe/android/uicore/elements/AutocompleteAddressController.kt
modifier = modifier,
)
}
}
Copy link
Collaborator Author

@samer-stripe samer-stripe Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe this is better secluded inside of the AddressElement component? My worry at the moment with secluding it in AddressElement is the complexity that will be added to it given how complex AddressElement already is.

tillh-stripe
tillh-stripe reviewed Jun 30, 2025
View reviewed changes
...et/src/test/java/com/stripe/android/paymentsheet/addresselement/AddressFormControllerTest.kt
getSectionFieldTextControllerWithLabel(addressFormController, UiCoreR.string.stripe_email)

nameElement?.onValueChange("joe")
assertThat(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's test this with Turbine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! This tests were from the original FormController. These all need to be modified.

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

@tillh-stripe tillh-stripe tillh-stripe left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samer-stripe @tillh-stripe