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

Networking Sign Up Pane (Native): Add back auto-focus #8557

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

Conversation

carlosmuvi-stripe
Copy link
Collaborator

@carlosmuvi-stripe carlosmuvi-stripe commented May 30, 2024

Summary

  • Replaces activity resizing by insets (keyboard now smoothly animates)
  • On signup pane, keeps bottom panel behind keyboard
  • Focuses on email when empty
  • Focuses on phone when email prefilled, but no phone prefilled
  • Does not focus when both fields prefill.
no_prefill.mp4
email_prefill.mp4
both_filled.mp4

Motivation

📔  Networking Sign Up Pane (Native): Add back auto-focus
🌐  BANKCON-10945

On networking sign up pane:

  1. if user has no email, we will auto-focus the keyboard on email and the "Not Now" button will be behind the keyboard (user can still easily dismiss keyboard or press the "X" button)
  2. if user has no phone (but prefill email), we will auto-focus the keyboard on phone (other details apply as above ^)
  3. if we prefill both, we don't auto-focus

PR's that removed it:

  1. email: FC Networking: disabled autofocus on email field on SignUp pane. stripe-ios#2526
  2. phone: FC Networking: disabled focusing phone field IF the email has been prefilled stripe-ios#2528

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

@carlosmuvi-stripe carlosmuvi-stripe added the work-cli Added to pull requests created with #work-cli for usage tracking. label May 30, 2024
@carlosmuvi-stripe carlosmuvi-stripe self-assigned this May 30, 2024
Copy link
Contributor

github-actions bot commented May 30, 2024

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 MiB │     2 MiB │ -1 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.1 KiB │   8.1 KiB │  0 B 
      res │ 301.5 KiB │ 301.5 KiB │  0 B │   455 KiB │   455 KiB │  0 B 
   native │   7.3 MiB │   7.3 MiB │  0 B │  18.4 MiB │  18.4 MiB │  0 B 
    asset │   1.5 MiB │   1.5 MiB │  0 B │   1.5 MiB │   1.5 MiB │  0 B 
    other │    87 KiB │    87 KiB │ +8 B │ 161.5 KiB │ 161.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │  12.2 MiB │  12.2 MiB │ +7 B │  25.8 MiB │  25.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 21689 │ 21689 │ 0 (+1 -1) 
   types │  6869 │  6869 │ 0 (+0 -0) 
 classes │  5634 │  5634 │ 0 (+0 -0) 
 methods │ 31451 │ 31451 │ 0 (+0 -0) 
  fields │ 18315 │ 18315 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3404 │ 3404 │  0
APK
   compressed    │  uncompressed   │                                           
──────────┬──────┼──────────┬──────┤                                           
 size     │ diff │ size     │ diff │ path                                      
──────────┼──────┼──────────┼──────┼───────────────────────────────────────────
 29.1 KiB │ +7 B │   64 KiB │  0 B │ ∆ META-INF/CERT.SF                        
    271 B │ +1 B │    120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
    2 MiB │ -1 B │  4.3 MiB │  0 B │ ∆ classes.dex                             
  1.2 KiB │ +1 B │  1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA                       
 25.9 KiB │ -1 B │ 63.9 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
──────────┼──────┼──────────┼──────┼───────────────────────────────────────────
  2.1 MiB │ +7 B │  4.4 MiB │  0 B │ (total)
DEX
STRINGS:

   old   │ new   │ diff      
  ───────┼───────┼───────────
   21689 │ 21689 │ 0 (+1 -1) 
  
  + ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:eccad00,r8-mode:full,version:8.3.37}
  
  - ~~R8{backend:dex,compilation-mode:release,has-checksums:false,min-api:21,pg-map-id:0f80f6e,r8-mode:full,version:8.3.37}

Comment on lines +60 to +61
modifier = modifier
.imePadding(),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hoisted imePadding a bit up so that users can customize it, and adds it for the Elements bottom sheet to keep current behavior.

@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review June 4, 2024 17:52
@carlosmuvi-stripe carlosmuvi-stripe requested review from a team as code owners June 4, 2024 17:52
@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as draft June 4, 2024 17:53
…muvi/BANKCON-10945/networking-sign-up-pane-native-add-back-auto-focus
…muvi/BANKCON-10945/networking-sign-up-pane-native-add-back-auto-focus
@carlosmuvi-stripe carlosmuvi-stripe marked this pull request as ready for review June 4, 2024 18:48
Copy link
Collaborator

@tillh-stripe tillh-stripe left a comment

Choose a reason for hiding this comment

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

Looks good!


// When pane loads, focus on email field if it's empty
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I don’t think we need these comments, as the code already explains it :)

internal inline fun Modifier.conditional(
condition: Boolean,
ifTrue: Modifier.() -> Modifier,
ifFalse: Modifier.() -> Modifier = { this },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: We don’t seem to use ifFalse. Should we rename this to fun Modifier.ifTrue(condition, action)?

@@ -47,6 +49,7 @@ import com.stripe.android.financialconnections.ui.theme.FinancialConnectionsThem
* @param inModal whether the layout is being used in a modal or not. If true, the [body] won't expand to fill the
* available content.
* @param showFooterShadowWhenScrollable whether to show a shadow at the top of the footer when the body is scrollable.
* @param applyImePadding whether to apply IME padding. When false, footer will be covered by the keyboard.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Should we rename this to keepFooterAboveKeyboard or something more concrete than applyImePadding?

emailFocusRequester.requestFocus()
}
}
// Everytime full form is shown, scroll to bottom and focus on phone number field (if not empty)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-cli Added to pull requests created with #work-cli for usage tracking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants