From 0cc833aa342f481f672000c63b60d8f331828392 Mon Sep 17 00:00:00 2001 From: Denys Bohdan <denys_bohdan@epam.com> Date: Mon, 9 Oct 2023 11:47:17 +0200 Subject: [PATCH] STCOM-1210 Fix state mutation in `<AccordionSet>`. Add `onRegisterAccordion` and `onUnregisterAccordion` props. (#2138) * STCOM-1210 Fix state mutation in `<AccordionSet>`. Add `onRegisterAccordion` and `onUnregisterAccordion` props. * STCOM-1210 Added test for <AccordionSet> onRegisterAccordion prop * STCOM-1210 Added test for <AccordionSet> onUnregisterAccordion prop --- CHANGELOG.md | 1 + lib/Accordion/AccordionSet.js | 25 +++- lib/Accordion/readme.md | 4 + lib/Accordion/tests/Accordion-test.js | 201 ++++++++++++++++---------- 4 files changed, 150 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 469fcc981..3abc5aa0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ * Update/unbreak `expandAllSections` and `collapseAllSections` keyboard shortcuts to work with updated `<AccordionStatus>` code. Fixes STCOM-1207. * Correctly handle multiple `<Callout>` elements when they are manipulated quickly. Refs STCOM-1209. * Make Advanced search query boxes expandable. Fixes STCOM-1205. +* Fix state mutation in `<AccordionSet>`. Add `onRegisterAccordion` and `onUnregisterAccordion` props. Refs STCOM-1210. ## [11.0.0](https://github.com/folio-org/stripes-components/tree/v11.0.0) (2023-01-30) [Full Changelog](https://github.com/folio-org/stripes-components/compare/v10.3.0...v11.0.0) diff --git a/lib/Accordion/AccordionSet.js b/lib/Accordion/AccordionSet.js index 9b21a098e..ca5c4e6c7 100644 --- a/lib/Accordion/AccordionSet.js +++ b/lib/Accordion/AccordionSet.js @@ -1,6 +1,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import indexOf from 'lodash/indexOf'; +import omit from 'lodash/omit'; +import noop from 'lodash/noop'; + import { Provider as AccProvider } from './AccordionSetContext'; import AccordionStatus from './AccordionStatus'; import { @@ -15,12 +18,16 @@ class AccordionSet extends React.Component { id: PropTypes.string, inheritStatus: PropTypes.bool, initialStatus: PropTypes.object, + onRegisterAccordion: PropTypes.func, onToggle: PropTypes.func, + onUnregisterAccordion: PropTypes.func, setStatus: PropTypes.func, } static defaultProps = { - inheritStatus: true + inheritStatus: true, + onRegisterAccordion: noop, + onUnregisterAccordion: noop, } constructor(props) { @@ -114,6 +121,8 @@ class AccordionSet extends React.Component { }); }); } + + this.props.onRegisterAccordion(id); } unregisterAccordion = (id) => { @@ -129,15 +138,21 @@ class AccordionSet extends React.Component { const statusUpdater = this.props.setStatus || this.setState.bind(this); statusUpdater((curState) => { - delete curState.status[id]; - return curState; + return { + ...curState, + status: omit(curState.status, [id]), + }; }); // eslint-disable-next-line this.innerStatus.current?.setStatus(current => { - delete current.status[id]; - return current; + return { + ...current, + status: omit(current.status, [id]), + }; }); + + this.props.onUnregisterAccordion(id); } render() { diff --git a/lib/Accordion/readme.md b/lib/Accordion/readme.md index 0db2c6eb0..46f808830 100644 --- a/lib/Accordion/readme.md +++ b/lib/Accordion/readme.md @@ -72,6 +72,10 @@ const initial = { ... ``` +### `onRegisterAccordion` and `onUnregisterAccordion` props +The `onRegisterAccordion` and `onUnregisterAccordion` props are called when an `<Accordion>` is registered and unregistered. This is useful when accordions can be mounted or unmounted and you want to notify a consumer of `<AccordionStatus>` when that has happened. +The props are called with a single argument - accordion's `id`. + ## Open render-prop Accordions can pass a their open status to their children via a functional child: diff --git a/lib/Accordion/tests/Accordion-test.js b/lib/Accordion/tests/Accordion-test.js index fe6cd7e1d..973f44c3d 100644 --- a/lib/Accordion/tests/Accordion-test.js +++ b/lib/Accordion/tests/Accordion-test.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useState } from 'react'; import { describe, beforeEach, it } from 'mocha'; import { expect } from 'chai'; import { Accordion as Interactor, Keyboard, Bigtest, runAxeTest, Button } from '@folio/stripes-testing'; @@ -13,7 +13,7 @@ const Focused = Button.extend('Accordion header') .selector(':focus') .actions({ pressKey: async ({ perform }, key) => { - await perform(async (e) => await Keyboard.pressKey(key)) + await perform(async () => Keyboard.pressKey(key)) } }); @@ -199,111 +199,160 @@ describe('Accordion', () => { }); }); - describe('Accordion - as part of an AccordionSet', () => { - const first = Interactor({ index: 0 }); - const second = Interactor({ index: 1 }); - const last = Interactor({ index: 3 }); +describe('Accordion - as part of an AccordionSet', () => { + const first = Interactor({ index: 0 }); + const second = Interactor({ index: 1 }); + const last = Interactor({ index: 3 }); - beforeEach(async () => { - await mountWithContext( - <AccordionSet initialStatus={{ + const onRegisterAccordionSpy = sinon.spy(); + + beforeEach(async () => { + await mountWithContext( + <AccordionSet + initialStatus={{ test1: false, test2: true, test3: true, test4: true, - }}> - <Accordion label="test1" id="test1"> - <input aria-label="test1" type="text" id="testControl1" /> - </Accordion> - <Accordion label="test2" id="test2"> - <input aria-label="test2" /> - </Accordion> - <Accordion label="test3" id="test3"> - <input aria-label="test3" /> - </Accordion> - <Accordion label="test4" id="test4"> - <input aria-label="test4" /> - </Accordion> - </AccordionSet> - ); - }); + }} + onRegisterAccordion={onRegisterAccordionSpy} + > + <Accordion label="test1" id="test1"> + <input aria-label="test1" type="text" id="testControl1" /> + </Accordion> + <Accordion label="test2" id="test2"> + <input aria-label="test2" /> + </Accordion> + <Accordion label="test3" id="test3"> + <input aria-label="test3" /> + </Accordion> + <Accordion label="test4" id="test4"> + <input aria-label="test4" /> + </Accordion> + </AccordionSet> + ); + }); - it('has no axe errors', () => runAxeTest); + it('has no axe errors', () => runAxeTest); - it('has a button', () => Button('test1').exists()); + it('has a button', () => Button('test1').exists()); - describe('contents ready for interaction following open', () => { - beforeEach(async () => { - await first.clickHeader(); - await Bigtest.TextField({id: 'testControl1', }).fillIn("test"); - }); + it('should call onRegisterAccordion callback', () => { + return expect(onRegisterAccordionSpy.calledWith('test1')).to.be.true; + }); - it('Child element was filled out successfully', () => Bigtest.TextField({id: 'testControl1'}).has({ value: "test" })); + describe('contents ready for interaction following open', () => { + beforeEach(async () => { + await first.clickHeader(); + await Bigtest.TextField({ id: 'testControl1' }).fillIn('test'); }); - describe('keyboard navigation: next accordion', () => { - beforeEach(async () => { - await first.focus(); - await Focused('test1').pressKey('ArrowDown'); - }); + it('Child element was filled out successfully', () => Bigtest.TextField({id: 'testControl1'}).has({ value: 'test' })); + }); - it('second accordion is in focus', async () => await second.is({ focused: true })); + describe('keyboard navigation: next accordion', () => { + beforeEach(async () => { + await first.focus(); + await Focused('test1').pressKey('ArrowDown'); }); - describe('keyboard navigation: previous accordion', () => { - beforeEach(async () => { - await second.focus(); - await Focused('test2').pressKey('ArrowUp'); - }); + it('second accordion is in focus', async () => second.is({ focused: true })); + }); - it('first accordion is in focus', async () => await first.is({ focused: true })); + describe('keyboard navigation: previous accordion', () => { + beforeEach(async () => { + await second.focus(); + await Focused('test2').pressKey('ArrowUp'); }); - describe('keyboard navigation: last accordion', () => { - beforeEach(async () => { - await first.focus(); - await Focused('test1').pressKey('End'); - }); + it('first accordion is in focus', async () => first.is({ focused: true })); + }); - it('Last accordion is in focus', async () => await last.is({ focused: true })); + describe('keyboard navigation: last accordion', () => { + beforeEach(async () => { + await first.focus(); + await Focused('test1').pressKey('End'); }); - describe('keyboard navigation: first accordion', () => { - beforeEach(async () => { - await last.focus(); - await Focused('test4').pressKey('Home'); - }); + it('Last accordion is in focus', async () => last.is({ focused: true })); + }); - it('First accordion is in focus', async () => await first.is({ focused: true })); + describe('keyboard navigation: first accordion', () => { + beforeEach(async () => { + await last.focus(); + await Focused('test4').pressKey('Home'); }); + + it('First accordion is in focus', async () => first.is({ focused: true })); + }); +}); + +describe('unmounting Accordion - as part of an AccordionSet', () => { + const onUnregisterAccordionSpy = sinon.spy(); + + const Wrapper = () => { + const [renderAccordion, setRenderAccordion] = useState(true); + + return ( + <> + <AccordionSet + initialStatus={{ + test1: true, + }} + onUnregisterAccordion={onUnregisterAccordionSpy} + > + {renderAccordion && ( + <Accordion label="test1" id="test1"> + <input aria-label="test1" type="text" id="testControl1" /> + </Accordion> + )} + </AccordionSet> + <button type="button" onClick={() => setRenderAccordion(false)}>Unmount accordion</button> + </> + ); + }; + + const first = Interactor({ index: 0 }); + const unmountAccordionButton = Button('Unmount accordion'); + + beforeEach(async () => { + await mountWithContext(<Wrapper />); + }); + + it('should call onUnregisterAccordion callback', async () => { + await first.is({ open: true }); + await unmountAccordionButton.click(); + + return expect(onUnregisterAccordionSpy.calledWith('test1')).to.be.true; + }); +}); + +describe('Accordion - updating state of parent component', () => { + const first = Interactor({ index: 0 }); + const second = Interactor({ index: 1 }); + const last = Interactor({ index: 3 }); + const textField = Bigtest.TextField({ id: 'testTextField' }); + beforeEach(async () => { + await mountWithContext(<UsageWithAccordionSet />) }); - describe('Accordion - updating state of parent component', () => { - const first = Interactor({ index: 0 }); - const second = Interactor({ index: 1 }); - const last = Interactor({ index: 3 }); - const textField = Bigtest.TextField({ id: 'testTextField' }); + it('renders second accordion as closed', () => second.is({ open: false })); + it('renders a blank textfield', () => textField.has({ value: ''})); + + describe('opening the closed accordion', () => { beforeEach(async () => { - await mountWithContext(<UsageWithAccordionSet/>) + await second.clickHeader(); }); - it('renders second accordion as closed', () => second.is({ open: false })); - it('renders a blank textfield', () => textField.has({ value: ''})); + it('renders second accordion as open', () => second.is({ open: true })); - describe('opening the closed accordion', () => { + describe('changing the text value/parent state', () => { beforeEach(async () => { - await second.clickHeader(); + await textField.fillIn('test'); }); + it('renders state value in textField', () => textField.has({ value: 'test' })); it('renders second accordion as open', () => second.is({ open: true })); - - describe('changing the text value/parent state', () => { - beforeEach(async () => { - await textField.fillIn('test'); - }); - - it('renders state value in textField', () => textField.has({ value: 'test' })); - it('renders second accordion as open', () => second.is({ open: true })); - }); }); }); +});