fix(preview-sidebar): fix preview versions behavior

Author: Machoper

src/constants.js

@@ -394,6 +394,9 @@ export const SIDEBAR_VIEW_METADATA: 'metadata' = 'metadata';
 export const SIDEBAR_VIEW_ACTIVITY: 'activity' = 'activity';
 export const SIDEBAR_VIEW_VERSIONS: 'versions' = 'versions';
 
+/* ------------------ Sidebar Path ---------------------- */
+export const SIDEBAR_PATH_VERSIONS = '/:sidebar(activity|details)/versions/:versionId?';
+
 /* ------------------ HTTP Requests ---------------------- */
 export const HTTP_GET: 'GET' = 'GET';
 export const HTTP_POST: 'POST' = 'POST';

src/elements/content-sidebar/Sidebar.js

@@ -10,13 +10,14 @@ import flow from 'lodash/flow';
 import getProp from 'lodash/get';
 import noop from 'lodash/noop';
 import uniqueid from 'lodash/uniqueId';
-import { withRouter } from 'react-router-dom';
+import { matchPath, withRouter } from 'react-router-dom';
 import type { Location, RouterHistory } from 'react-router-dom';
 import LoadingIndicator from '../../components/loading-indicator/LoadingIndicator';
 import LocalStore from '../../utils/LocalStore';
 import SidebarNav from './SidebarNav';
 import SidebarPanels from './SidebarPanels';
 import SidebarUtils from './SidebarUtils';
+import { SIDEBAR_PATH_VERSIONS } from '../../constants';
 import { withCurrentUser } from '../common/current-user';
 import { withFeatureConsumer } from '../common/feature-checking';
 import type { FeatureConfig } from '../common/feature-checking';
@@ -97,7 +98,7 @@ class Sidebar extends React.Component<Props, State> {
     }
 
     componentDidUpdate(prevProps: Props): void {
-        const { fileId, history, location }: Props = this.props;
+        const { fileId, history, location, onVersionChange }: Props = this.props;
         const { fileId: prevFileId, location: prevLocation }: Props = prevProps;
         const { isDirty }: State = this.state;
 
@@ -111,8 +112,18 @@ class Sidebar extends React.Component<Props, State> {
             this.setForcedByLocation();
             this.setState({ isDirty: true });
         }
+
+        // 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 = getProp(location, 'pathname', '');
+        return matchPath(pathname, SIDEBAR_PATH_VERSIONS);
+    };
+
     getUrlPrefix = (pathname: string) => {
         const basePath = pathname.substring(1).split('/')[0];
         return basePath;

src/elements/content-sidebar/SidebarPanels.js

@@ -18,6 +18,7 @@ import {
     ORIGIN_METADATA_SIDEBAR,
     ORIGIN_SKILLS_SIDEBAR,
     ORIGIN_VERSIONS_SIDEBAR,
+    SIDEBAR_PATH_VERSIONS,
     SIDEBAR_VIEW_ACTIVITY,
     SIDEBAR_VIEW_DETAILS,
     SIDEBAR_VIEW_METADATA,
@@ -250,7 +251,7 @@ class SidebarPanels extends React.Component<Props, State> {
                 )}
                 {hasVersions && (
                     <Route
-                        path="/:sidebar(activity|details)/versions/:versionId?"
+                        path={SIDEBAR_PATH_VERSIONS}
                         render={({ match }) => (
                             <LoadableVersionsSidebar
                                 fileId={fileId}

src/elements/content-sidebar/__tests__/Sidebar.test.js

@@ -74,6 +74,23 @@ describe('elements/content-sidebar/Sidebar', () => {
             wrapper.setProps({ location: { pathname: '/', state: { open: false } } });
             expect(instance.isForced).toHaveBeenCalledWith(false);
         });
+
+        test('should reset the current version if the versions route is no longer active', () => {
+            const onVersionChange = jest.fn();
+            const wrapper = getWrapper({ location: { pathname: '/activity' }, onVersionChange });
+
+            wrapper.setProps({ location: { pathname: '/activity/versions/123', state: { open: true } } });
+            expect(onVersionChange).not.toBeCalled();
+
+            wrapper.setProps({ location: { pathname: '/activity/versions/123', state: { open: false } } });
+            expect(onVersionChange).not.toBeCalled();
+
+            wrapper.setProps({ location: { pathname: '/activity/versions/456', state: { open: true } } });
+            expect(onVersionChange).not.toBeCalled();
+
+            wrapper.setProps({ location: { pathname: '/details' } });
+            expect(onVersionChange).lastCalledWith(null);
+        });
     });
 
     describe('handleVersionHistoryClick', () => {

src/elements/content-sidebar/versions/VersionsSidebarContainer.js

@@ -95,11 +95,6 @@ class VersionsSidebarContainer extends React.Component<Props, State> {
         }
     }
 
-    componentWillUnmount() {
-        // Reset the current version id since the wrapping route is no longer active
-        this.props.onVersionChange(null);
-    }
-
     handleActionDelete = (versionId: string): Promise<void> => {
         this.setState({ isLoading: true });
 

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();