-
Notifications
You must be signed in to change notification settings - Fork 135
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
[Mobile Payments] Remove duplicate back button from the Try Tap to Pay customer provided note screen #12787
Conversation
…ar and menu options
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving that @AnirudhBhat! I added a small suggestion—let me know what you think.
import org.wordpress.android.util.ActivityUtils | ||
import org.wordpress.android.util.DisplayUtils | ||
|
||
class SimplePaymentsCustomerNoteFragment : | ||
BaseFragment(R.layout.fragment_order_create_edit_customer_note), | ||
MenuProvider { | ||
BaseFragment(R.layout.fragment_order_create_edit_customer_note) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 I think we are not using any functionality provided by BaseFragment, so I suggest switching to the plain Fragment
class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using override val activityAppBarStatus: AppBarStatus get() = AppBarStatus.Hidden
from BaseFragment
to hide the default toolbar and use our own. I think using plain Fragment would again cause multiple toolbar (one from our own custom toolbar and another that exist from previous screen)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, you're right 👍🏼
Closes: #12778
Description
This PR removes duplicate back button that appeared on the Try Tap to Pay customer provided note screen.
Steps to reproduce
The tests that have been performed
Tested on both dark and light mode, tested adding notes, removing notes.
Images/gif
duplicate_back_button_ttp_note.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.Reviewer (or Author, in the case of optional code reviews):
Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement: