Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Bug 1942137: Add show/hide password feature via new ValidatedPasswordField component #524

Merged
5 commits merged into from
Apr 7, 2021

Conversation

mturley
Copy link
Collaborator

@mturley mturley commented Apr 7, 2021

Resolves #468 (https://bugzilla.redhat.com/show_bug.cgi?id=1942137)
Also resolves #452 in the interest of time, because separate PRs for these two fixes would conflict.

Adds a new common component ValidatedPasswordField which adds a show/hide icon button on the right side of the field. Replaces the plain password input with this component in 3 places: VMware provider password, OpenShift provider SA token, and ESXi host admin password.

The first commit puts the toggle icon next to the field label like it's done in MTC today, but then @vconzola pointed out that the PatternFly login page example has the icon in an InputGroup next to the input field (see patternfly/patternfly-react#5481), so the second commit switches to that design.

I had to duplicate some of the code from the lib-ui ValidatedTextInput component since it doesn't support InputGroups. We can add that feature to lib-ui and simplify this code in the future. Opened migtools/lib-ui#55 to track this.

xvCQrJKku7

@mturley mturley requested a review from a team April 7, 2021 19:42
@github-actions
Copy link

github-actions bot commented Apr 7, 2021

valid bug 1942137

@konveyor-preview-bot
Copy link

🚀 Deployed Preview: http://konveyor-forklift-ui-pr-524-preview.surge.sh

Compare with current main branch: http://konveyor-forklift-ui-preview.surge.sh

@codecov
Copy link

codecov bot commented Apr 7, 2021

Codecov Report

Merging #524 (f88dda4) into main (c2795b0) will increase coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #524      +/-   ##
==========================================
+ Coverage   59.70%   59.91%   +0.20%     
==========================================
  Files         133      134       +1     
  Lines        4301     4323      +22     
  Branches     1069     1076       +7     
==========================================
+ Hits         2568     2590      +22     
  Misses       1709     1709              
  Partials       24       24              
Impacted Files Coverage Δ
src/app/common/constants.ts 96.66% <ø> (ø)
...ents/AddEditProviderModal/AddEditProviderModal.tsx 87.50% <100.00%> (+0.19%) ⬆️
...ts/VMwareProviderHostsTable/SelectNetworkModal.tsx 29.78% <100.00%> (+1.52%) ⬆️
...c/app/common/components/ValidatedPasswordInput.tsx 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2795b0...f88dda4. Read the comment docs.

@mturley mturley marked this pull request as ready for review April 7, 2021 20:43
@ghost ghost merged commit d47a799 into kubev2v:main Apr 7, 2021
@mturley mturley deleted the 468-show-hide-passwords branch April 7, 2021 21:20
label="Password"
isRequired
fieldId="vmware-password"
/>
<ValidatedTextInput
field={vmwareForm.fields.fingerprint}
label="Certificate SHA1 Fingerprint"
label="SHA-1 Fingerprint"
Copy link

Choose a reason for hiding this comment

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

"Fingerprint" should be "fingerprint", per PF4 guidelines.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agh dammit, thanks @apinnick. Will fix in another PR.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants