Compose smart button#1531
Conversation
…fully. The flow is broken.
Works end to end and shows a successful response.
|
I tested this on my device and the flow works correctly, the only two things i noticed:
PS: Update this has been resolved. |
Restrict view model and repository to library group. Make sure that handle return is called only after flow is launched.
|
i also wondering why the Instrumentation Tests are failing, I havent see this error before |
Browser Switch 3.5.0 hasn't been published yet. I was using a 3.4.1-SNAPSHOT and didn't want to forget to change it. So, I made the change to 3.5.0. It is expected to fail until browser switch is published. |
|
ahhh totally makes sense, thank you! |
| } | ||
| } | ||
|
|
||
| suspend fun getPendingRequest(): String? { |
There was a problem hiding this comment.
Should we return a non null string here or throw an exception if the dataStore does not contain a pendingnRequest?
There was a problem hiding this comment.
I handle an empty pendingRequest here: https://github.com/braintree/braintree_android/pull/1531/changes#diff-23d7463af226bd84a2ce92dfa837cbda03e749c683c45da489b4439978fb988aR161
Non-null would be good, but I think I'd just have to return an empty string at the data store level and rest of the code would remain relatively the same. A few return types can be made non-null.
There was a problem hiding this comment.
Not sure if we can throw an exception. We could log the exception and return a failure to the merchant.
There was a problem hiding this comment.
I think the preferred error handling pattern is to throw exceptions and catch them at some level. I think at the ViewModel we could have a try/catch, then the view state could emit the error to the view.
There was a problem hiding this comment.
I skipped the view model and am using the repository directly in the composable.
There was a problem hiding this comment.
I am of the opinion that the repository should be able to send out an empty string.
I'm handling the exception at the view level where we know the state of the flow and we expect this value to be not empty.
Would you agree with my reasoning?
saralvasquez
left a comment
There was a problem hiding this comment.
Looks great! Tested on physical device and emulator and both worked as expected. Just a quick question. Did we want to set up a similar kind of layout for the different button color options as we did for the XML based buttons page?
| @OptIn(ExperimentalCoroutinesApi::class) | ||
| class PayPalPendingRequestRepositoryTest { | ||
|
|
||
| @get:Rule |
There was a problem hiding this comment.
Cool! That makes everything so much cleaner
I have a different ticket to setup the demo app. This one is to make sure the flow works. |
daf8428
into
ui-components-compose-support
* Paypal buttons compose (#1528) * WIP: add dependencies and PayPalButton * paypal button * Add logo offset. * cleanup * cleanup * Working circular progress indicator * Add corner shape * WIP: interaction states * focus indicator * make linter gods happy * linter * linter * use colorResource * update focus padding to use a resource * remove spacers from preview * Compose smart button (#1531) * Compiling demo of PayPal smart button on Compose. Doesn't run successfully. The flow is broken. * Reorder buttons in demo app * refactor file name * fix button size * fix button size in demo * Skip modular and store pendingRequest state within composable. Works end to end and shows a successful response. * hoist style to parent composable * update demo app * remove unused classes * linter' * remember PayPalLauncher and PayPalClient beyond recomposition * Address PR comments * surface failure scenarios to host app * persist pending request to data store * linter * Cleanup. Use anticipated dependency version. * Use scope properly * linter fixes * Address PR comments. Restrict view model and repository to library group. Make sure that handle return is called only after flow is launched. * make function private * add tests * Add analytics events for paypal compose button flow * fix CI * fix test. * remove viewmodel to simplify flow * linter * add kdoc on composable * Address PR comments * add jvmoverloads * make repository return a non null value for pending request * Venmo compose button (#1541) * Compiling demo of PayPal smart button on Compose. Doesn't run successfully. The flow is broken. * Reorder buttons in demo app * refactor file name * fix button size * fix button size in demo * Skip modular and store pendingRequest state within composable. Works end to end and shows a successful response. * hoist style to parent composable * update demo app * remove unused classes * linter' * remember PayPalLauncher and PayPalClient beyond recomposition * Address PR comments * surface failure scenarios to host app * persist pending request to data store * linter * Cleanup. Use anticipated dependency version. * Use scope properly * linter fixes * Address PR comments. Restrict view model and repository to library group. Make sure that handle return is called only after flow is launched. * make function private * add tests * Add analytics events for paypal compose button flow * fix CI * fix test. * remove viewmodel to simplify flow * linter * add kdoc on composable * Address PR comments * Venmo buttons compose UI * Update analytics * Venmo compose button * update demo app * rename options * update checkout values * consolidate extension functions * refactoring * refactor * add kdoc * Update repository to take in a moduleName parameter and update tests * rename file * Add exception message * update venmo button content description * update to 3.5.1 of browser switch * rename buttons * update changelog * Making ActivityResultRegistry a required parameter for Venmo/PayPal Launcher constructors. This preserves backwards compatibility. * Renaming VenmoSmartButton to VenmoButton * Add Changelog entry * linter fix * Update UIComponents/src/main/java/com/braintreepayments/api/uicomponents/compose/PayPalButton.kt Co-authored-by: Sara Vasquez <98496950+saralvasquez@users.noreply.github.com> * Update UIComponents/src/main/java/com/braintreepayments/api/uicomponents/compose/VenmoButton.kt Co-authored-by: Sara Vasquez <98496950+saralvasquez@users.noreply.github.com> * update changelog.md * update demo app * change requireNotNull to if condition * Sends merchants a Failure result when compose is not started from a ComponentActivity * linter * address PR comments * enable button after a failure * enable button after a failure * fix critical issue where flow freezes when activity is null * remove setting intent to null, need documentation for merchant to that on their end --------- Co-authored-by: Sara Vasquez <98496950+saralvasquez@users.noreply.github.com>
Summary of changes
Testing:
Dependent on braintree/browser-switch-android#127 being merged in and released.Checklist
Authors