Skip to content

Commit

Permalink
STCOM-1179 BREAKING bump react to v18 (#2084)
Browse files Browse the repository at this point in the history
Short version: use react v18, adjusting tests accordingly.

Long version:

* remove only in paneset test
* Use a better test component name
* fix paneset tests
* remove console logging
* try accordions in isolation
* use BTNG in ExpandAll, await things in accordion test
* accordions - ensure element focused before triggering key
* store accordions in state of accordionSet
* Use react state for registering accordions
* async the keyboard trigger also.
* leapfrog possibly pending state updates via setTimeout. Automated tests, u too fast!
* track registered accordions in state
* converge on focus in the popover tests
* use flushSync with ExpandAllbutton event handler
* return null if accordion is unregistered
* remove the syncflush
* sync-flush is needed in ExpandAll button
* remove setTimeouts
* remove commented code
* remove unused import
* cleanup
* switch css units

Co-authored-by: Zak Burke <zburke@ebsco.com>
  • Loading branch information
JohnC-80 and zburke authored Jul 17, 2023
1 parent 4f7619d commit a8d778a
Show file tree
Hide file tree
Showing 15 changed files with 219 additions and 136 deletions.
6 changes: 3 additions & 3 deletions hooks/tests/useFormatDate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import getHookExecutionResult from '../../tests/helpers/getHookExecutionResult';
import useFormatDate from '../useFormatDate';

describe('useFormatDate', () => {
const hookResult = getHookExecutionResult(useFormatDate);

it('should return correct date', () => {
expect(hookResult('2020-03-24T17:59:57.369+0000')).to.equal('3/24/2020');
getHookExecutionResult(useFormatDate).then(hookResult => {
expect(hookResult('2020-03-24T17:59:57.369+0000')).to.equal('3/24/2020');
});
});
});
6 changes: 3 additions & 3 deletions hooks/tests/useFormatTime-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import getHookExecutionResult from '../../tests/helpers/getHookExecutionResult';
import useFormatTime from '../useFormatTime';

describe('useFormatTime', () => {
const hookResult = getHookExecutionResult(useFormatTime);

it('should return correct time', () => {
expect(hookResult('2020-03-24T17:59:57.369+0000')).to.equal('5:59 PM');
getHookExecutionResult(useFormatTime).then(hookResult => {
expect(hookResult('2020-03-24T17:59:57.369+0000')).to.equal('5:59 PM');
});
});
});
5 changes: 4 additions & 1 deletion lib/Accordion/Accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,11 @@ const Accordion = (props) => {
const trackingId = useRef(id || uniqueId('acc')).current;
const labelId = useRef(`accordion-toggle-button-${trackingId}`).current;


const getRef = useRef(() => toggle.current).current;
const [isOpen, updateOpen] = useState(open || !closedByDefault);
const [focused, updateFocus] = useState(false);
const [registered, updateRegistered] = useState(!Boolean(accordionSet));

const uncontrolledToggle = useRef(() => {
updateOpen(current => !current);
Expand All @@ -125,7 +127,7 @@ const Accordion = (props) => {

useEffect(() => { // eslint-disable-line
if (accordionSet) {
accordionSet.registerAccordion(getRef, trackingId, closedByDefault);
accordionSet.registerAccordion(getRef, trackingId, closedByDefault, updateRegistered);
return () => {
accordionSet.unregisterAccordion(trackingId);
};
Expand All @@ -151,6 +153,7 @@ const Accordion = (props) => {
});
const headerElement = React.createElement(header, accordionHeaderProps);

if (!registered) return null;
return (
<section
id={trackingId}
Expand Down
69 changes: 40 additions & 29 deletions lib/Accordion/AccordionSet.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ class AccordionSet extends React.Component {
this.focusLastAccordion = this.focusLastAccordion.bind(this);
this.unregisterAccordion = this.unregisterAccordion.bind(this);

this.accordionList = [];

this.state = {};
this.state = {
status: {},
accordionList: []
};

this.toggleKeyHandlers = {
nextAccordion: this.focusNextAccordion,
Expand All @@ -55,76 +56,86 @@ class AccordionSet extends React.Component {

componentDidUpdate(prevProps, prevState) {
if (
Object.keys(prevState).length !== Object.keys(this.state).length) {
if (this.innerStatus.current) this.innerStatus.current.setStatus(this.state);
Object.keys(prevState).length !== Object.keys(this.state.status).length) {
if (this.innerStatus.current) this.innerStatus.current.setStatus(this.state.status);
}
}

focusNextAccordion(e) {
const currentAccordionIndex = this.accordionList.findIndex(a => e.target.id.includes(a.id));
const currentAccordionIndex = this.state.accordionList.findIndex(a => e.target.id.includes(a.id));
if (currentAccordionIndex !== -1) {
if (this.accordionList.length >= currentAccordionIndex + 2) {
this.accordionList[currentAccordionIndex + 1].getRef().focus();
}
if (currentAccordionIndex === this.accordionList.length - 1) {
if (this.state.accordionList.length >= currentAccordionIndex + 2) {
this.state.accordionList[currentAccordionIndex + 1].getRef().focus();
} else if (currentAccordionIndex === this.state.accordionList.length - 1) {
this.focusFirstAccordion();
}
}
}

focusPreviousAccordion(e) {
const currentAccordionIndex = this.accordionList.findIndex(a => e.target.id.includes(a.id));
const currentAccordionIndex = this.state.accordionList.findIndex(a => e.target.id.includes(a.id));
if (currentAccordionIndex !== -1) {
if (currentAccordionIndex > 0) {
const prevRef = this.accordionList[currentAccordionIndex - 1].getRef();
const prevRef = this.state.accordionList[currentAccordionIndex - 1].getRef();
prevRef.focus();
}
if (currentAccordionIndex === 0) {
} else if (currentAccordionIndex === 0) {
this.focusLastAccordion();
}
}
}

focusFirstAccordion() {
this.accordionList[0].getRef().focus();
this.state.accordionList[0].getRef().focus();
}

focusLastAccordion() {
this.accordionList[this.accordionList.length - 1].getRef().focus();
this.state.accordionList[this.state.accordionList.length - 1].getRef().focus();
}

registerAccordion = (getRef, id, closedByDefault = false) => {
registerAccordion = (getRef, id, closedByDefault = false, confirm) => {
const { initialStatus } = this.props;

const status = initialStatus || {};
if (indexOf(this.accordionList, id) === -1 && id !== null) {
this.accordionList.push({ id, getRef });
if (indexOf(this.state.accordionList, id) === -1 && id !== null) {
const statusUpdater = this.props.setStatus || this.setState.bind(this);
statusUpdater((curState) => (
{
...curState,
[id]: typeof status[id] !== 'undefined' ? status[id] : !closedByDefault
}
));
this.setState(cur => {
return { accordionList: [...cur.accordionList, { id, getRef }] }
}, () => {
confirm(true);
statusUpdater((curState) => {
const newState = {
...curState,
status: {
...curState.status,
[id]: typeof status[id] !== 'undefined' ? status[id] : !closedByDefault
}
};
return newState;
});
});
}
}

unregisterAccordion = (id) => {
const accordionIndex = this.accordionList.findIndex(accordion => accordion.id === id);
const accordionIndex = this.state.accordionList.findIndex(accordion => accordion.id === id);
if (accordionIndex === -1) {
return;
}

this.accordionList.splice(accordionIndex, 1);
this.setState(cur => {
const newList = cur.accordionList.splice(accordionIndex, 1);
return { accordionList: newList }
});

const statusUpdater = this.props.setStatus || this.setState.bind(this);
statusUpdater((curState) => {
delete curState[id];
delete curState.status[id];
return curState;
});

// eslint-disable-next-line
this.innerStatus.current?.setStatus(current => {
delete current[id];
delete current.status[id];
return current;
});
}
Expand Down
27 changes: 19 additions & 8 deletions lib/Accordion/AccordionStatus.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,39 @@ export default class AccordionStatus extends React.Component {
constructor(props) {
super(props);

this.state = props.accordionStatus || props.initialStatus || {};
this.state = {
status: props.accordionStatus || props.initialStatus || {}
};
}

static getDerivedStateFromProps(props) {
return props.accordionStatus ? props.accordionStatus : null;
return props.accordionStatus ? { status: props.accordionStatus } : null;
}

setStatus = fn => {
this.setState(fn);
if (typeof fn === 'function') {
this.setState(cur => fn(cur));
} else {
this.setState({ status: fn });
}
};

onToggle = ({ id }) => {
this.setStatus(current => ({
...current,
[id]: !current[id]
}));
this.setStatus(current => {
const newState = {
status: {
...current.status,
[id]: !current.status[id]
}
};
return newState;
});
};

render() {
const { children } = this.props;
const provided = {
status: this.state,
status: this.state.status,
setStatus: this.setStatus,
onToggle: this.onToggle
};
Expand Down
7 changes: 6 additions & 1 deletion lib/Accordion/ExpandAllButton.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import { flushSync } from 'react-dom';
import PropTypes from 'prop-types';
import { FormattedMessage } from 'react-intl';
import isEqual from 'lodash/isEqual';
Expand Down Expand Up @@ -27,7 +28,11 @@ const propTypes = {

function handleClick({ accordionStatus, func, setStatus, onToggle }) {
const newState = expandAll(accordionStatus, func);
if (setStatus) setStatus(newState);
if (setStatus) {
// we want the button change to be snappy, so we go ahead and push
// the state update to immediate mode vs React v18's batching.
flushSync(() => setStatus(newState));
}
if (onToggle) onToggle(newState);
}

Expand Down
56 changes: 36 additions & 20 deletions lib/Accordion/tests/Accordion-test.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import React from 'react';
import { describe, beforeEach, it } from 'mocha';
import { expect } from 'chai';
import { Accordion as Interactor, Keyboard, runAxeTest } from '@folio/stripes-testing';
import { Accordion as Interactor, Keyboard, runAxeTest, Button } from '@folio/stripes-testing';
import sinon from 'sinon';
import { mountWithContext } from '../../../tests/helpers';

import Accordion from '../Accordion';
import AccordionSet from '../AccordionSet';

const Focused = Button.extend('Accordion header')
.selector(':focus')
.actions({
pressKey: async ({ perform }, key) => {
await perform(async (e) => await Keyboard.pressKey(key))
}
});

describe('Accordion', () => {
const accordion = Interactor('test');

Expand Down Expand Up @@ -65,20 +73,22 @@ describe('Accordion', () => {
it('closes the accordion', () => accordion.is({ open: false }));

it('calls onClickToggle with correct "open" state', async () => {
expect(onClickToggleSpy.calledWith({
await expect(onClickToggleSpy.calledWith({
id: 'test-id',
label: 'test',
open: false,
})).to.be.true;
});

describe('clicking the header again', () => {
beforeEach(() => accordion.clickHeader());
beforeEach(async () => {
await accordion.clickHeader()
});

it('opens the accordion', () => accordion.is({ open: true }));

it('calls onClickToggle with correct "open" state', async () => {
expect(onClickToggleSpy.calledWith({
await expect(onClickToggleSpy.calledWith({
id: 'test-id',
label: 'test',
open: true,
Expand All @@ -103,12 +113,16 @@ describe('Accordion', () => {
it('is closed by default', () => accordion.is({ open: false }));

describe('clicking the header', () => {
beforeEach(() => accordion.clickHeader());
beforeEach(async () => {
await accordion.clickHeader();
});

it('opens the accordion', () => accordion.is({ open: true }));

describe('clicking the header again', () => {
beforeEach(() => accordion.clickHeader());
beforeEach(async () => {
await accordion.clickHeader()
});

it('closes the accordion', () => accordion.is({ open: false }));
});
Expand Down Expand Up @@ -182,25 +196,26 @@ describe('Accordion', () => {
});
});
});
});

describe('as part of an AccordionSet', () => {
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>
<Accordion label="test">
<Accordion label="test1" id="test1">
<input aria-label="test1" />
</Accordion>
<Accordion label="test">
<Accordion label="test2" id="test2">
<input aria-label="test2" />
</Accordion>
<Accordion label="test">
<Accordion label="test3" id="test3">
<input aria-label="test3" />
</Accordion>
<Accordion label="test">
<Accordion label="test4" id="test4">
<input aria-label="test4" />
</Accordion>
</AccordionSet>
Expand All @@ -209,40 +224,41 @@ describe('Accordion', () => {

it('has no axe errors', () => runAxeTest);

it('has a button', () => Button('test1').exists());

describe('keyboard navigation: next accordion', () => {
beforeEach(async () => {
await first.focus();
await Keyboard.arrowDown();
await Focused('test1').pressKey('ArrowDown');
});

it('second accordion is in focus', () => second.is({ focused: true }));
it('second accordion is in focus', async () => await second.is({ focused: true }));
});

describe('keyboard navigation: previous accordion', () => {
beforeEach(async () => {
await second.focus();
await Keyboard.arrowUp();
await Focused('test2').pressKey('ArrowUp');
});

it('first accordion is in focus', () => first.is({ focused: true }));
it('first accordion is in focus', async () => await first.is({ focused: true }));
});

describe('keyboard navigation: last accordion', () => {
beforeEach(async () => {
await first.focus();
await Keyboard.end();
await Focused('test1').pressKey('End');
});

it('Last accordion is in focus', () => last.is({ focused: true }));
it('Last accordion is in focus', async () => await last.is({ focused: true }));
});

describe('keyboard navigation: first accordion', () => {
beforeEach(async () => {
await last.focus();
await Keyboard.home();
await Focused('test4').pressKey('Home');
});

it('First accordion is in focus', () => first.is({ focused: true }));
it('First accordion is in focus', async () => await first.is({ focused: true }));
});
});
});
Loading

0 comments on commit a8d778a

Please sign in to comment.