Avoid mutating EuiFieldPassword's append prop when in dual mode (#3884)

* Avoid mutating EuiFieldPassword's append prop when in dual mode * changelog
elastic · Aug 18, 2020 · 240bdbf · 240bdbf
1 parent e91e601
commit 240bdbf
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 - Fixed `EuiFacetGroup` container expansion due to negative margin value ([#3871](https://github.com/elastic/eui/pull/3871))
 - Fixed `EuiComboBox` delimeter-separated option creation and empty state prompt text ([#3841](https://github.com/elastic/eui/pull/3841))
 - Fixed `EuiDataGrid` not properly resizing within a fixed height container ([#3894](https://github.com/elastic/eui/pull/3894))
+- Fixed bug in `EuiFieldPassword` where an edge case mutated its `append` prop  ([#3884](https://github.com/elastic/eui/pull/3884))
 
 **Breaking changes**
 

diff --git a/src/components/form/field_password/__snapshots__/field_password.test.tsx.snap b/src/components/form/field_password/__snapshots__/field_password.test.tsx.snap
@@ -66,7 +66,7 @@ exports[`EuiFieldPassword props compressed is rendered 1`] = `
 </div>
 `;
 
-exports[`EuiFieldPassword props dual type also renders append 1`] = `
+exports[`EuiFieldPassword props dual dual type also renders append 1`] = `
 <div
   class="euiFormControlLayout euiFormControlLayout--group"
 >
@@ -118,7 +118,7 @@ exports[`EuiFieldPassword props dual type also renders append 1`] = `
 </div>
 `;
 
-exports[`EuiFieldPassword props dualToggleProps is rendered 1`] = `
+exports[`EuiFieldPassword props dual dualToggleProps is rendered 1`] = `
 <div
   class="euiFormControlLayout euiFormControlLayout--group"
 >

diff --git a/src/components/form/field_password/field_password.test.tsx b/src/components/form/field_password/field_password.test.tsx
@@ -18,7 +18,7 @@
  */
 
 import React from 'react';
-import { render } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { requiredProps } from '../../../test/required_props';
 
 import { EuiFieldPassword, EuiFieldPasswordProps } from './field_password';
@@ -92,20 +92,58 @@ describe('EuiFieldPassword', () => {
       });
     });
 
-    test('dualToggleProps is rendered', () => {
-      const component = render(
-        <EuiFieldPassword type="dual" dualToggleProps={requiredProps} />
-      );
+    describe('dual', () => {
+      test('dualToggleProps is rendered', () => {
+        const component = render(
+          <EuiFieldPassword type="dual" dualToggleProps={requiredProps} />
+        );
 
-      expect(component).toMatchSnapshot();
-    });
+        expect(component).toMatchSnapshot();
+      });
 
-    test('dual type also renders append', () => {
-      const component = render(
-        <EuiFieldPassword type="dual" append={['String', <span>Span</span>]} />
-      );
+      test('dual type also renders append', () => {
+        const component = render(
+          <EuiFieldPassword
+            type="dual"
+            append={['String', <span>Span</span>]}
+          />
+        );
 
-      expect(component).toMatchSnapshot();
+        expect(component).toMatchSnapshot();
+      });
+
+      test('dual does not mutate the append array prop', () => {
+        const props: EuiFieldPasswordProps = {
+          type: 'dual',
+          append: ['one', 'two'],
+          dualToggleProps: {
+            'data-test-subj': 'toggleButton',
+          },
+        };
+        const component = mount(<EuiFieldPassword {...props} />);
+
+        expect(
+          component.find('button[data-test-subj="toggleButton"]').length
+        ).toBe(1);
+        expect(
+          component
+            .find('button[data-test-subj="toggleButton"] EuiIcon')
+            .props().type
+        ).toBe('eye');
+
+        component
+          .find('button[data-test-subj="toggleButton"]')
+          .simulate('click');
+
+        expect(
+          component.find('button[data-test-subj="toggleButton"]').length
+        ).toBe(1);
+        expect(
+          component
+            .find('button[data-test-subj="toggleButton"] EuiIcon')
+            .props().type
+        ).toBe('eyeClosed');
+      });
     });
   });
 });
diff --git a/src/components/form/field_password/field_password.tsx b/src/components/form/field_password/field_password.tsx
@@ -113,7 +113,7 @@ export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
 
   // Convert any `append` elements to an array so the visibility
   // toggle can be added to it
-  const appends = Array.isArray(append) ? append : [];
+  let appends = Array.isArray(append) ? append : [];
   if (append && !Array.isArray(append)) appends.push(append);
   // Add a toggling button to switch between `password` and `input` if consumer wants `dual`
   // https://www.w3schools.com/howto/howto_js_toggle_password.asp
@@ -130,7 +130,7 @@ export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
         disabled={rest.disabled}
       />
     );
-    appends.push(visibilityToggle);
+    appends = [...appends, visibilityToggle];
   }
 
   const finalAppend = appends.length ? appends : undefined;