Refs #33874 - improve password field + use PF4 (#9816)

* Refs #33874 - improve password field + use PF4 * Refs #33874 - set password value prop * Refs #33874 - only allowing showing passwords for passwords * Refs #33874 - review feedback * Refs #33874 - review feedback * Refs #33874 - form validation * Refs #33874 - fix validation when there's an existing password * Refs #33874 - fix validation when clearing existing password
Katello · Dec 2, 2021 · c19f7b4 · c19f7b4
1 parent de3dfa7
commit c19f7b4
Show file tree

Hide file tree

Showing 8 changed files with 532 additions and 300 deletions.
diff --git a/package.json b/package.json
@@ -28,6 +28,7 @@
     "@sheerun/mutationobserver-shim": "^0.3.3",
     "@testing-library/jest-dom": "^5.3.0",
     "@testing-library/react": "^10.0.2",
+    "@testing-library/user-event": "^13.5.0",
     "@theforeman/builder": ">= 6.0.0",
     "@theforeman/find-foreman": "^4.8.0",
     "axios-mock-adapter": "^1.10.0",

diff --git a/webpack/components/EditableTextInput/EditableTextInput.js b/webpack/components/EditableTextInput/EditableTextInput.js
@@ -1,20 +1,30 @@
 import React, { useState, useEffect } from 'react';
 import { TextInput, TextArea, Text, TextVariants, Button, Split, SplitItem } from '@patternfly/react-core';
-import { TimesIcon, CheckIcon, PencilAltIcon } from '@patternfly/react-icons';
+import {
+  EyeIcon,
+  EyeSlashIcon,
+  TimesIcon,
+  CheckIcon,
+  PencilAltIcon,
+} from '@patternfly/react-icons';
 import { translate as __ } from 'foremanReact/common/I18n';
 import PropTypes from 'prop-types';
 import Loading from '../Loading';
 import './editableTextInput.scss';
 
