From 78f928bbed29c1581a8d7e0c221edd3dc1f69349 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Mon, 8 Feb 2021 20:27:25 -0800 Subject: [PATCH 1/9] Commenting where multiselectable will go --- src/components/Accordion/Accordion.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index f65aeecca8..1d408c66bd 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -12,6 +12,7 @@ interface AccordionItem { interface AccordionProps { bordered?: boolean + /* multiselectable: boolean */ items: AccordionItem[] className?: string } @@ -51,16 +52,18 @@ export const AccordionItem = (props: AccordionItem): React.ReactElement => { } export const Accordion = (props: AccordionProps): React.ReactElement => { - const { bordered, items, className } = props + const { bordered, items, className /* multiselectable */ } = props const [openItems, setOpenState] = useState( items.filter((i) => !!i.expanded).map((i) => i.id) ) + // add multiselectable conditional const classes = classnames( 'usa-accordion', { 'usa-accordion--bordered': bordered, + // {'aria-multiselectable': multiselectable} }, className ) From 80f9e234909ddc47c03fad5b643553902cecc464 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Thu, 18 Feb 2021 14:19:33 -0800 Subject: [PATCH 2/9] Committing comments so I can merge changes from main --- src/components/Accordion/Accordion.tsx | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index 1d408c66bd..68c16eb229 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -12,7 +12,7 @@ interface AccordionItem { interface AccordionProps { bordered?: boolean - /* multiselectable: boolean */ + multiselectable?: boolean items: AccordionItem[] className?: string } @@ -52,32 +52,44 @@ export const AccordionItem = (props: AccordionItem): React.ReactElement => { } export const Accordion = (props: AccordionProps): React.ReactElement => { - const { bordered, items, className /* multiselectable */ } = props + const { bordered, items, className, multiselectable } = props const [openItems, setOpenState] = useState( items.filter((i) => !!i.expanded).map((i) => i.id) ) - // add multiselectable conditional const classes = classnames( 'usa-accordion', { 'usa-accordion--bordered': bordered, - // {'aria-multiselectable': multiselectable} + 'aria-multiselectable': multiselectable, }, className ) const toggleItem = (itemId: AccordionItem['id']): void => { const newOpenItems = [...openItems] + console.log('newOpenItems: ', newOpenItems) const itemIndex = openItems.indexOf(itemId) + const isMultiselectable = multiselectable if (itemIndex > -1) { + // item is open, it EXISTS in the array, close it by removing it from openItems newOpenItems.splice(itemIndex, 1) } else { - newOpenItems.push(itemId) + // item is closed and multiselectable is TRUE + // we want to be able to toggle multiple items open + // so we open item by adding it to openItems + if (isMultiselectable) { + newOpenItems.push(itemId) + } else { + // item is closed and multiselectable is FALSE + // so we open item, adding it to openItems + newOpenItems.push(itemId) + // close all other openItems that are not the current item by removing them from the array + // but how + } } - setOpenState(newOpenItems) } From df7822510f0b5c7c2cfa6d405a68c2f685126ff1 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Thu, 18 Feb 2021 15:09:36 -0800 Subject: [PATCH 3/9] Setting multiselectable prop to false as default, changed const to let for shallow copy of openItems variable in order to implement default behavior of one item expanded at a time, comments for tests to be done --- src/components/Accordion/Accordion.test.tsx | 12 ++++++++++++ src/components/Accordion/Accordion.tsx | 14 +++----------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/components/Accordion/Accordion.test.tsx b/src/components/Accordion/Accordion.test.tsx index 6a7dcc489b..ccba4a5c70 100644 --- a/src/components/Accordion/Accordion.test.tsx +++ b/src/components/Accordion/Accordion.test.tsx @@ -81,6 +81,18 @@ const testItems = [ }, ] +describe('multiselectable', () => { + // + // it.todo(‘default behavior: if an item is open and another item is clicked, the open item should close’) + // + // it.todo('default behavior: if multiselectable is false') + // + // it.todo('if multiselectable is set as true, if an item is open and another item is clicked, both items are open') + // + // it.todo + // +}) + describe('Accordion component', () => { it('renders without errors', () => { const { queryByTestId } = render() diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index 68c16eb229..7dfa6c24d2 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -52,7 +52,7 @@ export const AccordionItem = (props: AccordionItem): React.ReactElement => { } export const Accordion = (props: AccordionProps): React.ReactElement => { - const { bordered, items, className, multiselectable } = props + const { bordered, items, className, multiselectable = false } = props const [openItems, setOpenState] = useState( items.filter((i) => !!i.expanded).map((i) => i.id) @@ -68,26 +68,18 @@ export const Accordion = (props: AccordionProps): React.ReactElement => { ) const toggleItem = (itemId: AccordionItem['id']): void => { - const newOpenItems = [...openItems] - console.log('newOpenItems: ', newOpenItems) + let newOpenItems = [...openItems] const itemIndex = openItems.indexOf(itemId) const isMultiselectable = multiselectable if (itemIndex > -1) { - // item is open, it EXISTS in the array, close it by removing it from openItems newOpenItems.splice(itemIndex, 1) } else { - // item is closed and multiselectable is TRUE - // we want to be able to toggle multiple items open - // so we open item by adding it to openItems if (isMultiselectable) { newOpenItems.push(itemId) } else { - // item is closed and multiselectable is FALSE - // so we open item, adding it to openItems + newOpenItems = [] newOpenItems.push(itemId) - // close all other openItems that are not the current item by removing them from the array - // but how } } setOpenState(newOpenItems) From 896f5e34535c1ed982151e59c29563000abe5436 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Thu, 18 Feb 2021 17:21:56 -0800 Subject: [PATCH 4/9] Add multiselectable example to Storybook --- src/components/Accordion/Accordion.stories.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/Accordion/Accordion.stories.tsx b/src/components/Accordion/Accordion.stories.tsx index a17f4d783a..857dbe1863 100644 --- a/src/components/Accordion/Accordion.stories.tsx +++ b/src/components/Accordion/Accordion.stories.tsx @@ -99,6 +99,10 @@ export const bordered = (): React.ReactElement => ( ) +export const multiselectable = (): React.ReactElement => ( + +) + const customTestItems = [ { title: ( From 9178f2ae303c520aeb5d378e5ca775b5b37e7293 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Thu, 18 Feb 2021 17:44:05 -0800 Subject: [PATCH 5/9] Adding test checking for ability to open (and close) multiple different items simultaneously when multiselectable is true --- src/components/Accordion/Accordion.test.tsx | 53 ++++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/src/components/Accordion/Accordion.test.tsx b/src/components/Accordion/Accordion.test.tsx index ccba4a5c70..66997cf089 100644 --- a/src/components/Accordion/Accordion.test.tsx +++ b/src/components/Accordion/Accordion.test.tsx @@ -81,18 +81,6 @@ const testItems = [ }, ] -describe('multiselectable', () => { - // - // it.todo(‘default behavior: if an item is open and another item is clicked, the open item should close’) - // - // it.todo('default behavior: if multiselectable is false') - // - // it.todo('if multiselectable is set as true, if an item is open and another item is clicked, both items are open') - // - // it.todo - // -}) - describe('Accordion component', () => { it('renders without errors', () => { const { queryByTestId } = render() @@ -163,6 +151,47 @@ describe('Accordion component', () => { }) }) + describe('if multiselectable is true', () => { + it('when an item is open and another item is clicked, both items are open', () => { + const { getByText, getByTestId } = render( + + ) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + + fireEvent.click(getByText(testItems[0].title)) + fireEvent.click(getByText(testItems[1].title)) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + + fireEvent.click(getByText(testItems[0].title)) + fireEvent.click(getByText(testItems[3].title)) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + + fireEvent.click(getByText(testItems[2].title)) + fireEvent.click(getByText(testItems[4].title)) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).toBeVisible() + }) + }) + describe('with expanded items on mount', () => { const testExpandedItems = [ { From f635e731d06d6b6e3ca2e79e4396d327a3f0b377 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Fri, 19 Feb 2021 13:50:01 -0800 Subject: [PATCH 6/9] Amending test language to better represent what the test is testing, replacing the let I removed with const again and using splice to empty the array for default behavior --- src/components/Accordion/Accordion.test.tsx | 2 +- src/components/Accordion/Accordion.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Accordion/Accordion.test.tsx b/src/components/Accordion/Accordion.test.tsx index 66997cf089..062905abb6 100644 --- a/src/components/Accordion/Accordion.test.tsx +++ b/src/components/Accordion/Accordion.test.tsx @@ -152,7 +152,7 @@ describe('Accordion component', () => { }) describe('if multiselectable is true', () => { - it('when an item is open and another item is clicked, both items are open', () => { + it('when an item is opened, previously open items remain open', () => { const { getByText, getByTestId } = render( ) diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index 7dfa6c24d2..9e97eecb5a 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -68,7 +68,7 @@ export const Accordion = (props: AccordionProps): React.ReactElement => { ) const toggleItem = (itemId: AccordionItem['id']): void => { - let newOpenItems = [...openItems] + const newOpenItems = [...openItems] const itemIndex = openItems.indexOf(itemId) const isMultiselectable = multiselectable @@ -78,7 +78,7 @@ export const Accordion = (props: AccordionProps): React.ReactElement => { if (isMultiselectable) { newOpenItems.push(itemId) } else { - newOpenItems = [] + newOpenItems.splice(0, newOpenItems.length) newOpenItems.push(itemId) } } From 4ca693d987c0928f0d7dd9d066fa4ed0865666e2 Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Fri, 19 Feb 2021 14:05:31 -0800 Subject: [PATCH 7/9] Adding test explicitly testing default behavior where multiselectable is false --- src/components/Accordion/Accordion.test.tsx | 32 ++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/components/Accordion/Accordion.test.tsx b/src/components/Accordion/Accordion.test.tsx index 062905abb6..26337984e3 100644 --- a/src/components/Accordion/Accordion.test.tsx +++ b/src/components/Accordion/Accordion.test.tsx @@ -151,7 +151,37 @@ describe('Accordion component', () => { }) }) - describe('if multiselectable is true', () => { + describe('when multiselectable is false (default behavior)', () => { + it('when an item is opened, clicking a different item closes the previously opened item', () => { + const { getByText, getByTestId } = render() + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + + fireEvent.click(getByText(testItems[3].title)) + fireEvent.click(getByText(testItems[1].title)) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + + fireEvent.click(getByText(testItems[4].title)) + fireEvent.click(getByText(testItems[2].title)) + + expect(getByTestId(`accordionItem_${testItems[0].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[1].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[2].id}`)).toBeVisible() + expect(getByTestId(`accordionItem_${testItems[3].id}`)).not.toBeVisible() + expect(getByTestId(`accordionItem_${testItems[4].id}`)).not.toBeVisible() + }) + }) + + describe('when multiselectable is true', () => { it('when an item is opened, previously open items remain open', () => { const { getByText, getByTestId } = render( From ca87b1991aaac65578e184842b9312ddb24a1f1e Mon Sep 17 00:00:00 2001 From: Arianna Kellogg Date: Mon, 8 Mar 2021 15:03:39 -0800 Subject: [PATCH 8/9] Make multiselectable an attribute passed to the Accordion's parent div instead of a className --- src/components/Accordion/Accordion.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index 9e97eecb5a..e2a8e8b4a7 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -62,7 +62,6 @@ export const Accordion = (props: AccordionProps): React.ReactElement => { 'usa-accordion', { 'usa-accordion--bordered': bordered, - 'aria-multiselectable': multiselectable, }, className ) @@ -86,7 +85,10 @@ export const Accordion = (props: AccordionProps): React.ReactElement => { } return ( -
+
{items.map((item, i) => ( Date: Tue, 9 Mar 2021 12:06:40 -0800 Subject: [PATCH 9/9] Add logic so aria-multiselectable is not set if multiselectable is false to match USWDS syntax --- src/components/Accordion/Accordion.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Accordion/Accordion.tsx b/src/components/Accordion/Accordion.tsx index e2a8e8b4a7..a1a6553730 100644 --- a/src/components/Accordion/Accordion.tsx +++ b/src/components/Accordion/Accordion.tsx @@ -88,7 +88,7 @@ export const Accordion = (props: AccordionProps): React.ReactElement => {
+ aria-multiselectable={multiselectable || undefined}> {items.map((item, i) => (