fix(video-annotations): fix video annotation version change issue (#4380)

bfoxx1906 · web-flow · commit 121de79d76a6 · 2025-12-09T22:27:28.000Z
* fix(video-annotations): fix issue with video annotations not working on version switching

* fix(video-annotations): fixing unit tests

* fix(video-annotations): fixing version change issue with video annoations

* fix(video-annotations): refactoring

* feat(video-annotations): refactor

* feat(video-annotations): refactor

* feat(video-annotations): fix flow type

* feat(video-annotations): addressing pr comments

* feat(video-annotations): addressing PR comments

* feat(video-annotation): fix flow error

* feat(video-annotations): addressing PR comments

* feat(video-annotations): fixing flow errors

* feat(video-annotations): addressing PR comment

* feat(video-annotations): fixng tests
diff --git a/src/elements/content-preview/ContentPreview.js b/src/elements/content-preview/ContentPreview.js
@@ -61,6 +61,7 @@ import {
     ERROR_CODE_UNKNOWN,
 } from '../../constants';
 import type { Annotation } from '../../common/types/feed';
+import type { Target } from '../../common/types/annotations';
 import type { TargetingApi } from '../../features/targeting/types';
 import type { ErrorType, AdditionalVersionInfo } from '../common/flowTypes';
 import type { WithLoggerProps } from '../../common/types/logging';
@@ -193,6 +194,7 @@ const startAtTypes = {
 const InvalidIdError = new Error('Invalid id for Preview!');
 const PREVIEW_LOAD_METRIC_EVENT = 'load';
 const MARK_NAME_JS_READY = `${ORIGIN_CONTENT_PREVIEW}_${EVENT_JS_READY}`;
+const SCROLL_TO_ANNOTATION_EVENT = 'scrolltoannotation';
 
 mark(MARK_NAME_JS_READY);
 
@@ -228,6 +230,8 @@ class ContentPreview extends React.PureComponent<Props, State> {
 
     updateVersionToCurrent: ?() => void;
 
+    dynamicOnPreviewLoadAction: ?() => void;
+
     initialState: State = {
         canPrint: false,
         error: undefined,
@@ -734,6 +738,10 @@ class ContentPreview extends React.PureComponent<Props, State> {
         }
 
         this.handleCanPrint();
+
+        if (this.dynamicOnPreviewLoadAction) {
+            this.dynamicOnPreviewLoadAction();
+        }
     };
 
     /**
@@ -847,6 +855,7 @@ class ContentPreview extends React.PureComponent<Props, State> {
         const { Preview } = global.Box;
         this.preview = new Preview();
         this.preview.addListener('load', this.onPreviewLoad);
+
         this.preview.addListener('preview_error', this.onPreviewError);
         this.preview.addListener('preview_metric', this.onPreviewMetric);
         this.preview.addListener('thumbnailsOpen', () => this.setState({ isThumbnailSidebarOpen: true }));
@@ -1209,7 +1218,40 @@ class ContentPreview extends React.PureComponent<Props, State> {
         });
     };
 
-    handleAnnotationSelect = ({ file_version, id, target }: Annotation) => {
+    emitScrollToAnnotation = (id: string, target: Target) => {
+        const newViewer = this.getViewer();
+        // $FlowFixMe - Flow doesn't support optional chaining with method calls
+        newViewer?.emit(SCROLL_TO_ANNOTATION_EVENT, { id, target });
+    };
+
+    /**
+     * Handles scrolling to a frame-based annotation by waiting for video player to load first
+     *
+     * @param {string} id - The annotation ID
+     * @param {object} target - The annotation target
+     * @return {void}
+     */
+    scrollToFrameAnnotation = (id: string, target: Target): void => {
+        // $FlowFixMe: querySelector('video') returns an HTMLVideoElement
+        const videoPlayer: HTMLVideoElement = document.querySelector('.bp-media-container video');
+        if (!videoPlayer) {
+            this.dynamicOnPreviewLoadAction = null;
+            return;
+        }
+
+        const videoReadyToScroll = videoPlayer.readyState === 4;
+        if (videoReadyToScroll) {
+            this.emitScrollToAnnotation(id, target);
+            return;
+        }
+        const handleLoadedData = () => {
+            this.emitScrollToAnnotation(id, target);
+            videoPlayer.removeEventListener('loadeddata', handleLoadedData);
+        };
+        videoPlayer.addEventListener('loadeddata', handleLoadedData);
+    };
+
+    handleAnnotationSelect = ({ file_version, id, target }: Annotation, deferScrollToOnload: boolean = false) => {
         const { location = {} } = target;
         const { file, selectedVersion } = this.state;
         const annotationFileVersionId = getProp(file_version, 'id');
@@ -1227,8 +1269,19 @@ class ContentPreview extends React.PureComponent<Props, State> {
             });
         }
 
-        if (viewer) {
-            viewer.emit('scrolltoannotation', { id, target });
+        if (viewer && !deferScrollToOnload) {
+            viewer.emit(SCROLL_TO_ANNOTATION_EVENT, { id, target });
+        } else if (viewer && deferScrollToOnload) {
+            this.dynamicOnPreviewLoadAction = () => {
+                if (target?.location?.type === 'frame') {
+                    this.scrollToFrameAnnotation(id, target);
+                } else {
+                    const newViewer = this.getViewer();
+                    // $FlowFixMe - Flow doesn't support optional chaining with method calls
+                    newViewer?.emit(SCROLL_TO_ANNOTATION_EVENT, { id, target });
+                }
+                this.dynamicOnPreviewLoadAction = null;
+            };
         }
     };
 
diff --git a/src/elements/content-preview/__tests__/ContentPreview.test.js b/src/elements/content-preview/__tests__/ContentPreview.test.js
@@ -791,7 +791,7 @@ describe('elements/content-preview/ContentPreview', () => {
         beforeEach(() => {
             props = {
                 collection: [{}, {}],
-                fileId: file.id,
+                fileId: 123,
                 onLoad: jest.fn(),
                 token: 'token',
             };
@@ -824,6 +824,16 @@ describe('elements/content-preview/ContentPreview', () => {
             instance.onPreviewLoad(data);
             expect(instance.state.isLoading).toBe(false);
         });
+
+        test('should call dynamicOnPreviewLoadAction if it is defined', () => {
+            instance.dynamicOnPreviewLoadAction = jest.fn();
+            instance.onPreviewLoad(data);
+            expect(instance.dynamicOnPreviewLoadAction).toBeCalled();
+        });
+
+        test('should not call dynamicOnPreviewLoadAction if it is not defined', () => {
+            expect(() => instance.onPreviewLoad(data)).not.toThrow();
+        });
     });
 
     describe('onPreviewMetric()', () => {
@@ -1285,6 +1295,7 @@ describe('elements/content-preview/ContentPreview', () => {
             annotationFileVersionId | selectedVersionId | locationType | setStateCount
             ${'123'}                | ${'124'}          | ${'page'}    | ${1}
             ${'124'}                | ${'124'}          | ${'page'}    | ${0}
+            ${'123'}                | ${'124'}          | ${'frame'}   | ${0}
             ${'123'}                | ${'124'}          | ${''}        | ${0}
             ${undefined}            | ${'124'}          | ${'page'}    | ${0}
         `(
@@ -1317,6 +1328,112 @@ describe('elements/content-preview/ContentPreview', () => {
                 expect(emit).toBeCalledWith('scrolltoannotation', { id: annotation.id, target: annotation.target });
             },
         );
+
+        test.each`
+            annotationFileVersionId | selectedVersionId | locationType | deferScrollToOnload
+            ${'123'}                | ${'124'}          | ${'frame'}   | ${true}
+            ${'123'}                | ${'124'}          | ${'page'}    | ${false}
+        `(
+            'should not call emit scrolltoannotation if deferScrollToOnload is $deferScrollToOnload',
+            ({ annotationFileVersionId, selectedVersionId, locationType, deferScrollToOnload }) => {
+                const annotation = {
+                    id: '123',
+                    file_version: { id: annotationFileVersionId },
+                    target: { location: { type: locationType } },
+                };
+                const wrapper = getWrapper();
+                const instance = wrapper.instance();
+                wrapper.setState({ selectedVersion: { id: selectedVersionId } });
+                const emit = jest.fn();
+                jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+                instance.setState = jest.fn();
+
+                instance.handleAnnotationSelect(annotation, deferScrollToOnload);
+                if (deferScrollToOnload) {
+                    expect(emit).not.toBeCalled();
+                    expect(instance.dynamicOnPreviewLoadAction).toBeDefined();
+                } else {
+                    expect(emit).toBeCalledWith('scrolltoannotation', { id: annotation.id, target: annotation.target });
+                    expect(instance.dynamicOnPreviewLoadAction).not.toBeDefined();
+                }
+            },
+        );
+        test('should set dynamicOnPreviewLoadAction and execute scrollToFrameAnnotation properly if deferScrollToOnload is true', () => {
+            const annotation = {
+                id: '123',
+                file_version: { id: '123' },
+                target: { location: { type: 'frame' } },
+            };
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            wrapper.setState({ selectedVersion: { id: 123 } });
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            instance.setState = jest.fn();
+            instance.handleAnnotationSelect(annotation, true);
+            expect(instance.dynamicOnPreviewLoadAction).toBeDefined();
+            let handleLoadedData;
+            const mockAddEventListener = jest.fn().mockImplementation((listener, callback) => {
+                expect(listener).toBe('loadeddata');
+                expect(callback).toBeDefined();
+                handleLoadedData = callback;
+            });
+            const mockRemoveEventListener = jest.fn().mockImplementation((listener, callback) => {
+                expect(listener).toBe('loadeddata');
+                expect(callback).toEqual(handleLoadedData);
+                handleLoadedData = null;
+            });
+            jest.spyOn(document, 'querySelector').mockReturnValue({
+                addEventListener: mockAddEventListener,
+                removeEventListener: mockRemoveEventListener,
+            });
+            instance.dynamicOnPreviewLoadAction();
+            expect(mockAddEventListener).toBeCalledWith('loadeddata', handleLoadedData);
+            handleLoadedData();
+            expect(emit).toBeCalledWith('scrolltoannotation', { id: annotation.id, target: annotation.target });
+            expect(instance.dynamicOnPreviewLoadAction).toBeNull();
+            expect(mockRemoveEventListener).toHaveBeenCalledTimes(1);
+        });
+
+        test('dynamicOnPreviewLoadAction should return if video player does not exist', () => {
+            const annotation = {
+                id: '123',
+                file_version: { id: '123' },
+                target: { location: { type: 'frame' } },
+            };
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            wrapper.setState({ selectedVersion: { id: 123 } });
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            instance.setState = jest.fn();
+            jest.spyOn(document, 'querySelector').mockReturnValue(null);
+            instance.handleAnnotationSelect(annotation, true);
+            instance.dynamicOnPreviewLoadAction();
+            expect(instance.dynamicOnPreviewLoadAction).toBeNull();
+            expect(emit).not.toBeCalled();
+        });
+
+        test('dynamicOnPreviewLoadAction should still call emit scrolltoannotation if target location type is not frame', () => {
+            const annotation = {
+                id: '123',
+                file_version: { id: '123' },
+                target: { location: { type: 'page' } },
+            };
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            wrapper.setState({ selectedVersion: { id: 123 } });
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            const scrollToFrameAnnotation = jest.fn();
+            jest.spyOn(instance, 'scrollToFrameAnnotation').mockReturnValue(scrollToFrameAnnotation);
+            instance.setState = jest.fn();
+            instance.handleAnnotationSelect(annotation, true);
+            instance.dynamicOnPreviewLoadAction();
+            expect(scrollToFrameAnnotation).not.toBeCalled();
+            expect(emit).toBeCalledWith('scrolltoannotation', { id: annotation.id, target: annotation.target });
+            expect(instance.dynamicOnPreviewLoadAction).toBeNull();
+        });
     });
 
     describe('getThumbnail()', () => {
@@ -1351,4 +1468,77 @@ describe('elements/content-preview/ContentPreview', () => {
             expect(pageThumbnail).toBeNull();
         });
     });
+
+    describe('scrollToFrameAnnotation()', () => {
+        let annotation;
+        let addEventListener;
+        let removeEventListener;
+        let mockVideoPlayer;
+        beforeEach(() => {
+            addEventListener = jest.fn();
+            removeEventListener = jest.fn();
+            mockVideoPlayer = {
+                addEventListener,
+                removeEventListener,
+                readyState: 0,
+            };
+
+            annotation = {
+                id: '123',
+                file_version: { id: '123' },
+                target: { location: { type: 'frame' } },
+            };
+
+            jest.spyOn(document, 'querySelector').mockImplementation(selector => {
+                expect(selector).toBe('.bp-media-container video');
+                return mockVideoPlayer;
+            });
+        });
+
+        afterEach(() => {
+            jest.clearAllMocks();
+        });
+
+        test('should return if the video player is not found', () => {
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            jest.spyOn(document, 'querySelector').mockReturnValue(null);
+            instance.scrollToFrameAnnotation(annotation.id, annotation.target);
+            expect(instance.dynamicOnPreviewLoadAction).toBeNull();
+            expect(emit).not.toBeCalled();
+        });
+
+        test('should emit scrolltoannotation if video player is ready to seek and do not set video player event listener', () => {
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            mockVideoPlayer.readyState = 4;
+            instance.scrollToFrameAnnotation(annotation.id, annotation.target);
+            expect(emit).toBeCalledWith('scrolltoannotation', { id: annotation.id, target: annotation.target });
+            expect(mockVideoPlayer.addEventListener).not.toBeCalled();
+        });
+
+        test('should not throw an error if the video player is ready to seek and the viewer is not found', () => {
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            jest.spyOn(instance, 'getViewer').mockReturnValue(null);
+            mockVideoPlayer.readyState = 4;
+            instance.scrollToFrameAnnotation(annotation.id, annotation.target);
+        });
+
+        test('should add loadeddata event listener to video player if video player is not ready to seek', () => {
+            const wrapper = getWrapper();
+            const instance = wrapper.instance();
+            wrapper.setState({ selectedVersion: { id: 123 } });
+            const emit = jest.fn();
+            jest.spyOn(instance, 'getViewer').mockReturnValue({ emit });
+            mockVideoPlayer.readyState = 0;
+            instance.scrollToFrameAnnotation(annotation.id, annotation.target);
+            expect(emit).not.toBeCalled();
+            expect(addEventListener).toBeCalledWith('loadeddata', expect.any(Function));
+        });
+    });
 });
diff --git a/src/elements/content-sidebar/ActivitySidebar.js b/src/elements/content-sidebar/ActivitySidebar.js
@@ -1153,9 +1153,12 @@ class ActivitySidebar extends React.PureComponent<Props, State> {
             } else {
                 history.push(getAnnotationsPath(annotationFileVersionId, nextActiveAnnotationId));
             }
-        }
 
-        onAnnotationSelect(annotation);
+            const deferScrollToOnload = annotation?.target?.location?.type === 'frame';
+            onAnnotationSelect(annotation, deferScrollToOnload);
+        } else {
+            onAnnotationSelect(annotation);
+        }
     };
 
     handleItemsFiltered = (status?: ActivityFilterItemType) => {
diff --git a/src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js b/src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js
