-
Notifications
You must be signed in to change notification settings - Fork 130
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
[Custom Fields] Some fixes #12582
[Custom Fields] Some fixes #12582
Conversation
Before this, clicks were not registered when done on the field value text
Generated by 🚫 Danger |
📲 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.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## issue/12578-copy-field #12582 +/- ##
============================================================
+ Coverage 40.64% 40.65% +0.01%
- Complexity 5670 5671 +1
============================================================
Files 1228 1228
Lines 68938 68947 +9
Branches 9547 9550 +3
============================================================
+ Hits 28019 28032 +13
+ Misses 38345 38343 -2
+ Partials 2574 2572 -2 ☔ View full report in Codecov by Sentry. |
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.
Good job @hichamboushaba 👍🏼
While all the test worked as expected I did notice an scenario that it is not handled. It is is possible to add multiple new custom fields with the same key value. It is not until I click save that duplicates are handled. However, the resulting behavior is a bit weird from a users perspective. I think we should avoid being able to add new custom fields with the same key, just as you added a check to avoid editing an existing key to have the same value of other custom fields.
allowsToAddDuplicatesAndThenMerges.mp4
Feel free to address this change in a different PR. Won't block this one for it.
Great find @JorgeMucientes, I'll address this later, and I'll share it with the iOS team, as for now we are checking against the saved items, and we don't check against the items that are not saved yet. |
Description
This PR fixes some issues with the Custom Fields feature:
948e8ee
0c8557f
Steps to reproduce
Testing information
TC1:
TC2:
TC3:
The tests that have been performed
The above.
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: