[EuiButtonIcon] change iconType prop to required (elastic#4106)

kshitij86 · Nov 29, 2020 · 64c73ad · 64c73ad
1 parent d27ecaa
commit 64c73ad
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,7 @@
 **Bug fixes**
 
 - Fixed custom color render of `EuiIcon` app (two-tone) icons ([#4104](https://github.com/elastic/eui/pull/4104))
-
+- Changed `iconType` prop to be `required` in `EuiButtonIcon` ([#4106](https://github.com/elastic/eui/pull/4106))
 
 ## [`29.4.0`](https://github.com/elastic/eui/tree/v29.4.0)
 

diff --git a/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap b/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap
@@ -6,79 +6,139 @@ exports[`EuiButtonIcon is rendered 1`] = `
   class="euiButtonIcon euiButtonIcon--primary testClass1 testClass2"
   data-test-subj="test subject string"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color accent is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--accent"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color danger is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--danger"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color disabled is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--disabled"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color ghost is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--ghost"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color primary is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--primary"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color subdued is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--subdued"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color success is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--success"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color text is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--text"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props color warning is rendered 1`] = `
 <button
   aria-label="button"
   class="euiButtonIcon euiButtonIcon--warning"
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props href secures the rel attribute when the target is _blank 1`] = `
@@ -88,7 +148,13 @@ exports[`EuiButtonIcon props href secures the rel attribute when the target is _
   href="#"
   rel="noopener noreferrer"
   target="_blank"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</a>
 `;
 
 exports[`EuiButtonIcon props iconType is rendered 1`] = `
@@ -111,7 +177,13 @@ exports[`EuiButtonIcon props isDisabled is rendered 1`] = `
   class="euiButtonIcon euiButtonIcon--primary"
   disabled=""
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
 
 exports[`EuiButtonIcon props isDisabled renders a button even when href is defined 1`] = `
@@ -120,5 +192,11 @@ exports[`EuiButtonIcon props isDisabled renders a button even when href is defin
   class="euiButtonIcon euiButtonIcon--primary"
   disabled=""
   type="button"
-/>
+>
+  <span
+    aria-hidden="true"
+    class="euiButtonIcon__icon"
+    data-euiicon-type="user"
+  />
+</button>
 `;
diff --git a/src/components/button/button_icon/button_icon.test.tsx b/src/components/button/button_icon/button_icon.test.tsx
@@ -25,7 +25,9 @@ import { EuiButtonIcon, COLORS } from './button_icon';
 
 describe('EuiButtonIcon', () => {
   test('is rendered', () => {
-    const component = render(<EuiButtonIcon {...requiredProps} />);
+    const component = render(
+      <EuiButtonIcon iconType="user" {...requiredProps} />
+    );
 
     expect(component).toMatchSnapshot();
   });
@@ -34,15 +36,20 @@ describe('EuiButtonIcon', () => {
     describe('isDisabled', () => {
       it('is rendered', () => {
         const component = render(
-          <EuiButtonIcon aria-label="button" isDisabled />
+          <EuiButtonIcon iconType="user" aria-label="button" isDisabled />
         );
 
         expect(component).toMatchSnapshot();
       });
 
       it('renders a button even when href is defined', () => {
         const component = render(
-          <EuiButtonIcon aria-label="button" href="#" isDisabled />
+          <EuiButtonIcon
+            iconType="user"
+            aria-label="button"
+            href="#"
+            isDisabled
+          />
         );
 
         expect(component).toMatchSnapshot();
@@ -63,7 +70,7 @@ describe('EuiButtonIcon', () => {
       COLORS.forEach((color) => {
         test(`${color} is rendered`, () => {
           const component = render(
-            <EuiButtonIcon aria-label="button" color={color} />
+            <EuiButtonIcon iconType="user" aria-label="button" color={color} />
           );
 
           expect(component).toMatchSnapshot();
@@ -74,7 +81,12 @@ describe('EuiButtonIcon', () => {
     describe('href', () => {
       it('secures the rel attribute when the target is _blank', () => {
         const component = render(
-          <EuiButtonIcon aria-label="button" href="#" target="_blank" />
+          <EuiButtonIcon
+            iconType="user"
+            aria-label="button"
+            href="#"
+            target="_blank"
+          />
         );
 
         expect(component).toMatchSnapshot();
@@ -85,7 +97,12 @@ describe('EuiButtonIcon', () => {
       it('supports onClick and href', () => {
         const handler = jest.fn();
         const component = mount(
-          <EuiButtonIcon aria-label="hoi" href="#" onClick={handler} />
+          <EuiButtonIcon
+            iconType="user"
+            aria-label="hoi"
+            href="#"
+            onClick={handler}
+          />
         );
         component.find('a').simulate('click');
         expect(handler.mock.calls.length).toEqual(1);
@@ -94,7 +111,7 @@ describe('EuiButtonIcon', () => {
       it('supports onClick as a button', () => {
         const handler = jest.fn();
         const component = mount(
-          <EuiButtonIcon aria-label="hoi" onClick={handler} />
+          <EuiButtonIcon iconType="user" aria-label="hoi" onClick={handler} />
         );
         component.find('button').simulate('click');
         expect(handler.mock.calls.length).toEqual(1);

diff --git a/src/components/button/button_icon/button_icon.tsx b/src/components/button/button_icon/button_icon.tsx
@@ -50,7 +50,7 @@ export type EuiButtonIconColor =
   | 'warning';
 
 export interface EuiButtonIconProps extends CommonProps {
-  iconType?: IconType;
+  iconType: IconType;
   color?: EuiButtonIconColor;
   'aria-label'?: string;
   'aria-labelledby'?: string;

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
@@ -146,7 +146,7 @@ exports[`EuiFieldPassword props dual dualToggleProps is rendered 1`] = `
     </div>
   </div>
   <button
-    aria-label="Show password as plain text. Note: this will visually expose your password on the screen."
+    aria-label="aria-label"
     class="euiButtonIcon euiButtonIcon--primary euiFormControlLayout__append testClass1 testClass2"
     data-test-subj="test subject string"
     title="Show password as plain text. Note: this will visually expose your password on the screen."

diff --git a/src/components/form/field_password/field_password.tsx b/src/components/form/field_password/field_password.tsx
@@ -32,7 +32,7 @@ import {
 } from '../form_control_layout';
 
 import { EuiValidatableControl } from '../validatable_control';
-import { EuiButtonIcon, EuiButtonIconProps } from '../../button';
+import { EuiButtonIcon, EuiButtonIconPropsForButton } from '../../button';
 import { useEuiI18n } from '../../i18n';
 import { useCombinedRefs } from '../../../services';
 
@@ -70,7 +70,7 @@ export type EuiFieldPasswordProps = Omit<
     /**
      * Additional props to apply to the dual toggle. Extends EuiButtonIcon
      */
-    dualToggleProps?: EuiButtonIconProps;
+    dualToggleProps?: Partial<EuiButtonIconPropsForButton>;
   };
 
 export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
@@ -108,11 +108,18 @@ export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
   const [inputRef, _setInputRef] = useState<HTMLInputElement | null>(null);
   const setInputRef = useCombinedRefs([_setInputRef, _inputRef]);
 
-  const handleToggle = (isVisible: boolean) => {
+  const handleToggle = (
+    event: React.MouseEvent<HTMLButtonElement>,
+    isVisible: boolean
+  ) => {
     setInputType(isVisible ? 'password' : 'text');
     if (inputRef) {
       inputRef.focus();
     }
+
+    if (dualToggleProps && dualToggleProps.onClick) {
+      dualToggleProps.onClick(event);
+    }
   };
 
   // Convert any `append` elements to an array so the visibility
@@ -126,12 +133,12 @@ export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
 
     const visibilityToggle = (
       <EuiButtonIcon
-        {...dualToggleProps}
         iconType={isVisible ? 'eyeClosed' : 'eye'}
-        onClick={() => handleToggle(isVisible)}
         aria-label={isVisible ? maskPasswordLabel : showPasswordLabel}
         title={isVisible ? maskPasswordLabel : showPasswordLabel}
         disabled={rest.disabled}
+        {...dualToggleProps}
+        onClick={(e) => handleToggle(e, isVisible)}
       />
     );
     appends = [...appends, visibilityToggle];