diff --git a/src/elements/content-sidebar/SidebarPanels.js b/src/elements/content-sidebar/SidebarPanels.js index 916ff3a9e0..4d2b0359f2 100644 --- a/src/elements/content-sidebar/SidebarPanels.js +++ b/src/elements/content-sidebar/SidebarPanels.js @@ -6,7 +6,7 @@ import * as React from 'react'; import flow from 'lodash/flow'; -import { Redirect, Route, Switch } from 'react-router-dom'; +import { matchPath, Redirect, Route, Switch, type Location } from 'react-router-dom'; import SidebarUtils from './SidebarUtils'; import withSidebarAnnotations from './withSidebarAnnotations'; import { withAnnotatorContext } from '../common/annotator-context'; @@ -47,6 +47,7 @@ type Props = { hasSkills: boolean, hasVersions: boolean, isOpen: boolean, + location: Location, metadataSidebarProps: MetadataSidebarProps, onAnnotationSelect?: Function, onVersionChange?: Function, @@ -87,6 +88,8 @@ const LoadableVersionsSidebar = SidebarUtils.getAsyncSidebarContent( MARK_NAME_JS_LOADING_VERSIONS, ); +const SIDEBAR_PATH_VERSIONS = '/:sidebar(activity|details)/versions/:versionId?'; + class SidebarPanels extends React.Component { activitySidebar: ElementRefType = React.createRef(); @@ -102,6 +105,21 @@ class SidebarPanels extends React.Component { this.setState({ isInitialized: true }); } + componentDidUpdate(prevProps: Props): void { + const { location, onVersionChange } = this.props; + const { location: prevLocation } = prevProps; + + // Reset the current version id if the wrapping versions route is no longer active + if (onVersionChange && this.getVersionsMatchPath(prevLocation) && !this.getVersionsMatchPath(location)) { + onVersionChange(null); + } + } + + getVersionsMatchPath = (location: Location) => { + const { pathname } = location; + return matchPath(pathname, SIDEBAR_PATH_VERSIONS); + }; + /** * Refreshes the contents of the active sidebar * @returns {void} @@ -250,7 +268,7 @@ class SidebarPanels extends React.Component { )} {hasVersions && ( ( { const getWrapper = ({ path = '/', ...rest } = {}) => mount( - - - , + , + { + wrappingComponent: MemoryRouter, + wrappingComponentProps: { + initialEntries: [path], + keyLength: 0, + }, + }, ); describe('render', () => { @@ -144,4 +149,38 @@ describe('elements/content-sidebar/SidebarPanels', () => { expect(instance.versionsSidebar.current.refresh).toHaveBeenCalledWith(); }); }); + + describe('componentDidUpdate', () => { + const onVersionChange = jest.fn(); + + test.each([ + ['/activity/versions/123', '/activity/versions/456'], + ['/activity/versions/123', '/details/versions/456'], + ['/activity/versions', '/activity/versions/123'], + ['/activity/versions', '/details/versions'], + ])('should not reset the current version if the versions route is still active', (prevPathname, pathname) => { + const wrapper = getWrapper({ location: { pathname: prevPathname }, onVersionChange }); + wrapper.setProps({ location: { pathname } }); + expect(onVersionChange).not.toBeCalled(); + }); + + test.each([true, false])('should not reset the current version if the sidebar is toggled', isOpen => { + const wrapper = getWrapper({ isOpen, location: { pathname: '/details/versions/123' }, onVersionChange }); + wrapper.setProps({ isOpen: !isOpen }); + expect(onVersionChange).not.toBeCalled(); + }); + + test.each([ + ['/activity/versions/123', '/metadata'], + ['/activity/versions/123', '/activity'], + ['/activity/versions', '/metadata'], + ['/details/versions/123', '/metadata'], + ['/details/versions/123', '/details'], + ['/details/versions', '/metadata'], + ])('should reset the current version if the versions route is no longer active', (prevPathname, pathname) => { + const wrapper = getWrapper({ location: { pathname: prevPathname }, onVersionChange }); + wrapper.setProps({ location: { pathname } }); + expect(onVersionChange).toBeCalledWith(null); + }); + }); }); diff --git a/src/elements/content-sidebar/versions/VersionsSidebarContainer.js b/src/elements/content-sidebar/versions/VersionsSidebarContainer.js index 175a773d11..c8d7596a95 100644 --- a/src/elements/content-sidebar/versions/VersionsSidebarContainer.js +++ b/src/elements/content-sidebar/versions/VersionsSidebarContainer.js @@ -95,11 +95,6 @@ class VersionsSidebarContainer extends React.Component { } } - componentWillUnmount() { - // Reset the current version id since the wrapping route is no longer active - this.props.onVersionChange(null); - } - handleActionDelete = (versionId: string): Promise => { this.setState({ isLoading: true }); diff --git a/src/elements/content-sidebar/versions/__tests__/VersionsSidebarContainer.test.js b/src/elements/content-sidebar/versions/__tests__/VersionsSidebarContainer.test.js index 6d7f2c26d5..6178d9efad 100644 --- a/src/elements/content-sidebar/versions/__tests__/VersionsSidebarContainer.test.js +++ b/src/elements/content-sidebar/versions/__tests__/VersionsSidebarContainer.test.js @@ -58,17 +58,6 @@ describe('elements/content-sidebar/versions/VersionsSidebarContainer', () => { }); }); - describe('componentWillUnmount', () => { - test('should forward verison id reset to the parent component', () => { - const onVersionChange = jest.fn(); - const wrapper = getWrapper({ onVersionChange }); - - wrapper.instance().componentWillUnmount(); - - expect(onVersionChange).toBeCalledWith(null); - }); - }); - describe('componentDidMount', () => { test('should call onLoad after a successful fetchData() call', async () => { const onLoad = jest.fn();