feat(router): optional router in withAnnotatorContext and withAnnotations (#4136)

rafalmaksymiuk · mergify[bot] · web-flow · commit d08545ef0650 · 2025-06-17T12:26:21.000Z
* feat: optional router in withAnnotatorContext and withAnnotations

* feat: tests updated, some converted to RTL

* feat: added some missing flow types

* feat: flow types update

* feat: updates after review

* feat: Small updates after review

* feat: Small updates after review

---------

Co-authored-by: mergify[bot] &lt;37929162+mergify[bot]@users.noreply.github.com&gt;
diff --git a/src/elements/common/annotator-context/__tests__/withAnnotations.test.tsx b/src/elements/common/annotator-context/__tests__/withAnnotations.test.tsx
@@ -3,6 +3,7 @@ import { shallow, ShallowWrapper } from 'enzyme';
 import { createMemoryHistory, History, Location } from 'history';
 import { Action, Annotator, AnnotatorContext, withAnnotations, Status } from '../index';
 import { WithAnnotationsProps, ComponentWithAnnotations } from '../withAnnotations';
+import { FeedEntryType, ViewType } from '../../types/SidebarNavigation';
 
 type ComponentProps = {
     className?: string;
@@ -49,6 +50,37 @@ describe('elements/common/annotator-context/withAnnotations', () => {
             const wrapper = getWrapper({ history, location: history.location });
             expect(wrapper.state('activeAnnotationId')).toBe(null);
         });
+
+        test('should use sidebarNavigation prop to initialize state with activeAnnotationId if routerDisabled is true', () => {
+            const wrapper = getWrapper({
+                routerDisabled: true,
+                sidebarNavigation: {
+                    activeFeedEntryType: FeedEntryType.ANNOTATIONS,
+                    activeFeedEntryId: '456',
+                    sidebar: ViewType.ACTIVITY,
+                    fileVersionId: '123',
+                },
+            });
+            expect(wrapper.state('activeAnnotationId')).toBe('456');
+        });
+
+        test('should use sidebarNavigation prop to initialize state with activeAnnotationId if routerDisabled is true in feature flags', () => {
+            const wrapper = getWrapper({
+                features: { routerDisabled: { value: true } },
+                sidebarNavigation: {
+                    activeFeedEntryType: FeedEntryType.ANNOTATIONS,
+                    activeFeedEntryId: '456',
+                    sidebar: ViewType.ACTIVITY,
+                    fileVersionId: '123',
+                },
+            });
+            expect(wrapper.state('activeAnnotationId')).toBe('456');
+        });
+
+        test('should not initialize state with activeAnnotationId if routerDisabled is true and sidebarNavigation is not provided', () => {
+            const wrapper = getWrapper({ routerDisabled: true });
+            expect(wrapper.state('activeAnnotationId')).toBe(null);
+        });
     });
 
     test('should pass onAnnotator and onPreviewDestroy as props on the wrapped component', () => {
@@ -93,6 +125,20 @@ describe('elements/common/annotator-context/withAnnotations', () => {
         });
     });
 
