From 79396f9c8a29116f4bfc7281c152b24026d7bc93 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Tue, 29 Aug 2023 14:04:11 -0400 Subject: [PATCH 1/7] chore(ChipGroup): test revamp --- .../Chip/__tests__/ChipGroup.test.tsx | 102 +++--- .../__snapshots__/ChipGroup.test.tsx.snap | 340 ------------------ 2 files changed, 50 insertions(+), 392 deletions(-) delete mode 100644 packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 69055dcc3ef..77170c9da70 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -6,65 +6,63 @@ import userEvent from '@testing-library/user-event'; import { ChipGroup } from '../index'; import { Chip } from '../../Chip'; -describe('ChipGroup', () => { - test('chip group default', () => { - const { asFragment } = render( - - 1.1 - - ); +test('chip group default', () => { + render( + + 1.1 + + ); - expect(asFragment()).toMatchSnapshot(); - }); + expect(screen.getByRole('group')).toHaveClass(''); +}); - test('chip group with category', () => { - const { asFragment } = render( - - 1.1 - - ); - expect(asFragment()).toMatchSnapshot(); - }); +test('chip group with category', () => { + render( + + 1.1 + + ); + expect(screen.getByText('category')).toBeVisible(); +}); - test('chip group with closable category', () => { - const { asFragment } = render( - - 1.1 - - ); - expect(asFragment()).toMatchSnapshot(); - }); +test('chip group with closable category', () => { + render( + + 1.1 + + ); + expect(screen.getByLabelText('Close chip group').closest('div')).toHaveClass('pf-v5-c-chip-group__close'); +}); - test('chip group expanded', async () => { - const user = userEvent.setup(); +test('chip group expanded', async () => { + const user = userEvent.setup(); - render( - - 1 - 2 - 3 - 4 - - ); + render( + + 1 + 2 + 3 + 4 + + ); - const moreText = screen.getByText('1 more'); - expect(moreText).toBeInTheDocument(); + const moreText = screen.getByText('1 more'); + expect(moreText).toBeInTheDocument(); - await user.click(moreText); - expect(screen.getByText('Show Less')).toBeInTheDocument(); - }); + await user.click(moreText); + expect(screen.getByText('Show Less')).toBeInTheDocument(); +}); - test('chip group will not render if no children passed', () => { - render(); - expect(screen.queryByRole('group')).toBeNull(); - }); +test('chip group will not render if no children passed', () => { + render(); + expect(screen.queryByRole('group')).toBeNull(); +}); - test('chip group with category and tooltip', () => { - const { asFragment } = render( - - 1.1 - - ); - expect(asFragment()).toMatchSnapshot(); - }); +test('chip group with category and tooltip', () => { + render( + + 1.1 + + ); + expect(screen.getByText('A very long category name')).toBeVisible(); }); diff --git a/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap b/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap deleted file mode 100644 index 0b8e5375434..00000000000 --- a/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap +++ /dev/null @@ -1,340 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ChipGroup chip group default 1`] = ` - -
-
-
    -
  • -
    - - - 1.1 - - - - - -
    -
    • -
-
-
- -`; - -exports[`ChipGroup chip group with category 1`] = ` - -
-
- - category - -
    -
  • -
    - - - 1.1 - - - - - -
    -
    • -
-
-
- -`; - -exports[`ChipGroup chip group with category and tooltip 1`] = ` - -
-
- - A very long category name - -
    -
  • -
    - - - 1.1 - - - - - -
    -
    • -
-
-
- -`; - -exports[`ChipGroup chip group with closable category 1`] = ` - -
-
- - category - -
    -
  • -
    - - - 1.1 - - - - - -
    -
    • -
-
-
- -
-
- -`; From 0409b128aaf1dcaaa76d9317423e2174a08005ad Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Tue, 29 Aug 2023 14:08:32 -0400 Subject: [PATCH 2/7] add className --- .../react-core/src/components/Chip/__tests__/ChipGroup.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 77170c9da70..0c4c43381da 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -13,7 +13,7 @@ test('chip group default', () => { ); - expect(screen.getByRole('group')).toHaveClass(''); + expect(screen.getByRole('group')).toHaveClass('pf-v5-c-chip-group'); }); test('chip group with category', () => { From a14afbc2fcc3217f757f232249465971a6dc9444 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Wed, 27 Sep 2023 09:16:17 -0400 Subject: [PATCH 3/7] chip group updates, WIP --- .../Chip/__tests__/ChipGroup.test.tsx | 87 ++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 0c4c43381da..73d74ad3498 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -5,6 +5,7 @@ import userEvent from '@testing-library/user-event'; import { ChipGroup } from '../index'; import { Chip } from '../../Chip'; +import styles from '@patternfly/react-styles/css/components/Chip/chip-group'; test('chip group default', () => { render( @@ -13,7 +14,37 @@ test('chip group default', () => { ); - expect(screen.getByRole('group')).toHaveClass('pf-v5-c-chip-group'); + expect(screen.getByRole('group')).toHaveClass(styles.chipGroup); +}); + +test('chip group with custom className', () => { + render( + + 1.1 + + ); + + expect(screen.getByRole('group')).toHaveClass(`${styles.chipGroup} test`); +}); + +test('chip group has aria-label by default when categoryName is not passed', () => { + render( + + 1.1 + + ); + + expect(screen.getByRole('group')).toHaveAccessibleName('Chip group category'); +}); + +test('chip group with custom aria-label', () => { + render( + + 1.1 + + ); + + expect(screen.getByRole('group')).toHaveAccessibleName('test chip group'); }); test('chip group with category', () => { @@ -25,13 +56,59 @@ test('chip group with category', () => { expect(screen.getByText('category')).toBeVisible(); }); +test('chip group expands', async () => { + const user = userEvent.setup(); + const chips = ['Chip one', 'Really long chip that goes on and on', 'Chip three', 'Chip four', 'Chip five']; + + render( + + {chips.map((currentChip) => ( + + {currentChip} + + ))} + + ); + + await user.click(screen.getByText('2 more')); + expect(screen.getByText('chip five')).toBeVisible(); +}); + test('chip group with closable category', () => { render( 1.1 ); - expect(screen.getByLabelText('Close chip group').closest('div')).toHaveClass('pf-v5-c-chip-group__close'); + expect(screen.getByLabelText('Close chip group').closest('div')).toHaveClass(styles.chipGroupClose); +}); + +test('chip group onClick', async () => { + const onClose = jest.fn(); + const user = userEvent.setup(); + + render( + + 1.1 + + ); + + await user.click(screen.getByLabelText('Close chip group')); + expect(onClose).toHaveBeenCalled(); +}); + +test('chip group onOverflowChipClick', async () => { + const onOverflowChipClick = jest.fn(); + const user = userEvent.setup(); + + render( + + 1.1 + + ); + + await user.click(screen.getByLabelText('Close chip group')); + expect(onOverflowChipClick).toHaveBeenCalled(); }); test('chip group expanded', async () => { @@ -66,3 +143,9 @@ test('chip group with category and tooltip', () => { ); expect(screen.getByText('A very long category name')).toBeVisible(); }); + +test('chip group renders to match snapshot', () => { + const { asFragment } = render(); + expect(screen.getByLabelText('test chip group')).toBeInTheDocument(); + expect(asFragment()).toMatchSnapshot(); +}); From 2ee4ef98801072c2a87608e775a8bba5ac8a4863 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Thu, 26 Oct 2023 15:53:31 -0400 Subject: [PATCH 4/7] PR feedback from Eric, added tests --- .../Chip/__tests__/ChipGroup.test.tsx | 177 ++++++++++++++++-- .../__snapshots__/ChipGroup.test.tsx.snap | 29 +++ 2 files changed, 190 insertions(+), 16 deletions(-) create mode 100644 packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 73d74ad3498..09678d6ffeb 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -56,6 +56,33 @@ test('chip group with category', () => { expect(screen.getByText('category')).toBeVisible(); }); +test('chip group with category renders with class pf-m-category', () => { + render( + + 1.1 + + ); + expect(screen.getByRole('group')).toHaveClass(`${styles.chipGroup} pf-m-category`); +}); + +test('chip group has aria-labelledby attribute', () => { + render( + + 1.1 + + ); + expect(screen.getByRole('group')).toHaveAttribute('aria-labelledby', expect.stringContaining(`pf-random-id`)); +}); + +test('chip group has aria-labelledby attribute set to ID value', () => { + render( + + 1.1 + + ); + expect(screen.getByRole('group')).toHaveAttribute('aria-labelledby', 'chip-group-id'); +}); + test('chip group expands', async () => { const user = userEvent.setup(); const chips = ['Chip one', 'Really long chip that goes on and on', 'Chip three', 'Chip four', 'Chip five']; @@ -63,15 +90,13 @@ test('chip group expands', async () => { render( {chips.map((currentChip) => ( - - {currentChip} - + {currentChip} ))} ); await user.click(screen.getByText('2 more')); - expect(screen.getByText('chip five')).toBeVisible(); + expect(screen.getByText('Chip five')).toBeVisible(); }); test('chip group with closable category', () => { @@ -80,7 +105,16 @@ test('chip group with closable category', () => { 1.1 ); - expect(screen.getByLabelText('Close chip group').closest('div')).toHaveClass(styles.chipGroupClose); + expect(screen.getByRole('group').lastChild).toHaveClass(styles.chipGroupClose); +}); + +test('chip group with closeBtnAriaLabel', () => { + render( + + 1.1 + + ); + expect(screen.getByLabelText('close button aria label')).toBeInTheDocument(); }); test('chip group onClick', async () => { @@ -103,11 +137,14 @@ test('chip group onOverflowChipClick', async () => { render( - 1.1 + 1 + 2 + 3 + 4 ); - await user.click(screen.getByLabelText('Close chip group')); + await user.click(screen.getByText('1 more')); expect(onOverflowChipClick).toHaveBeenCalled(); }); @@ -130,22 +167,130 @@ test('chip group expanded', async () => { expect(screen.getByText('Show Less')).toBeInTheDocument(); }); -test('chip group will not render if no children passed', () => { - render(); - expect(screen.queryByRole('group')).toBeNull(); +test('overflow chip does not render by default when < 4 children are passed', async () => { + const user = userEvent.setup(); + + render( + + 1 + 2 + 3 + + ); + + expect(screen.queryByText('1 more')).not.toBeInTheDocument(); }); -test('chip group with category and tooltip', () => { +test('overflow chip collapsed by default', async () => { + const user = userEvent.setup(); + render( - - 1.1 + + 1 + 2 + 3 + 4 + + ); + + const moreText = screen.getByText('1 more'); + expect(moreText).toBeInTheDocument(); +}); + +test('overflow chip renders with the default numChips prop value of 3', async () => { + const user = userEvent.setup(); + + render( + + 1 + 2 + 3 + 4 + + ); + + const moreText = screen.getByText('1 more'); + expect(moreText).toBeInTheDocument(); + + await user.click(moreText); + expect(screen.getByText('Show Less')).toBeInTheDocument(); +}); + +test('overflow chip renders with numChips prop value of 2', async () => { + const user = userEvent.setup(); + + render( + + 1 + 2 + 3 ); - expect(screen.getByText('A very long category name')).toBeVisible(); + + const moreText = screen.getByText('1 more'); + expect(moreText).toBeInTheDocument(); + + await user.click(moreText); + expect(screen.getByText('Show Less')).toBeInTheDocument(); +}); + +test('clicking the overflow chip causes the text to update with the default props', async () => { + const user = userEvent.setup(); + + render( + + 1 + 2 + 3 + 4 + + ); + + const moreText = screen.getByText('1 more'); + + await user.click(moreText); + expect(screen.getByText('Show Less')).toBeInTheDocument(); +}); + +test('passing in the expandedText and collapsedText props work', async () => { + const user = userEvent.setup(); + + render( + + 1 + 2 + 3 + 4 + + ); + + const moreText = screen.getByText('Expand'); + expect(moreText).toBeInTheDocument(); + + await user.click(moreText); + expect(screen.getByText('Collapse')).toBeInTheDocument(); +}); + +test('passing defaultIsOpen of true causes the chip group to be expanded by default', async () => { + render( + + 1 + 2 + 3 + 4 + + ); + + expect(screen.getByText('Show Less')).toBeInTheDocument(); +}); + +test('chip group will not render if no children passed', () => { + render(); + expect(screen.queryByRole('group')).toBeNull(); }); test('chip group renders to match snapshot', () => { - const { asFragment } = render(); - expect(screen.getByLabelText('test chip group')).toBeInTheDocument(); + const { asFragment } = render(chips); + expect(screen.getByRole('group')).toBeInTheDocument(); expect(asFragment()).toMatchSnapshot(); }); diff --git a/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap b/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap new file mode 100644 index 00000000000..e54bd1962a6 --- /dev/null +++ b/packages/react-core/src/components/Chip/__tests__/__snapshots__/ChipGroup.test.tsx.snap @@ -0,0 +1,29 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`chip group renders to match snapshot 1`] = ` + +
+
+
    +
  • + chips +
    • +
+
+
+ +`; From 58967d9868992eae48abb5c5c1c15ff75129e2e0 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:45:23 -0400 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com> --- .../src/components/Chip/__tests__/ChipGroup.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 09678d6ffeb..3726d263d98 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -62,7 +62,7 @@ test('chip group with category renders with class pf-m-category', () => { 1.1 ); - expect(screen.getByRole('group')).toHaveClass(`${styles.chipGroup} pf-m-category`); + expect(screen.getByRole('group')).toHaveClass(`${styles.modifiers.category}`); }); test('chip group has aria-labelledby attribute', () => { @@ -110,8 +110,8 @@ test('chip group with closable category', () => { test('chip group with closeBtnAriaLabel', () => { render( - - 1.1 + + 1.1 ); expect(screen.getByLabelText('close button aria label')).toBeInTheDocument(); @@ -128,7 +128,7 @@ test('chip group onClick', async () => { ); await user.click(screen.getByLabelText('Close chip group')); - expect(onClose).toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); }); test('chip group onOverflowChipClick', async () => { From 77b8388ff33a51755284e3aba178416ce2b484e4 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:47:11 -0400 Subject: [PATCH 6/7] update onOverflowChipClick test to match onClick --- .../react-core/src/components/Chip/__tests__/ChipGroup.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index 3726d263d98..ebb42828d17 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -145,7 +145,7 @@ test('chip group onOverflowChipClick', async () => { ); await user.click(screen.getByText('1 more')); - expect(onOverflowChipClick).toHaveBeenCalled(); + expect(onOverflowChipClick).toHaveBeenCalledTimes(1); }); test('chip group expanded', async () => { From dd70f94ee42927583588eb95c3e88c36fbd9beb7 Mon Sep 17 00:00:00 2001 From: Jenny <32821331+jenny-s51@users.noreply.github.com> Date: Fri, 27 Oct 2023 13:07:28 -0400 Subject: [PATCH 7/7] add check for accessibleName --- .../react-core/src/components/Chip/__tests__/ChipGroup.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx index ebb42828d17..807944710f1 100644 --- a/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx +++ b/packages/react-core/src/components/Chip/__tests__/ChipGroup.test.tsx @@ -115,6 +115,7 @@ test('chip group with closeBtnAriaLabel', () => { ); expect(screen.getByLabelText('close button aria label')).toBeInTheDocument(); + expect(screen.getByLabelText('close button aria label')).toHaveAccessibleName("close button aria label category"); }); test('chip group onClick', async () => {