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

[Payment Method Improvements] Create the "Mark Order as Complete" Button #11570

Conversation

backwardstruck
Copy link
Contributor

@backwardstruck backwardstruck commented May 22, 2024

Closes: #11525

Depends on this PR being merged first:
#11569

Description

Create the "Mark Order as Complete" Button in the Change Due Calculator screen and make sure to get keyboard focus for Cash Received field.

Testing instructions

  1. Enable OTHER_PAYMENT_METHODS
  2. Go to an order that needs to be paid
  3. Select "Cash"

Expected: You should see the Change Due screen with the num pad open and Cash Received in focus

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@backwardstruck backwardstruck added type: enhancement A request for an enhancement. feature: mobile payments Related to mobile payments / card present payments / Woo Payments. labels May 22, 2024
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented May 22, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commitf99dc1d
Direct Downloadwoocommerce-prototype-build-pr11570-f99dc1d.apk

…e-due-calculator-ui-part-4

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/changeduecalculator/ChangeDueCalculatorFragment.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/changeduecalculator/ChangeDueCalculatorScreen.kt
@backwardstruck backwardstruck marked this pull request as ready for review May 24, 2024 21:36
it.requestFocus()
context.getSystemService(
InputMethodManager::class.java
).showSoftInput(it, InputMethodManager.SHOW_IMPLICIT)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like this to work the same as CustomAmountsFragment and pop the num pad but for some reason I'm seeing the full keyboard.

Copy link
Contributor

@kidinov kidinov May 27, 2024

Choose a reason for hiding this comment

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

Have you considered using WCOutlinedTypedTextField the same as they do in OrderCreateEditProductDiscountScreen? There are quite a few advantages of not using "XML" views in compose, including that now preview doesn't work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great suggestion. Indeed it broke the preview and I would prefer to use proper Compose views. I will change this today in this draft PR:
#11587

@kidinov
Copy link
Contributor

kidinov commented May 27, 2024

@backwardstruck 👋

Feel free to fix that in the following PRs (or may it's already fixed, not sure):

  • It looks like it's not scrollable so a user can not reach the bottom of the screen if the keyboard is open
  • Background is not consistent with the rest of the screens
  • Should be the "collect button" at the bottom of the screen?
05-27--15-16.mp4

).showSoftInput(it, InputMethodManager.SHOW_IMPLICIT)
}
}

AndroidView(
Copy link
Contributor

Choose a reason for hiding this comment

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

In case you still would want to steak with AndroidView reuse here, I'd suggest to export that into a separate composable function for code simplification

Copy link
Contributor

@kidinov kidinov left a comment

Choose a reason for hiding this comment

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

I left a few comments - feel free to update the code in the following PRs

@backwardstruck
Copy link
Contributor Author

I left a few comments - feel free to update the code in the following PRs

Thanks a lot. I added those items to my ticket here:

#11583

@backwardstruck backwardstruck merged commit e4f14b7 into 11525-change-due-calculator-ui-part-3 May 27, 2024
16 of 17 checks passed
@backwardstruck backwardstruck deleted the 11525-change-due-calculator-ui-part-4 branch May 27, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: enhancement A request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Payment Method Improvements] Implement Change Due Calculator UI for Cash Payments
3 participants