-
Notifications
You must be signed in to change notification settings - Fork 115
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
[Shipping labels] Add WooShippingAddressField to represent an address field #14949
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y and state selection
Generated by 🚫 Danger |
|
4 tasks
toupper
approved these changes
Jan 23, 2025
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.
LGTM! 🚢
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of: #13781
Description
This PR does not change the current behavior, but it encapsulates the logic for an address field in the Woo Shipping flow into
WooShippingAddressField
. This new type holds the field's type, its value, whether it's required, the method for validating its value, and whether it has a validation error. (This requires quite a few lines of changes to support moving the logic to the new type.)This change simplifies how the view handles these fields: we can pass each field to its corresponding
AddressTextField
orAddressSelection
view and use the field to populate those child views.In the view model, each field is now a
WooShippingAddressField
(rather than aString
for the field value). The view model is now focused on defining the rules for validating each field and handling special cases where one field depends on another (e.g. name and company fields, or state and country fields).These changes will facilitate the validation UI behavior outlined in #14943 and hopefully make it easier to understand and maintain the logic around each field.
Testing information
For now, the validation logic isn't yet used in the UI. However, you can confirm that UI works the same as before:
Screenshots
RELEASE-NOTES.txt
if necessary.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: