Skip to content

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Nov 2, 2022

Part of #7891
Based on #7995 just for easier testing

Description

To follow the design for the password field, this PR added an πŸ‘οΈ button to AccountCreationFormFieldView when the view model's isSecure is true. Tapping on the button shows/hides the secure input. Because of the rounded border design, the reveal button has to be added using a ZStack with some custom insets to the text field style and button image size using @ScaledMetric for dynamic type.

One minor issue I noticed is the slight height change between the shown/hidden state. Please feel free to share any suggestions to fix it!

Testing instructions

  • Launch the app
  • Log out and skip onboarding if needed
  • Tap Create a Store --> there should be an πŸ‘οΈ/ icon with a slash in the password field
  • Enter some text into the password field
  • Tap on the πŸ‘οΈ / icon --> the password input should be shown, and the icon becomes πŸ‘οΈ without a slash

Screenshots

secure input hidden secure input shown
Simulator Screen Shot - iPhone 14 - 2022-11-02 at 09 48 59 Simulator Screen Shot - iPhone 14 - 2022-11-02 at 09 49 14

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync added the type: enhancement A request for an enhancement. label Nov 2, 2022
@jaclync jaclync added this to the 11.1 milestone Nov 2, 2022
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 2, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8006-a09e7e3 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@itsmeichigo itsmeichigo self-assigned this Nov 2, 2022
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

Looks great! πŸŽ‰ I have a suggestion to fix the height change in the comment below.

Also - do you think we should extract this ZStack into a reusable view, in case we need it for a native login screen? It's something for the future, so we can do that later too.

func _body(configuration: TextField<Self._Label>) -> some View {
configuration
.padding(10)
.padding(insets)
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the change of height when switching between field types is due to vertical padding and the content height difference of SecureField and TextField.

A quick fix would be to add a fixed height, we may need to take scale into consideration:

Suggested change
.padding(insets)
.padding(insets)
.frame(height: 44 * scale)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some testing, the insets aren't the cause of the mismatched height, the height is just slightly different between the TextField and SecureField. In a09e7e3, I passed an optional height parameter for AccountCreationFormFieldView to set 44 * scale.

…sword-input

* feat/7891-design-polishes:
  Make focused/unfocused border color configurable with previews.

# Conflicts:
#	WooCommerce/Classes/ViewRelated/ReusableViews/SwiftUI Components/TextFieldStyles.swift
Base automatically changed from feat/7891-design-polishes to trunk November 2, 2022 04:29
…that the password field has the same height when the secure content is shown and hidden.
@jaclync
Copy link
Contributor Author

jaclync commented Nov 2, 2022

Also - do you think we should extract this ZStack into a reusable view, in case we need it for a native login screen? It's something for the future, so we can do that later too.

When we support login in the future, I was thinking we probably want to use the SwiftUI field view for both login and signup. So maybe we can just rename AccountCreationFormFieldView to like AccountFormFieldView or AuthFormFieldView?

@itsmeichigo
Copy link
Contributor

itsmeichigo commented Nov 2, 2022

So maybe we can just rename AccountCreationFormFieldView to like AccountFormFieldView or AuthFormFieldView?

My suggestion was to create a generic RoundedBorderTextField with the option to be secure (with the πŸ‘οΈ button) or not; not the whole form view πŸ˜… But please feel free to leave that for later if necessary.

@jaclync
Copy link
Contributor Author

jaclync commented Nov 2, 2022

My suggestion was to create a generic RoundedBorderTextField with the option to be secure (with the πŸ‘οΈ button) or not; not the whole form view πŸ˜… But please feel free to leave that for later if necessary.

I feel like the secure field is mostly useful during authentication πŸ€” Let's go with the PR and we can create it when it's needed for another use case.

@jaclync jaclync merged commit c85406c into trunk Nov 2, 2022
@jaclync jaclync deleted the feat/7891-show-hide-password-input branch November 2, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement A request for an enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants