Address feedback

mongodb · Feb 29, 2024 · 557c7e6 · 557c7e6
1 parent 6295d17
commit 557c7e6
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 100 deletions.
diff --git a/.changeset/weak-stingrays-live.md b/.changeset/weak-stingrays-live.md
@@ -1,5 +1,5 @@
 ---
-'@leafygreen-ui/form-footer': patch
+'@leafygreen-ui/form-footer': minor
 ---
 
 Adds `cancelButtonProps` and `backButtonProps` to `FormFooter` component for customizing buttons.

diff --git a/packages/form-footer/src/FormFooter.spec.tsx b/packages/form-footer/src/FormFooter.spec.tsx
@@ -3,15 +3,16 @@ import { render } from '@testing-library/react';
 import { axe } from 'jest-axe';
 
 import Button from '@leafygreen-ui/button';
-import X from '@leafygreen-ui/icon/dist/X';
+import XIcon from '@leafygreen-ui/icon/dist/X';
 
 import { FormFooterProps } from './FormFooter';
 import FormFooter from '.';
 
-const buttonTestId = {
-  back: 'lg-form_footer-back-button',
-  cancel: 'lg-form_footer-cancel-button',
-  primary: 'lg-form_footer-primary-button',
+const testId = {
+  backButton: 'lg-form_footer-back_button',
+  backButtonIcon: 'lg-form_footer-back_button-icon',
+  cancelButton: 'lg-form_footer-cancel_button',
+  primaryButton: 'lg-form_footer-primary_button',
 };
 
 const renderFooter = (props: FormFooterProps) => {
@@ -28,15 +29,15 @@ describe('packages/form-footer', () => {
   });
 
   describe('rendering', () => {
-    test('Renders basic primary button', () => {
+    test('renders basic primary button', () => {
       const { getByText } = renderFooter({
         primaryButton: { text: 'Test button' },
       });
       const ButtonElement = getByText('Test button');
       expect(ButtonElement).toBeInTheDocument();
     });
 
-    test('Renders JSX primary button', () => {
+    test('renders JSX primary button', () => {
       const { getByText } = render(
         <FormFooter
           primaryButton={<Button data-testid="test-button">Test button</Button>}
@@ -46,17 +47,17 @@ describe('packages/form-footer', () => {
       expect(ButtonElement).toBeInTheDocument();
     });
 
-    // remove once deprecated props are removed
+    // TODO @stephl3: remove once deprecated props are removed
     describe('deprecated cancel button and back button props', () => {
-      test('Renders cancel button', () => {
+      test('renders cancel button', () => {
         const { getByText } = renderFooter({
           primaryButton: { text: 'Test button' },
         });
         const Cancel = getByText('Cancel');
         expect(Cancel).toBeInTheDocument();
       });
 
-      test('Renders cancel button with custom text', () => {
+      test('renders cancel button with custom text', () => {
         const { getByText } = renderFooter({
           primaryButton: { text: 'Test button' },
           cancelButtonText: 'CancelText',
@@ -65,7 +66,7 @@ describe('packages/form-footer', () => {
         expect(Cancel).toBeInTheDocument();
       });
 
-      test('Renders back button', () => {
+      test('renders back button', () => {
         const { getByText } = renderFooter({
           primaryButton: { text: 'Test button' },
           backButtonText: 'Back',
@@ -81,7 +82,7 @@ describe('packages/form-footer', () => {
         const { queryByText } = renderFooter({
           primaryButton: { text: 'Test button' },
           cancelButtonProps: {
-            text: cancelButtonText,
+            children: cancelButtonText,
           },
         });
         const Cancel = queryByText(cancelButtonText);
@@ -91,11 +92,37 @@ describe('packages/form-footer', () => {
       test('does not render if cancelButtonProps is not defined', () => {
         const { queryByTestId } = renderFooter({
           primaryButton: { text: 'Test button' },
-          cancelButtonText: '', // remove once deprecated props are removed
+          cancelButtonText: '', // TODO @stephl3: remove once deprecated props are removed
         });
-        const Cancel = queryByTestId(buttonTestId.cancel);
+        const Cancel = queryByTestId(testId.cancelButton);
         expect(Cancel).not.toBeInTheDocument();
       });
+
+      describe('left glyph', () => {
+        test('does not render if cancelButtonProps is defined and cancelButtonProps.leftGlyph is undefined', () => {
+          const { getByTestId } = renderFooter({
+            primaryButton: { text: 'Test button' },
+            cancelButtonProps: {
+              children: 'Cancel',
+            },
+          });
+          const Cancel = getByTestId(testId.cancelButton);
+          expect(Cancel.querySelector('svg')).toBeNull();
+        });
+
+        test('renders custom leftGlyph if cancelButtonProps is defined and cancelButtonProps.leftGlyph is defined', () => {
+          const leftGlyphTestId = 'custom-icon-id';
+          const { queryByTestId } = renderFooter({
+            primaryButton: { text: 'Test button' },
+            cancelButtonProps: {
+              children: 'Cancel',
+              leftGlyph: <XIcon data-testid={leftGlyphTestId} />,
+            },
+          });
+          const CancelButtonIcon = queryByTestId(leftGlyphTestId);
+          expect(CancelButtonIcon).toBeInTheDocument();
+        });
+      });
     });
 
     describe('back button', () => {
@@ -104,55 +131,49 @@ describe('packages/form-footer', () => {
         const { queryByText } = renderFooter({
           primaryButton: { text: 'Test button' },
           backButtonProps: {
-            text: backButtonText,
+            children: backButtonText,
           },
         });
         const Back = queryByText(backButtonText);
         expect(Back).toBeInTheDocument();
       });
 
-      test('does not render', () => {
+      test('does not render if backButtonProps is undefined', () => {
         const { queryByTestId } = renderFooter({
           primaryButton: { text: 'Test button' },
         });
-        const Back = queryByTestId(buttonTestId.back);
+        const Back = queryByTestId(testId.backButton);
         expect(Back).not.toBeInTheDocument();
       });
 
-      describe('Back button left glyph', () => {
-        test('Renders ArrowLeftIcon if leftGlyph is undefined', () => {
+      describe('left glyph', () => {
+        test('renders ArrowLeftIcon if backButtonProps is undefined and backButtonText is defined', () => {
           const { queryByTestId } = renderFooter({
             primaryButton: { text: 'Test button' },
-            backButtonProps: {
-              text: 'Back',
-              leftGlyph: undefined,
-            },
+            backButtonText: 'Back',
           });
-          const BackButtonIcon = queryByTestId(
-            'lg-form_footer-back-button-icon',
-          );
+          const BackButtonIcon = queryByTestId(testId.backButtonIcon);
           expect(BackButtonIcon).toBeInTheDocument();
         });
 
-        test('Does not render if leftGlyph is null', () => {
+        test('does not render if backButtonProps is defined and backButtonProps.leftGlyph is undefined', () => {
           const { getByTestId } = renderFooter({
             primaryButton: { text: 'Test button' },
             backButtonProps: {
-              text: 'Back',
-              leftGlyph: null,
+              children: 'Back',
             },
           });
-          const Back = getByTestId(buttonTestId.back);
+          const Back = getByTestId(testId.backButton);
           expect(Back.querySelector('svg')).toBeNull();
         });
 
-        test('Renders custom leftGlyph icon if leftGlyph is ReactElement', () => {
+        test('renders custom leftGlyph if backButtonProps is defined and backButtonProps.leftGlyph is defined', () => {
           const leftGlyphTestId = 'custom-icon-id';
           const { queryByTestId } = renderFooter({
             primaryButton: { text: 'Test button' },
             backButtonProps: {
-              text: 'Back',
-              leftGlyph: <X data-testid={leftGlyphTestId} />,
+              children: 'Back',
+              leftGlyph: <XIcon data-testid={leftGlyphTestId} />,
             },
           });
           const BackButtonIcon = queryByTestId(leftGlyphTestId);

diff --git a/packages/form-footer/src/FormFooter.story.tsx b/packages/form-footer/src/FormFooter.story.tsx
@@ -43,11 +43,10 @@ const meta: StoryMetaType<typeof FormFooter> = {
       combineArgs: {
         backButtonProps: [
           undefined,
-          { text: 'Back', leftGlyph: undefined },
-          { text: 'Back', leftGlyph: null },
+          { children: 'Back', leftGlyph: undefined },
+          { children: 'Back', leftGlyph: <Icon glyph="ArrowLeft" /> },
         ],
-        cancelButtonProps: [undefined, { text: 'Cancel' }],
-        cancelButtonText: [''], // remove once deprecated props are removed
+        cancelButtonProps: [undefined, { children: 'Cancel' }],
         errorMessage: [undefined, 'This is an error message'],
       },
       decorator: StoryFn => (
@@ -61,6 +60,7 @@ const meta: StoryMetaType<typeof FormFooter> = {
     darkMode: false,
     primaryButtonText: 'Button',
     primaryButton: { text: 'Button' },
+    cancelButtonText: '', // TODO @stephl3: remove once deprecated props are removed
     errorMessage: 'Error message',
   },
   argTypes: {
@@ -100,8 +100,8 @@ LiveExample.parameters = {
   },
 };
 LiveExample.args = {
-  backButtonProps: { text: 'Back' },
-  cancelButtonProps: { text: 'Cancel' },
+  backButtonProps: { children: 'Back' },
+  cancelButtonProps: { children: 'Cancel' },
 };
 
 export const WithCustomPrimaryButton: StoryType<typeof FormFooter> =
@@ -117,8 +117,8 @@ WithCustomPrimaryButton.args = {
       Save to cloud
     </Button>
   ),
-  backButtonProps: { text: 'Back' },
-  cancelButtonProps: { text: 'Cancel' },
+  backButtonProps: { children: 'Back' },
+  cancelButtonProps: { children: 'Cancel' },
 };
 WithCustomPrimaryButton.parameters = {
   chromatic: {