From 1e2d2af6d287f846615c706ae43fec165165957d Mon Sep 17 00:00:00 2001 From: Jin Jun Oh Date: Fri, 5 Apr 2024 12:39:25 -0700 Subject: [PATCH 1/4] SWC-6753: prevent Available Evaluation Lists from collapsing twice --- .../AvailableEvaluationQueueList.test.tsx | 44 +++++-------------- .../AvailableEvaluationQueueList.tsx | 38 ++++++---------- 2 files changed, 25 insertions(+), 57 deletions(-) diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx index ffa25ed999..42e784f36c 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx @@ -40,15 +40,11 @@ function setUp(props: AvailableEvaluationQueueListProps) { const user = userEvent.setup() const component = renderComponent(props) const selectInput = screen.queryByLabelText('Selected Evaluation Queue') - const showStaticListButton = screen.queryByRole('button', { - name: /all available evaluation queues/i, - }) const staticListHeading = screen.queryByText('Available Evaluation Queues:') return { component, user, selectInput, - showStaticListButton, staticListHeading, } } @@ -59,22 +55,20 @@ describe('AvailableEvaluationQueueList', () => { }) it('handles 0 available evaluations', () => { - const { selectInput, showStaticListButton } = setUp(defaultProps) + const { selectInput } = setUp(defaultProps) screen.getByText('No evaluations found') expect(selectInput).toBeNull() - expect(showStaticListButton).toBeNull() }) it('handles 1 available evaluation', () => { - const { selectInput, showStaticListButton, staticListHeading } = setUp({ + const { selectInput, staticListHeading } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue], }) screen.getByText(mockEvaluationQueue.name!) expect(selectInput).toBeNull() - expect(showStaticListButton).toBeNull() expect(staticListHeading).toBeNull() expect(onChangeSelectedEvaluation).toHaveBeenCalledTimes(1) @@ -82,7 +76,7 @@ describe('AvailableEvaluationQueueList', () => { }) it('handles 1 available evaluation that is not selectable', () => { - const { selectInput, showStaticListButton, staticListHeading } = setUp({ + const { selectInput, staticListHeading } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue], isSelectable: false, @@ -90,26 +84,17 @@ describe('AvailableEvaluationQueueList', () => { screen.getByText(mockEvaluationQueue.name!) expect(staticListHeading).toBeNull() - expect(showStaticListButton).toBeNull() expect(selectInput).toBeNull() expect(onChangeSelectedEvaluation).not.toHaveBeenCalled() }) it('handles >1 available evaluations', async () => { - const { selectInput, showStaticListButton, staticListHeading, user } = - setUp({ - ...defaultProps, - evaluations: [mockEvaluationQueue, secondEvaluation], - }) + const { selectInput, staticListHeading, user } = setUp({ + ...defaultProps, + evaluations: [mockEvaluationQueue, secondEvaluation], + }) expect(selectInput).not.toBeNull() - expect(showStaticListButton).not.toBeNull() - expect(showStaticListButton?.textContent).toContain('Show') - expect(staticListHeading).not.toBeVisible() - - await user.click(showStaticListButton!) - - expect(showStaticListButton?.textContent).toContain('Hide') expect(staticListHeading).toBeVisible() expect(screen.getByText(mockEvaluationQueue.name!)).toBeVisible() expect(screen.getByText(secondEvaluation.name)).toBeVisible() @@ -122,18 +107,13 @@ describe('AvailableEvaluationQueueList', () => { }) it('handles >1 available evaluations that are not selectable', async () => { - const { selectInput, staticListHeading, showStaticListButton, user } = - setUp({ - ...defaultProps, - evaluations: [mockEvaluationQueue, secondEvaluation], - isSelectable: false, - }) + const { selectInput, staticListHeading } = setUp({ + ...defaultProps, + evaluations: [mockEvaluationQueue, secondEvaluation], + isSelectable: false, + }) expect(selectInput).toBeNull() - expect(showStaticListButton).not.toBeNull() - expect(staticListHeading).not.toBeVisible() - - await user.click(showStaticListButton!) expect(staticListHeading).toBeVisible() expect(screen.getByText(mockEvaluationQueue.name!)).toBeVisible() diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx index fbede16d76..214fafcb57 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx @@ -2,8 +2,6 @@ import { HelpOutlineTwoTone } from '@mui/icons-material' import { Autocomplete, Box, - Button, - Collapse, List, ListItem, MenuItem, @@ -57,32 +55,22 @@ function AvailableEvaluationQueueStaticList( props: AvailableEvaluationQueueStaticListProps, ) { const { evaluations } = props - const [showList, setShowList] = useState(false) return ( - - - Available Evaluation Queues: - - {evaluations.map(evaluation => { - return ( - - - - ) - })} - - + Available Evaluation Queues: + + {evaluations.map(evaluation => { + return ( + + + + ) + })} + ) } From fffe8db9cb8b348c3dab1c7e0f790b498375a575 Mon Sep 17 00:00:00 2001 From: Jin Jun Oh Date: Fri, 5 Apr 2024 14:02:32 -0700 Subject: [PATCH 2/4] SWC-6753: resolve lint error --- .../ChallengeSubmission/AvailableEvaluationQueueList.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx index 42e784f36c..73ff41e833 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx @@ -106,7 +106,7 @@ describe('AvailableEvaluationQueueList', () => { expect(onChangeSelectedEvaluation).toHaveBeenCalledWith(secondEvaluation) }) - it('handles >1 available evaluations that are not selectable', async () => { + it('handles >1 available evaluations that are not selectable', () => { const { selectInput, staticListHeading } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue, secondEvaluation], From 91e95704603e75b177db75484a0df1d5580379f3 Mon Sep 17 00:00:00 2001 From: Jin Jun Oh Date: Mon, 8 Apr 2024 14:16:52 -0700 Subject: [PATCH 3/4] SWC-6753: change behavior depending on isSelectable --- .../AvailableEvaluationQueueList.stories.tsx | 7 ++ .../AvailableEvaluationQueueList.test.tsx | 83 +++++++++++++++++-- .../AvailableEvaluationQueueList.tsx | 62 ++++++++++---- .../src/mocks/entity/mockEvaluationQueue.ts | 4 + 4 files changed, 137 insertions(+), 19 deletions(-) diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.stories.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.stories.tsx index 0a5c6f15b9..76f327b5f4 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.stories.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.stories.tsx @@ -4,6 +4,7 @@ import React from 'react' import { generatedEvaulations, mockEvaluationQueue, + sevenGeneratedEvaulations, } from '../../mocks/entity/mockEvaluationQueue' import AvailableEvaluationQueueList from './AvailableEvaluationQueueList' @@ -36,6 +37,12 @@ export const OneAvailable: Story = { }, } +export const SevenAvailable: Story = { + args: { + evaluations: sevenGeneratedEvaulations, + }, +} + export const ManyAvailable: Story = { args: { evaluations: generatedEvaulations, diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx index 73ff41e833..885b5dd47a 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx @@ -13,6 +13,11 @@ const secondEvaluation = { id: '12345', name: 'another evaluation queue', } +const eightEvaluationQueues: any[] = Array.from({ length: 8 }, (_, index) => ({ + ...mockEvaluationQueue, + id: `${index + 1}`, + name: `Evaluation Queue ${index + 1}`, +})) async function chooseAutocompleteOption( el: HTMLElement, @@ -40,11 +45,15 @@ function setUp(props: AvailableEvaluationQueueListProps) { const user = userEvent.setup() const component = renderComponent(props) const selectInput = screen.queryByLabelText('Selected Evaluation Queue') + const showStaticListButton = screen.queryByRole('button', { + name: /all available evaluation queues/i, + }) const staticListHeading = screen.queryByText('Available Evaluation Queues:') return { component, user, selectInput, + showStaticListButton, staticListHeading, } } @@ -89,12 +98,20 @@ describe('AvailableEvaluationQueueList', () => { }) it('handles >1 available evaluations', async () => { - const { selectInput, staticListHeading, user } = setUp({ - ...defaultProps, - evaluations: [mockEvaluationQueue, secondEvaluation], - }) + const { selectInput, staticListHeading, showStaticListButton, user } = + setUp({ + ...defaultProps, + evaluations: [mockEvaluationQueue, secondEvaluation], + }) expect(selectInput).not.toBeNull() + expect(staticListHeading).not.toBeNull() + expect(showStaticListButton?.textContent).toContain('Show') + expect(staticListHeading).not.toBeVisible() + + await user.click(showStaticListButton!) + + expect(showStaticListButton?.textContent).toContain('Hide') expect(staticListHeading).toBeVisible() expect(screen.getByText(mockEvaluationQueue.name!)).toBeVisible() expect(screen.getByText(secondEvaluation.name)).toBeVisible() @@ -114,11 +131,67 @@ describe('AvailableEvaluationQueueList', () => { }) expect(selectInput).toBeNull() - expect(staticListHeading).toBeVisible() expect(screen.getByText(mockEvaluationQueue.name!)).toBeVisible() expect(screen.getByText(secondEvaluation.name)).toBeVisible() expect(onChangeSelectedEvaluation).not.toHaveBeenCalled() }) + + it('handles >7 available evaluations', async () => { + const { selectInput, showStaticListButton, staticListHeading, user } = + setUp({ + ...defaultProps, + evaluations: eightEvaluationQueues, + }) + + expect(selectInput).not.toBeNull() + expect(showStaticListButton).not.toBeNull() + expect(showStaticListButton?.textContent).toContain('Show') + expect(staticListHeading).not.toBeVisible() + + await user.click(showStaticListButton!) + + expect(showStaticListButton?.textContent).toContain('Hide') + expect(staticListHeading).toBeVisible() + eightEvaluationQueues.forEach((queue, index) => { + expect(screen.getByText(queue.name)).toBeVisible() + }) + + await chooseAutocompleteOption( + selectInput!, + eightEvaluationQueues[1].name, + user, + ) + + expect(selectInput).toHaveValue(eightEvaluationQueues[1].name) + expect(onChangeSelectedEvaluation).toHaveBeenCalledTimes(1) + expect(onChangeSelectedEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + name: eightEvaluationQueues[1].name, + }), + ) + }) + + it('handles >7 available evaluations that are not selectable', async () => { + const { selectInput, showStaticListButton, staticListHeading, user } = + setUp({ + ...defaultProps, + evaluations: eightEvaluationQueues, + isSelectable: false, + }) + + expect(selectInput).toBeNull() + expect(showStaticListButton).not.toBeNull() + expect(staticListHeading).not.toBeVisible() + + await user.click(showStaticListButton!) + + expect(staticListHeading).toBeVisible() + eightEvaluationQueues.forEach((queue, index) => { + expect(screen.getByText(queue.name)).toBeVisible() + }) + + expect(onChangeSelectedEvaluation).not.toHaveBeenCalled() + }) }) diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx index 214fafcb57..e84e05fd3e 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx @@ -2,6 +2,8 @@ import { HelpOutlineTwoTone } from '@mui/icons-material' import { Autocomplete, Box, + Button, + Collapse, List, ListItem, MenuItem, @@ -50,27 +52,59 @@ type AvailableEvaluationQueueStaticListProps = Pick< AvailableEvaluationQueueListProps, 'evaluations' > - function AvailableEvaluationQueueStaticList( props: AvailableEvaluationQueueStaticListProps, ) { const { evaluations } = props - return ( - + <> Available Evaluation Queues: - {evaluations.map(evaluation => { - return ( - - - - ) - })} + {evaluations.map(evaluation => ( + + + + ))} + + ) +} + +type AvailableEvaluationQueueCollapsableListProps = Pick< + AvailableEvaluationQueueListProps, + 'evaluations' | 'isSelectable' +> + +function AvailableEvaluationQueueCollapsableList( + props: AvailableEvaluationQueueCollapsableListProps, +) { + const { evaluations, isSelectable } = props + const [showList, setShowList] = useState(false) + const collapseBound = isSelectable ? 2 : 8 + + return ( + + {evaluations.length >= (collapseBound as number) ? ( + <> + + + + + + ) : ( + <> + + + )} ) } @@ -160,7 +194,7 @@ function AvailableEvaluationQueueList( className="AvailableEvaluationQueueList" > {isSelectable && } - + ) } diff --git a/packages/synapse-react-client/src/mocks/entity/mockEvaluationQueue.ts b/packages/synapse-react-client/src/mocks/entity/mockEvaluationQueue.ts index 2903ed4fac..eb0fc61a86 100644 --- a/packages/synapse-react-client/src/mocks/entity/mockEvaluationQueue.ts +++ b/packages/synapse-react-client/src/mocks/entity/mockEvaluationQueue.ts @@ -30,3 +30,7 @@ export const mockEvaluations: Evaluation[] = [mockEvaluationQueue] export const generatedEvaulations: Evaluation[] = times(10, () => generateEvaluation(), ) + +export const sevenGeneratedEvaulations: Evaluation[] = times(7, () => + generateEvaluation(), +) From 69be69da2f4705cf5fda484ef4119c83c817638e Mon Sep 17 00:00:00 2001 From: Jin Jun Oh Date: Tue, 9 Apr 2024 08:32:20 -0700 Subject: [PATCH 4/4] SWC-6753: make suggested changes for unit test --- .../AvailableEvaluationQueueList.test.tsx | 23 ++++++++++++------- .../AvailableEvaluationQueueList.tsx | 5 ++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx index 885b5dd47a..faacfd9244 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.test.tsx @@ -64,14 +64,15 @@ describe('AvailableEvaluationQueueList', () => { }) it('handles 0 available evaluations', () => { - const { selectInput } = setUp(defaultProps) + const { selectInput, showStaticListButton } = setUp(defaultProps) screen.getByText('No evaluations found') expect(selectInput).toBeNull() + expect(showStaticListButton).toBeNull() }) it('handles 1 available evaluation', () => { - const { selectInput, staticListHeading } = setUp({ + const { selectInput, staticListHeading, showStaticListButton } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue], }) @@ -79,13 +80,14 @@ describe('AvailableEvaluationQueueList', () => { screen.getByText(mockEvaluationQueue.name!) expect(selectInput).toBeNull() expect(staticListHeading).toBeNull() + expect(showStaticListButton).toBeNull() expect(onChangeSelectedEvaluation).toHaveBeenCalledTimes(1) expect(onChangeSelectedEvaluation).toHaveBeenCalledWith(mockEvaluationQueue) }) it('handles 1 available evaluation that is not selectable', () => { - const { selectInput, staticListHeading } = setUp({ + const { selectInput, staticListHeading, showStaticListButton } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue], isSelectable: false, @@ -93,11 +95,12 @@ describe('AvailableEvaluationQueueList', () => { screen.getByText(mockEvaluationQueue.name!) expect(staticListHeading).toBeNull() + expect(showStaticListButton).toBeNull() expect(selectInput).toBeNull() expect(onChangeSelectedEvaluation).not.toHaveBeenCalled() }) - it('handles >1 available evaluations', async () => { + it('displays collapsed list for <8 selectable available evaluations', async () => { const { selectInput, staticListHeading, showStaticListButton, user } = setUp({ ...defaultProps, @@ -106,6 +109,7 @@ describe('AvailableEvaluationQueueList', () => { expect(selectInput).not.toBeNull() expect(staticListHeading).not.toBeNull() + expect(showStaticListButton).not.toBeNull() expect(showStaticListButton?.textContent).toContain('Show') expect(staticListHeading).not.toBeVisible() @@ -123,8 +127,8 @@ describe('AvailableEvaluationQueueList', () => { expect(onChangeSelectedEvaluation).toHaveBeenCalledWith(secondEvaluation) }) - it('handles >1 available evaluations that are not selectable', () => { - const { selectInput, staticListHeading } = setUp({ + it('displays static list for <8 available evaluations that are not selectable', () => { + const { selectInput, staticListHeading, showStaticListButton } = setUp({ ...defaultProps, evaluations: [mockEvaluationQueue, secondEvaluation], isSelectable: false, @@ -132,13 +136,14 @@ describe('AvailableEvaluationQueueList', () => { expect(selectInput).toBeNull() expect(staticListHeading).toBeVisible() + expect(showStaticListButton).toBeNull() expect(screen.getByText(mockEvaluationQueue.name!)).toBeVisible() expect(screen.getByText(secondEvaluation.name)).toBeVisible() expect(onChangeSelectedEvaluation).not.toHaveBeenCalled() }) - it('handles >7 available evaluations', async () => { + it('displays collapsed list for >8 selectable available evaluations', async () => { const { selectInput, showStaticListButton, staticListHeading, user } = setUp({ ...defaultProps, @@ -173,7 +178,7 @@ describe('AvailableEvaluationQueueList', () => { ) }) - it('handles >7 available evaluations that are not selectable', async () => { + it('displays collapsed list for >8 available evaluations that are not selectable', async () => { const { selectInput, showStaticListButton, staticListHeading, user } = setUp({ ...defaultProps, @@ -183,10 +188,12 @@ describe('AvailableEvaluationQueueList', () => { expect(selectInput).toBeNull() expect(showStaticListButton).not.toBeNull() + expect(showStaticListButton?.textContent).toContain('Show') expect(staticListHeading).not.toBeVisible() await user.click(showStaticListButton!) + expect(showStaticListButton?.textContent).toContain('Hide') expect(staticListHeading).toBeVisible() eightEvaluationQueues.forEach((queue, index) => { expect(screen.getByText(queue.name)).toBeVisible() diff --git a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx index e84e05fd3e..a0d8b1a485 100644 --- a/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx +++ b/packages/synapse-react-client/src/components/ChallengeSubmission/AvailableEvaluationQueueList.tsx @@ -83,11 +83,12 @@ function AvailableEvaluationQueueCollapsableList( ) { const { evaluations, isSelectable } = props const [showList, setShowList] = useState(false) - const collapseBound = isSelectable ? 2 : 8 + const nEvaluationsCollapseLimit = isSelectable ? 2 : 8 + const shouldCollapse = evaluations.length >= nEvaluationsCollapseLimit return ( - {evaluations.length >= (collapseBound as number) ? ( + {shouldCollapse ? ( <>