From 6b884b88e0e0d46ff10a83fbbd91060764a7a304 Mon Sep 17 00:00:00 2001 From: Nik Savchenko Date: Thu, 31 Mar 2022 12:19:57 +0300 Subject: [PATCH] feat(accordion): add data-testid, improve ids, add interaction tests (#617) * feat(accordion): add data-testid prop - add data-testid - reformat component * refactor(accordion): minor code conventions (lint) refactoring - remove redundant const * fix(accordion): improve AccordionItem/ExpandCollapse ids - ExpandCollapse component have to get prop id for correct A11y (aria-controls) - AccordionItem now getting id on base of Accordion-Id and Index - If Accordion-Id not specified we are taking default COMPONENT_ID * test(accordion): add interaction tests for single-active mode - add interactions tests - connect tests to storybook - minor fix: remove unused import * test(accordion): sync snapshot according to last changes - data-testid - ids improvements * test(accordion): check that aria-expanded of the first item is false - add additional expectation - after switch to another item: aria-expanded of the first item is false - according to review feedback --- .../Accordion/Accordion/Accordion.jsx | 112 +++++++++++------- .../__stories__/accordion.stories.mdx | 4 +- .../accordion-snapshot-tests.jest.js.snap | 57 +++++---- .../__tests__/accordion.interactions.js | 55 +++++++++ .../Accordion/AccordionItem/AccordionItem.jsx | 8 +- .../accordionItem-snapshot-tests.jest.js.snap | 39 +++--- 6 files changed, 190 insertions(+), 85 deletions(-) create mode 100644 src/components/Accordion/Accordion/__tests__/accordion.interactions.js diff --git a/src/components/Accordion/Accordion/Accordion.jsx b/src/components/Accordion/Accordion/Accordion.jsx index 101f88e3bc..bd9adc3fd2 100644 --- a/src/components/Accordion/Accordion/Accordion.jsx +++ b/src/components/Accordion/Accordion/Accordion.jsx @@ -4,62 +4,77 @@ import cx from "classnames"; import useMergeRefs from "../../../hooks/useMergeRefs"; import "./Accordion.scss"; -const Accordion = forwardRef(({ children: originalChildren, allowMultiple, defaultIndex, className, id }, ref) => { - const componentRef = useRef(null); - const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); +const COMPONENT_ID = "monday-accordion"; - const [expandedItems, setExpandedItems] = useState(defaultIndex); +function defineChildId(index, props, accordionId) { + if (props.id) { + return props.id; + } + if (accordionId) { + return `${accordionId}--item-${index}`; + } + return `${COMPONENT_ID}--item-${index}`; +} - const children = useMemo(() => React.Children.toArray(originalChildren), [originalChildren]); +const Accordion = forwardRef( + ({ children: originalChildren, allowMultiple, "data-testid": dataTestId, defaultIndex, className, id }, ref) => { + const componentRef = useRef(null); + const mergedRef = useMergeRefs({ refs: [ref, componentRef] }); - const isChildExpanded = useCallback( - itemIndex => { - return expandedItems.includes(itemIndex); - }, - [expandedItems] - ); + const [expandedItems, setExpandedItems] = useState(defaultIndex); - const onChildClick = useCallback( - itemIndex => { - if (allowMultiple) { - const newExpandedItems = [...expandedItems]; - if (isChildExpanded(itemIndex)) { - const index = newExpandedItems.indexOf(itemIndex); - if (index > -1) { - newExpandedItems.splice(index, 1); + const children = useMemo(() => React.Children.toArray(originalChildren), [originalChildren]); + + const isChildExpanded = useCallback( + itemIndex => { + return expandedItems.includes(itemIndex); + }, + [expandedItems] + ); + + const onChildClick = useCallback( + itemIndex => { + if (allowMultiple) { + const newExpandedItems = [...expandedItems]; + if (isChildExpanded(itemIndex)) { + const index = newExpandedItems.indexOf(itemIndex); + if (index > -1) { + newExpandedItems.splice(index, 1); + } + } else { + newExpandedItems.push(itemIndex); } - } else { - newExpandedItems.push(itemIndex); + setExpandedItems(newExpandedItems); + return; } - setExpandedItems(newExpandedItems); - return; - } - setExpandedItems([itemIndex]); - }, - [isChildExpanded, expandedItems, allowMultiple] - ); + setExpandedItems([itemIndex]); + }, + [isChildExpanded, expandedItems, allowMultiple] + ); - const renderChildElements = useMemo(() => { - const childElements = React.Children.map(children, (child, itemIndex) => { - return React.cloneElement(child, { - ...child?.props, - onClickAccordionCallback: () => { - onChildClick(itemIndex); - }, - open: isChildExpanded(itemIndex) + const renderChildElements = useMemo(() => { + return React.Children.map(children, (child, itemIndex) => { + const originalProps = { ...child?.props }; + const childId = defineChildId(itemIndex, originalProps, id); + return React.cloneElement(child, { + ...originalProps, + id: childId, + onClickAccordionCallback: () => { + onChildClick(itemIndex); + }, + open: isChildExpanded(itemIndex) + }); }); - }); - - return childElements; - }, [isChildExpanded, onChildClick, children]); + }, [children, id, isChildExpanded, onChildClick]); - return ( -
- {children && renderChildElements} -
- ); -}); + return ( +
+ {children && renderChildElements} +
+ ); + } +); Accordion.propTypes = { /** @@ -78,6 +93,10 @@ Accordion.propTypes = { * Array of initial expanded indexes */ defaultIndex: PropTypes.array, + /** + * Unique TestId - can be used as Selector for integration tests and other needs (tracking, etc.) + */ + "data-testid": PropTypes.string, /** * The value of the expandable section */ @@ -89,6 +108,7 @@ Accordion.defaultProps = { id: undefined, allowMultiple: false, children: null, + "data-testid": COMPONENT_ID, defaultIndex: [] }; diff --git a/src/components/Accordion/Accordion/__stories__/accordion.stories.mdx b/src/components/Accordion/Accordion/__stories__/accordion.stories.mdx index 1cdbf4dcd2..7cf425c32c 100644 --- a/src/components/Accordion/Accordion/__stories__/accordion.stories.mdx +++ b/src/components/Accordion/Accordion/__stories__/accordion.stories.mdx @@ -1,10 +1,10 @@ import Accordion from "../Accordion"; import AccordionItem from "../../AccordionItem/AccordionItem"; import { ArgsTable, Story, Canvas, Meta } from "@storybook/addon-docs"; -import { createComponentTemplate } from "../../../../storybook/functions/create-component-story"; import Checkbox from "../../../Checkbox/Checkbox.js"; import { WIZARD, BREADCRUBMS, STEPPER } from "../../../../storybook/components/related-components/component-description-map"; import "./accordion.stories.scss"; +import { accordionSingleActivePlaySuite } from "../__tests__/accordion.interactions" - +
diff --git a/src/components/Accordion/Accordion/__tests__/__snapshots__/accordion-snapshot-tests.jest.js.snap b/src/components/Accordion/Accordion/__tests__/__snapshots__/accordion-snapshot-tests.jest.js.snap index f09cc734c9..d3200bd0f2 100644 --- a/src/components/Accordion/Accordion/__tests__/__snapshots__/accordion-snapshot-tests.jest.js.snap +++ b/src/components/Accordion/Accordion/__tests__/__snapshots__/accordion-snapshot-tests.jest.js.snap @@ -3,19 +3,21 @@ exports[`Accordion renders correctly with allowMultiple 1`] = `