+    test('should not pass annotations router props if routerDisabled is true', () => {
+        const wrapper = getWrapper({ routerDisabled: true });
+        const contextProvider = getContextProvider(wrapper);
+        expect(contextProvider.prop('value').getAnnotationsMatchPath).toBeUndefined();
+        expect(contextProvider.prop('value').getAnnotationsPath).toBeUndefined();
+    });
+
+    test('should not pass annotations router props if routerDisabled is true in feature flags', () => {
+        const wrapper = getWrapper({ features: { routerDisabled: { value: true } } });
+        const contextProvider = getContextProvider(wrapper);
+        expect(contextProvider.prop('value').getAnnotationsMatchPath).toBeUndefined();
+        expect(contextProvider.prop('value').getAnnotationsPath).toBeUndefined();
+    });
+
     describe('emitActiveAnnotationChangeEvent', () => {
         test('should call annotator emit on action', () => {
             const wrapper = getWrapper();
diff --git a/src/elements/common/annotator-context/__tests__/withAnnotatorContext.test.tsx b/src/elements/common/annotator-context/__tests__/withAnnotatorContext.test.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { shallow } from 'enzyme';
+import { render } from '../../../../test-utils/testing-library';
 import withAnnotatorContext, { WithAnnotatorContextProps } from '../withAnnotatorContext';
 import { Action } from '../types';
 
@@ -13,18 +13,59 @@ describe('elements/common/annotator-context/withAnnotatorContext', () => {
         className?: string | undefined;
     };
 
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    type WrappedComponentProps<T> = ComponentProps & WithAnnotatorContextProps; // T is supposed to be allowed component props
+    type WrappedComponentProps = ComponentProps & WithAnnotatorContextProps;
 
-    const Component = (props: WrappedComponentProps<HTMLDivElement>) => <div {...props} />;
+    beforeEach(() => jest.resetAllMocks());
 
-    const WrappedComponent = withAnnotatorContext(Component);
+    test('should apply the annotator context to the wrapped component as a prop', () => {
+        const annotatorState = {
+            annotation: { foo: 'bar' },
+            action: Action.CREATE_START,
+        };
+        const mockEmitActiveAnnotationChangeEvent = jest.fn();
+        const mockEmitAnnotationRemoveEvent = jest.fn();
+        const mockEmitAnnotationReplyCreateEvent = jest.fn();
+        const mockEmitAnnotationReplyDeleteEvent = jest.fn();
+        const mockMmitAnnotationReplyUpdateEvent = jest.fn();
+        const mockEmitAnnotationUpdateEvent = jest.fn();
+        const mockGetAnnotationsMatchPath = jest.fn();
+        const mockGetAnnotationsPath = jest.fn();
 
-    const getWrapper = (props?: WrappedComponentProps<HTMLDivElement>) => shallow(<WrappedComponent {...props} />);
+        mockContext.mockReturnValue({
+            state: annotatorState,
+            emitActiveAnnotationChangeEvent: mockEmitActiveAnnotationChangeEvent,
+            emitAnnotationRemoveEvent: mockEmitAnnotationRemoveEvent,
+            emitAnnotationReplyCreateEvent: mockEmitAnnotationReplyCreateEvent,
+            emitAnnotationReplyDeleteEvent: mockEmitAnnotationReplyDeleteEvent,
+            emitAnnotationReplyUpdateEvent: mockMmitAnnotationReplyUpdateEvent,
+            emitAnnotationUpdateEvent: mockEmitAnnotationUpdateEvent,
+            getAnnotationsMatchPath: mockGetAnnotationsMatchPath,
+            getAnnotationsPath: mockGetAnnotationsPath,
+        });
 
-    beforeEach(() => jest.resetAllMocks());
+        const MockComponent = jest.fn<JSX.Element | null, [WrappedComponentProps]>(() => null);
+        const WrappedWithMockComponent = withAnnotatorContext(MockComponent);
 
-    test('should apply the annotator context to the wrapped component as a prop', () => {
+        render(<WrappedWithMockComponent />);
+
+        expect(MockComponent).toHaveBeenCalledTimes(1);
+        const props = MockComponent.mock.calls[0][0];
+
+        expect(props.annotatorState).toEqual({
+            annotation: { foo: 'bar' },
+            action: Action.CREATE_START,
+        });
+        expect(props.emitActiveAnnotationChangeEvent).toBe(mockEmitActiveAnnotationChangeEvent);
+        expect(props.emitAnnotationRemoveEvent).toBe(mockEmitAnnotationRemoveEvent);
+        expect(props.emitAnnotationReplyCreateEvent).toBe(mockEmitAnnotationReplyCreateEvent);
+        expect(props.emitAnnotationReplyDeleteEvent).toBe(mockEmitAnnotationReplyDeleteEvent);
+        expect(props.emitAnnotationReplyUpdateEvent).toBe(mockMmitAnnotationReplyUpdateEvent);
+        expect(props.emitAnnotationUpdateEvent).toBe(mockEmitAnnotationUpdateEvent);
+        expect(props.getAnnotationsMatchPath).toBe(mockGetAnnotationsMatchPath);
+        expect(props.getAnnotationsPath).toBe(mockGetAnnotationsPath);
+    });
+
+    test('should apply the annotator context to the wrapped component without router props when routerDisabled is true', () => {
         const annotatorState = {
             annotation: { foo: 'bar' },
             action: Action.CREATE_START,
@@ -50,22 +91,77 @@ describe('elements/common/annotator-context/withAnnotatorContext', () => {
             getAnnotationsPath: mockGetAnnotationsPath,
         });
 
-        const wrapper = getWrapper();
-        const wrappedComponent = wrapper.dive().find(Component);
-        const props = wrappedComponent.props() as WrappedComponentProps<HTMLDivElement>;
+        const MockComponent = jest.fn<JSX.Element | null, [WrappedComponentProps]>(() => null);
+        const WrappedWithMockComponent = withAnnotatorContext(MockComponent);
+
+        render(<WrappedWithMockComponent routerDisabled={true} />);
+
+        expect(MockComponent).toHaveBeenCalledTimes(1);
+        const props = MockComponent.mock.calls[0][0];
 
-        expect(wrappedComponent.exists()).toBeTruthy();
         expect(props.annotatorState).toEqual({
             annotation: { foo: 'bar' },
             action: Action.CREATE_START,
         });
-        expect(props.emitActiveAnnotationChangeEvent).toEqual(mockEmitActiveAnnotationChangeEvent);
-        expect(props.emitAnnotationRemoveEvent).toEqual(mockEmitAnnotationRemoveEvent);
-        expect(props.emitAnnotationReplyCreateEvent).toEqual(mockEmitAnnotationReplyCreateEvent);
-        expect(props.emitAnnotationReplyDeleteEvent).toEqual(mockEmitAnnotationReplyDeleteEvent);
-        expect(props.emitAnnotationReplyUpdateEvent).toEqual(mockMmitAnnotationReplyUpdateEvent);
-        expect(props.emitAnnotationUpdateEvent).toEqual(mockEmitAnnotationUpdateEvent);
-        expect(props.getAnnotationsMatchPath).toEqual(mockGetAnnotationsMatchPath);
-        expect(props.getAnnotationsPath).toEqual(mockGetAnnotationsPath);
+        expect(props.emitActiveAnnotationChangeEvent).toBe(mockEmitActiveAnnotationChangeEvent);
+        expect(props.emitAnnotationRemoveEvent).toBe(mockEmitAnnotationRemoveEvent);
+        expect(props.emitAnnotationReplyCreateEvent).toBe(mockEmitAnnotationReplyCreateEvent);
+        expect(props.emitAnnotationReplyDeleteEvent).toBe(mockEmitAnnotationReplyDeleteEvent);
+        expect(props.emitAnnotationReplyUpdateEvent).toBe(mockMmitAnnotationReplyUpdateEvent);
+        expect(props.emitAnnotationUpdateEvent).toBe(mockEmitAnnotationUpdateEvent);
+
+        // Router-related props should not be passed when routerDisabled is true
+        expect(props.getAnnotationsMatchPath).toBeUndefined();
+        expect(props.getAnnotationsPath).toBeUndefined();
+    });
+
+    test('should apply the annotator context to the wrapped component without router props when routerDisabled in feature flags is true', () => {
+        const annotatorState = {
+            annotation: { foo: 'bar' },
+            action: Action.CREATE_START,
+        };
+        const mockEmitActiveAnnotationChangeEvent = jest.fn();
+        const mockEmitAnnotationRemoveEvent = jest.fn();
+        const mockEmitAnnotationReplyCreateEvent = jest.fn();
+        const mockEmitAnnotationReplyDeleteEvent = jest.fn();
+        const mockMmitAnnotationReplyUpdateEvent = jest.fn();
+        const mockEmitAnnotationUpdateEvent = jest.fn();
+        const mockGetAnnotationsMatchPath = jest.fn();
+        const mockGetAnnotationsPath = jest.fn();
+
+        mockContext.mockReturnValue({
+            state: annotatorState,
+            emitActiveAnnotationChangeEvent: mockEmitActiveAnnotationChangeEvent,
+            emitAnnotationRemoveEvent: mockEmitAnnotationRemoveEvent,
+            emitAnnotationReplyCreateEvent: mockEmitAnnotationReplyCreateEvent,
+            emitAnnotationReplyDeleteEvent: mockEmitAnnotationReplyDeleteEvent,
+            emitAnnotationReplyUpdateEvent: mockMmitAnnotationReplyUpdateEvent,
+            emitAnnotationUpdateEvent: mockEmitAnnotationUpdateEvent,
+            getAnnotationsMatchPath: mockGetAnnotationsMatchPath,
+            getAnnotationsPath: mockGetAnnotationsPath,
+        });
+
+        const MockComponent = jest.fn<JSX.Element | null, [WrappedComponentProps]>(() => null);
+        const WrappedWithMockComponent = withAnnotatorContext(MockComponent);
+
+        render(<WrappedWithMockComponent features={{ routerDisabled: { value: true } }} />);
+
+        expect(MockComponent).toHaveBeenCalledTimes(1);
+        const props = MockComponent.mock.calls[0][0];
+
+        expect(props.annotatorState).toEqual({
+            annotation: { foo: 'bar' },
+            action: Action.CREATE_START,
+        });
+        expect(props.emitActiveAnnotationChangeEvent).toBe(mockEmitActiveAnnotationChangeEvent);
+        expect(props.emitAnnotationRemoveEvent).toBe(mockEmitAnnotationRemoveEvent);
+        expect(props.emitAnnotationReplyCreateEvent).toBe(mockEmitAnnotationReplyCreateEvent);
+        expect(props.emitAnnotationReplyDeleteEvent).toBe(mockEmitAnnotationReplyDeleteEvent);
+        expect(props.emitAnnotationReplyUpdateEvent).toBe(mockMmitAnnotationReplyUpdateEvent);
+        expect(props.emitAnnotationUpdateEvent).toBe(mockEmitAnnotationUpdateEvent);
+
+        // Router-related props should not be passed when routerDisabled is true
+        expect(props.getAnnotationsMatchPath).toBeUndefined();
+        expect(props.getAnnotationsPath).toBeUndefined();
     });
 });
diff --git a/src/elements/common/annotator-context/types.js.flow b/src/elements/common/annotator-context/types.js.flow
@@ -74,8 +74,8 @@ export interface AnnotatorContext {
         annotation: Object,
         isStartEvent?: boolean
     ) => void;
-    getAnnotationsMatchPath: GetMatchPath;
-    getAnnotationsPath: (fileVersionId?: string, annotationId?: string) => string;
+    getAnnotationsMatchPath?: GetMatchPath;
+    getAnnotationsPath?: (fileVersionId?: string, annotationId?: string) => string;
     state: AnnotatorState;
 }
 declare export var Status: {|
diff --git a/src/elements/common/annotator-context/types.ts b/src/elements/common/annotator-context/types.ts
@@ -53,8 +53,8 @@ export interface AnnotatorContext {
     emitAnnotationReplyDeleteEvent?: (id: string, annotationId: string, isStartEvent?: boolean) => void;
     emitAnnotationReplyUpdateEvent?: (reply: Object, annotationId: string, isStartEvent?: boolean) => void;
     emitAnnotationUpdateEvent?: (annotation: Object, isStartEvent?: boolean) => void;
-    getAnnotationsMatchPath: GetMatchPath;
-    getAnnotationsPath: (fileVersionId?: string, annotationId?: string) => string;
+    getAnnotationsMatchPath?: GetMatchPath;
+    getAnnotationsPath?: (fileVersionId?: string, annotationId?: string) => string;
     state: AnnotatorState;
 }
 
diff --git a/src/elements/common/annotator-context/withAnnotations.js.flow b/src/elements/common/annotator-context/withAnnotations.js.flow
@@ -18,6 +18,10 @@ import {
     MatchParams,
     Status
 } from "./types";
+import { SidebarNavigation } from '../types/SidebarNavigation';
+import { type FeatureConfig } from '../feature-checking';
+
+
 export type ActiveChangeEvent = {
     annotationId: string | null,
     fileVersionId: string,
@@ -48,11 +52,11 @@ export type ComponentWithAnnotations = {
         isStartEvent?: boolean
     ) => void,
     getAction: (eventData: AnnotationActionEvent) => Action,
-    getAnnotationsPath: (
+    getAnnotationsPath?: (
         fileVersionId?: string,
         annotationId?: string | null
     ) => string,
-    getMatchPath: GetMatchPath,
+    getMatchPath?: GetMatchPath,
     handleActiveChange: ActiveChangeEventHandler,
     handleAnnotationChangeEvent: (id: string | null) => void,
     handleAnnotationCreate: (eventData: AnnotationActionEvent) => void,
@@ -70,6 +74,7 @@ export type ComponentWithAnnotations = {
     ...
 };
 export type WithAnnotationsProps = {
+    features?: FeatureConfig;
     location?: Location,
     onAnnotator: (annotator: Annotator) => void,
     onError?: (
@@ -78,7 +83,9 @@ export type WithAnnotationsProps = {
         contextInfo?: { [key: string]: mixed, ... }
     ) => void,
     onPreviewDestroy: (shouldReset?: boolean) => void,
-    ...
+    routerDisabled?: boolean;
+    sidebarNavigation?: SidebarNavigation;
+     ...
 };
 export type WithAnnotationsComponent<P> = React.ComponentClass<
     P & WithAnnotationsProps
diff --git a/src/elements/common/annotator-context/withAnnotations.tsx b/src/elements/common/annotator-context/withAnnotations.tsx
@@ -3,7 +3,9 @@ import getProp from 'lodash/get';
 import { generatePath, match as matchType, matchPath } from 'react-router-dom';
 import { Location } from 'history';
 import AnnotatorContext from './AnnotatorContext';
+import { isFeatureEnabled, type FeatureConfig } from '../feature-checking';
 import { Action, Annotator, AnnotationActionEvent, AnnotatorState, GetMatchPath, MatchParams, Status } from './types';
+import { FeedEntryType, SidebarNavigation } from '../types/SidebarNavigation';
 
 export type ActiveChangeEvent = {
     annotationId: string | null;
@@ -41,10 +43,13 @@ export type ComponentWithAnnotations = {
 };
 
 export type WithAnnotationsProps = {
+    features?: FeatureConfig;
     location?: Location;
     onAnnotator: (annotator: Annotator) => void;
     onError?: (error: Error, code: string, contextInfo?: Record<string, unknown>) => void;
     onPreviewDestroy: (shouldReset?: boolean) => void;
+    routerDisabled?: boolean;
+    sidebarNavigation?: SidebarNavigation;
 };
 
 export type WithAnnotationsComponent<P> = React.ComponentClass<P & WithAnnotationsProps>;
@@ -71,10 +76,26 @@ export default function withAnnotations<P extends object>(
         constructor(props: P & WithAnnotationsProps) {
             super(props);
 
-            // Determine by url if there is already a deeply linked annotation
-            const { location } = props;
-            const match = this.getMatchPath(location);
-            const activeAnnotationId = getProp(match, 'params.annotationId', null);
+            const { routerDisabled, sidebarNavigation } = props;
+            let activeAnnotationId = null;
+
+            const isRouterDisabled = routerDisabled || isFeatureEnabled(props?.features, 'routerDisabled.value');
+
+            if (isRouterDisabled) {
+                if (
+                    sidebarNavigation &&
+                    'activeFeedEntryType' in sidebarNavigation &&
+                    sidebarNavigation.activeFeedEntryType === FeedEntryType.ANNOTATIONS &&
+                    'activeFeedEntryId' in sidebarNavigation
+                ) {
+                    activeAnnotationId = sidebarNavigation.activeFeedEntryId;
+                }
+            } else {
+                // Determine by url if there is already a deeply linked annotation
+                const { location } = props;
+                const match = this.getMatchPath(location);
+                activeAnnotationId = getProp(match, 'params.annotationId', null);
+            }
 
             // Seed the initial state with the activeAnnotationId if any from the location path
             this.state = { ...defaultState, activeAnnotationId };
@@ -192,6 +213,7 @@ export default function withAnnotations<P extends object>(
             });
         }
 
+        // remove this method with routerDisabled switch
         getMatchPath(location?: Location): matchType<MatchParams> | null {
             const pathname = getProp(location, 'pathname', '');
             return matchPath<MatchParams>(pathname, {
@@ -319,6 +341,14 @@ export default function withAnnotations<P extends object>(
         };
 
         render(): JSX.Element {
+            const isRouterDisabled =
+                this.props?.routerDisabled || isFeatureEnabled(this.props?.features, 'routerDisabled.value');
+            const annotationsRouterProps = isRouterDisabled
+                ? {}
+                : {
+                      getAnnotationsMatchPath: this.getMatchPath,
+                      getAnnotationsPath: this.getAnnotationsPath,
+                  };
             return (
                 <AnnotatorContext.Provider
                     value={{
@@ -328,8 +358,7 @@ export default function withAnnotations<P extends object>(
                         emitAnnotationReplyDeleteEvent: this.emitAnnotationReplyDeleteEvent,
                         emitAnnotationReplyUpdateEvent: this.emitAnnotationReplyUpdateEvent,
                         emitAnnotationUpdateEvent: this.emitAnnotationUpdateEvent,
-                        getAnnotationsMatchPath: this.getMatchPath,
-                        getAnnotationsPath: this.getAnnotationsPath,
+                        ...annotationsRouterProps,
                         state: this.state,
                     }}
                 >
diff --git a/src/elements/common/annotator-context/withAnnotatorContext.js.flow b/src/elements/common/annotator-context/withAnnotatorContext.js.flow
@@ -7,6 +7,7 @@
 import * as React from "react";
 import AnnotatorContext from "./AnnotatorContext";
 import { AnnotatorState, GetMatchPath } from "./types";
+
 export interface WithAnnotatorContextProps {
     annotatorState?: AnnotatorState;
     emitActiveAnnotationChangeEvent?: (id: string) => void;
diff --git a/src/elements/common/annotator-context/withAnnotatorContext.tsx b/src/elements/common/annotator-context/withAnnotatorContext.tsx
diff --git a/src/elements/common/types/SidebarNavigation.flow.js b/src/elements/common/types/SidebarNavigation.flow.js
diff --git a/src/elements/common/types/SidebarNavigation.ts b/src/elements/common/types/SidebarNavigation.ts
