Skip to content

Commit

Permalink
fix collapsible transition behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
Spencer Canner committed Feb 18, 2021
1 parent 6864ea5 commit 483b08a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 11 deletions.
1 change: 1 addition & 0 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
- Fixed accessibility issue on `Tabs` disclosure popover on close ([#3994](https://github.com/Shopify/polaris-react/pull/3994))
- Fixed accessibility issue when tabbing into `IndexTable` ([#4004](https://github.com/Shopify/polaris-react/pull/4004))
- Fixed an issue where inline code would be hard to select ([#4005](https://github.com/Shopify/polaris-react/pull/4005))
- Fixed `Collapsible` bug where animation complete logic was being prematurely triggered by transitions in the children ([#4000](https://github.com/Shopify/polaris-react/pull/4000))

### Documentation

Expand Down
27 changes: 16 additions & 11 deletions src/components/Collapsible/Collapsible.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function Collapsible({
const [height, setHeight] = useState(0);
const [isOpen, setIsOpen] = useState(open);
const [animationState, setAnimationState] = useState<AnimationState>('idle');
const collapisbleContainer = useRef<HTMLDivElement>(null);
const collapsibleContainer = useRef<HTMLDivElement>(null);

const isFullyOpen = animationState === 'idle' && open && isOpen;
const isFullyClosed = animationState === 'idle' && !open && !isOpen;
Expand All @@ -59,10 +59,15 @@ export function Collapsible({
},
};

const handleCompleteAnimation = useCallback(() => {
setAnimationState('idle');
setIsOpen(open);
}, [open]);
const handleCompleteAnimation = useCallback(
({target}: React.TransitionEvent<HTMLDivElement>) => {
if (target === collapsibleContainer.current) {
setAnimationState('idle');
setIsOpen(open);
}
},
[open],
);

useEffect(() => {
if (open !== isOpen) {
Expand All @@ -71,32 +76,32 @@ export function Collapsible({
}, [open, isOpen]);

useEffect(() => {
if (!open || !collapisbleContainer.current) return;
if (!open || !collapsibleContainer.current) return;
// If collapsible defaults to open, set an initial height
setHeight(collapisbleContainer.current.scrollHeight);
setHeight(collapsibleContainer.current.scrollHeight);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
if (!collapisbleContainer.current) return;
if (!collapsibleContainer.current) return;

switch (animationState) {
case 'idle':
break;
case 'measuring':
setHeight(collapisbleContainer.current.scrollHeight);
setHeight(collapsibleContainer.current.scrollHeight);
setAnimationState('animating');
break;
case 'animating':
setHeight(open ? collapisbleContainer.current.scrollHeight : 0);
setHeight(open ? collapsibleContainer.current.scrollHeight : 0);
}
}, [animationState, open, isOpen]);

return (
<div
id={id}
style={collapsibleStyles}
ref={collapisbleContainer}
ref={collapsibleContainer}
className={wrapperClassName}
onTransitionEnd={handleCompleteAnimation}
aria-expanded={open}
Expand Down
40 changes: 40 additions & 0 deletions src/components/Collapsible/tests/Collapsible.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
// eslint-disable-next-line no-restricted-imports
import {mountWithAppProvider} from 'test-utilities/legacy';
import {mountWithApp} from 'test-utilities/react-testing';
import {Tokens} from 'utilities/theme';

import {Collapsible} from '../Collapsible';
Expand Down Expand Up @@ -84,4 +85,43 @@ describe('<Collapsible />', () => {
expect(collapsible.props()).toMatchObject({transition: {timingFunction}});
});
});

describe('onTransitionEnd', () => {
it('adds an isFullyClosed class to the collapsible onTransitionEnd if the event target is the collapsible div', () => {
const id = 'test-collapsible';
const collapsible = mountWithApp(<Collapsible id={id} open />);
collapsible.setProps({open: false});

const wrapper = collapsible.find('div', {id})!;
wrapper.trigger('onTransitionEnd', {
target: wrapper.domNode as HTMLDivElement,
});

expect(
collapsible.find('div', {
id,
'aria-expanded': false,
className: 'Collapsible isFullyClosed',
}),
).not.toBeNull();
});

it('does not add an isFullyClosed class to the collapsible onTransitionEnd if the event target is not the collapsible div', () => {
const id = 'test-collapsible';
const collapsible = mountWithApp(<Collapsible id={id} open />);
collapsible.setProps({open: false});

collapsible.find('div', {id})!.trigger('onTransitionEnd', {
target: document.createElement('div'),
});

expect(
collapsible.find('div', {
id,
'aria-expanded': false,
className: 'Collapsible',
}),
).not.toBeNull();
});
});
});

0 comments on commit 483b08a

Please sign in to comment.