Skip to content

Commit

Permalink
fix(Offcanvas): prevent children mounting twice on show when not resp…
Browse files Browse the repository at this point in the history
…onsive (#6416)
  • Loading branch information
kyletsang committed Aug 1, 2022
1 parent f66cf79 commit e2c4eeb
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 3 deletions.
9 changes: 8 additions & 1 deletion src/NavbarOffcanvas.tsx
Expand Up @@ -9,7 +9,14 @@ const NavbarOffcanvas = React.forwardRef<HTMLDivElement, NavbarOffcanvasProps>(
(props, ref) => {
const context = useContext(NavbarContext);

return <Offcanvas ref={ref} show={!!context?.expanded} {...props} />;
return (
<Offcanvas
ref={ref}
show={!!context?.expanded}
{...props}
renderStaticNode
/>
);
},
);

Expand Down
19 changes: 17 additions & 2 deletions src/Offcanvas.tsx
Expand Up @@ -45,6 +45,7 @@ export interface OffcanvasProps
scroll?: boolean;
placement?: OffcanvasPlacement;
responsive?: 'sm' | 'md' | 'lg' | 'xl' | 'xxl' | string;
renderStaticNode?: boolean;
}

const propTypes = {
Expand Down Expand Up @@ -176,6 +177,13 @@ const propTypes = {
*/
container: PropTypes.any,

/**
* For internal use to render static node from NavbarOffcanvas.
*
* @private
*/
renderStaticNode: PropTypes.bool,

'aria-labelledby': PropTypes.string,
};

Expand All @@ -188,6 +196,7 @@ const defaultProps: Partial<OffcanvasProps> = {
enforceFocus: true,
restoreFocus: true,
placement: 'start',
renderStaticNode: false,
};

function DialogTransition(props) {
Expand Down Expand Up @@ -231,6 +240,7 @@ const Offcanvas: BsPrefixRefForwardingComponent<'div', OffcanvasProps> =
onExited,
backdropClassName,
manager: propsManager,
renderStaticNode,
...props
},
ref,
Expand Down Expand Up @@ -317,9 +327,14 @@ const Offcanvas: BsPrefixRefForwardingComponent<'div', OffcanvasProps> =
<>
{/*
Only render static elements when offcanvas isn't shown so we
don't duplicate elements
don't duplicate elements.
TODO: Should follow bootstrap behavior and don't unmount children
when show={false} in BaseModal. Will do this next major version.
*/}
{!showOffcanvas && renderDialog({})}
{!showOffcanvas &&
(responsive || renderStaticNode) &&
renderDialog({})}

<ModalContext.Provider value={modalContext}>
<BaseModal
Expand Down
29 changes: 29 additions & 0 deletions test/OffcanvasSpec.tsx
@@ -1,4 +1,5 @@
import * as React from 'react';
import { useEffect } from 'react';
import { expect } from 'chai';
import ModalManager from '@restart/ui/ModalManager';
import { fireEvent, render } from '@testing-library/react';
Expand Down Expand Up @@ -286,4 +287,32 @@ describe('<Offcanvas>', () => {
const offcanvasElem = getByTestId('test');
expect(offcanvasElem.getAttribute('role')).to.not.exist;
});

it('should not mount, unmount and mount content on show', () => {
const InnerComponent = ({ onMount, onUnmount }) => {
useEffect(() => {
onMount();
return () => {
onUnmount();
};
}, []);

return <div>Content</div>;
};

const onMountSpy = sinon.spy();
const onUnmountSpy = sinon.spy();

const { unmount } = render(
<Offcanvas data-testid="test" onHide={noop} show>
<InnerComponent onMount={onMountSpy} onUnmount={onUnmountSpy} />
</Offcanvas>,
);

onMountSpy.callCount.should.equal(1);

unmount();

onUnmountSpy.callCount.should.equal(1);
});
});

0 comments on commit e2c4eeb

Please sign in to comment.