+const PASSWORD_MASK = '••••••••';
+
 const EditableTextInput = ({
-  onEdit, value, textArea, attribute, placeholder,
+  onEdit, value, textArea, attribute, placeholder, isPassword, hasPassword,
   component, currentAttribute, setCurrentAttribute, disabled,
 }) => {
   // Tracks input box state
   const [inputValue, setInputValue] = useState(value);
   const [editing, setEditing] = useState(false);
   const [submitting, setSubmitting] = useState(false);
-
+  const [passwordPlaceholder, setPasswordPlaceholder] = useState(hasPassword
+    ? PASSWORD_MASK : null);
+  const [showPassword, setShowPassword] = useState(false);
 
   useEffect(() => {
     if (setCurrentAttribute && currentAttribute) {
@@ -27,6 +37,12 @@ const EditableTextInput = ({
   const onEditClick = () => {
     setEditing(true);
     if (setCurrentAttribute) setCurrentAttribute(attribute);
+
+    if (isPassword) {
+      if (passwordPlaceholder) {
+        setPasswordPlaceholder(null);
+      }
+    }
   };
 
   // Setting didCancel to prevent actions from happening after component has been unmounted
@@ -37,9 +53,16 @@ const EditableTextInput = ({
     const onSubmit = async () => {
       if (submitting) { // no dependency array because this check takes care of it
         await onEdit(inputValue, attribute);
+
         if (!didCancel) {
           setSubmitting(false);
           setEditing(false);
+
+          if (isPassword) {
+            if (inputValue?.length > 0) {
+              setPasswordPlaceholder(PASSWORD_MASK);
+            }
+          }
         }
       }
     };
@@ -64,10 +87,20 @@ const EditableTextInput = ({
   }, [editing]);
 
   const onClear = () => {
+    if (isPassword) {
+      if (hasPassword || inputValue?.length > 0) {
+        setPasswordPlaceholder(PASSWORD_MASK);
+      }
+    }
+
     setInputValue(value);
     setEditing(false);
   };
 
+  const toggleShowPassword = () => {
+    setShowPassword(prevShowPassword => !prevShowPassword);
+  };
+
   const inputProps = {
     component,
     value: inputValue || '',
@@ -81,7 +114,7 @@ const EditableTextInput = ({
         <SplitItem>
           {textArea ?
             (<TextArea {...inputProps} aria-label={`${attribute} text area`} />) :
-            (<TextInput {...inputProps} type="text" aria-label={`${attribute} text input`} />)}
+            (<TextInput {...inputProps} type={(isPassword && !showPassword) ? 'password' : 'text'} aria-label={`${attribute} text input`} />)}
         </SplitItem>
         <SplitItem>
           <Button
@@ -97,14 +130,24 @@ const EditableTextInput = ({
             <TimesIcon />
           </Button>
         </SplitItem>
+        { isPassword ?
+          <SplitItem>
+            <Button aria-label={`show-password ${attribute}`} variant="plain" isDisabled={!inputValue?.length} onClick={toggleShowPassword}>
+              { showPassword ?
+                (<EyeSlashIcon />) :
+                (<EyeIcon />)}
+            </Button>
+          </SplitItem> :
+          null
+        }
       </Split>
     );
   }
   return (
     <Split>
       <SplitItem>
         <Text aria-label={`${attribute} text value`} component={component || TextVariants.p}>
-          {value || (<i>{placeholder}</i>)}
+          {passwordPlaceholder || inputValue || (<i>{placeholder}</i>)}
         </Text>
       </SplitItem>
       {!disabled &&
@@ -133,6 +176,8 @@ EditableTextInput.propTypes = {
   currentAttribute: PropTypes.string,
   setCurrentAttribute: PropTypes.func,
   disabled: PropTypes.bool,
+  isPassword: PropTypes.bool,
+  hasPassword: PropTypes.bool,
 };
 
 EditableTextInput.defaultProps = {
@@ -143,6 +188,8 @@ EditableTextInput.defaultProps = {
   currentAttribute: undefined,
   setCurrentAttribute: undefined,
   disabled: false,
+  isPassword: false,
+  hasPassword: false,
 };
 
 export default EditableTextInput;
diff --git a/webpack/components/EditableTextInput/__tests__/editableTextInput.test.js b/webpack/components/EditableTextInput/__tests__/editableTextInput.test.js
@@ -50,3 +50,85 @@ test('input is set back to original value after clearing', () => {
   // Original value is still showing even though it's been edited
   expect(getByLabelText(`${attribute} text value`)).toHaveTextContent(actualValue);
 });
+
+test('shows a mask over the password when there is one', () => {
+  const { getByLabelText } = render(<EditableTextInput
+    attribute={attribute}
+    onEdit={jest.fn()}
+    isPassword
+    hasPassword
+  />);
+
+  expect(getByLabelText(`${attribute} text value`)).toHaveTextContent('••••••••');
+});
+
+test('shows a mask over the password after undoing changes', () => {
+  const { getByLabelText } = render(<EditableTextInput
+    attribute={attribute}
+    onEdit={jest.fn()}
+    isPassword
+    hasPassword
+  />);
+
+  getByLabelText(`edit ${attribute}`).click();
+  expect(getByLabelText(`${attribute} text input`)).toHaveTextContent('');
+
+  getByLabelText(`clear ${attribute}`).click();
+  expect(getByLabelText(`${attribute} text value`)).toHaveTextContent('••••••••');
+});
+
+test('shows a mask over the password after editing', async () => {
+  const newPassword = 'Pizza';
+  const { getByLabelText } = render(<EditableTextInput
+    attribute={attribute}
+    onEdit={jest.fn()}
+    isPassword
+    hasPassword
+  />);
+
+  getByLabelText(`edit ${attribute}`).click();
+  fireEvent.change(getByLabelText(`${attribute} text input`), { target: { value: newPassword } });
+  expect(getByLabelText(`${attribute} text input`)).toHaveValue(newPassword);
+  getByLabelText(`submit ${attribute}`).click();
+
+  await patientlyWaitFor(() => expect(getByLabelText(`${attribute} text value`)).toBeInTheDocument());
+  expect(getByLabelText(`${attribute} text value`)).toHaveTextContent('••••••••');
+});
+
+test('shows a placeholder after clearing the password', async () => {
+  const { getByLabelText } = render(<EditableTextInput
+    attribute={attribute}
+    onEdit={jest.fn()}
+    isPassword
+    hasPassword
+  />);
+
+  getByLabelText(`edit ${attribute}`).click();
+  getByLabelText(`submit ${attribute}`).click();
+
+  await patientlyWaitFor(() => expect(getByLabelText(`${attribute} text value`)).toBeInTheDocument());
+  expect(getByLabelText(`${attribute} text value`)).toHaveTextContent('None provided');
+});
+
+test('can toggle showing the current password', async () => {
+  const { getByLabelText } = render(<EditableTextInput
+    attribute={attribute}
+    onEdit={jest.fn()}
+    isPassword
+    hasPassword
+  />);
+
+  getByLabelText(`edit ${attribute}`).click();
+
+  expect(getByLabelText(`show-password ${attribute}`)).toHaveAttribute('disabled', '');
+
+  const newPassword = 'New Password';
+  fireEvent.change(getByLabelText(`${attribute} text input`), { target: { value: newPassword } });
+  expect(getByLabelText(`${attribute} text input`)).toHaveAttribute('type', 'password');
+
+  getByLabelText(`show-password ${attribute}`).click();
+  expect(getByLabelText(`${attribute} text input`)).toHaveAttribute('type', 'text');
+
+  getByLabelText(`show-password ${attribute}`).click();
+  expect(getByLabelText(`${attribute} text input`)).toHaveAttribute('type', 'password');
+});