fix(image): Update reply popup when file scale/rotation changes (#529)

Author: jstoffan

src/common/BaseAnnotator.ts

@@ -5,7 +5,6 @@ import EventEmitter from './EventEmitter';
 import i18n from '../utils/i18n';
 import messages from '../messages';
 import { Event, IntlOptions, LegacyEvent, Permissions } from '../@types';
-import { setIsInitialized } from '../store';
 import './BaseAnnotator.scss';
 
 export type Container = string | HTMLElement;
@@ -109,15 +108,13 @@ export default class BaseAnnotator extends EventEmitter {
         this.annotatedEl.classList.add(CSS_LOADED_CLASS);
         this.containerEl.classList.add(CSS_CONTAINER_CLASS);
 
-        // Update the store with the options provided by preview
-        this.store.dispatch(store.setRotationAction(rotation));
-        this.store.dispatch(store.setScaleAction(scale));
-
         // Defer to the child class to render annotations
         this.render();
 
         // Update the store now that annotations have been rendered
-        this.store.dispatch(setIsInitialized());
+        this.store.dispatch(store.setRotationAction(rotation));
+        this.store.dispatch(store.setScaleAction(scale));
+        this.store.dispatch(store.setIsInitialized());
     }
 
     public removeAnnotation = (annotationId: string): void => {

src/common/__tests__/usePrevious-test.tsx

@@ -0,0 +1,30 @@
+import * as React from 'react';
+import { mount, ReactWrapper } from 'enzyme';
+import usePrevious from '../usePrevious';
+
+describe('src/common/usePrevious', () => {
+    function TestComponent({ value }: { value: string }): JSX.Element {
+        const previousValue = usePrevious(value);
+
+        return (
+            <div>
+                <div data-testid="current">{value}</div>
+                <div data-testid="previous">{previousValue}</div>
+            </div>
+        );
+    }
+
+    const getWrapper = (value: string): ReactWrapper => mount(<TestComponent value={value} />);
+
+    test('should render the current and previous value, if one is present', () => {
+        const wrapper = getWrapper('test');
+
+        expect(wrapper.find('[data-testid="current"]').text()).toEqual('test');
+        expect(wrapper.find('[data-testid="previous"]').text()).toEqual('');
+
+        wrapper.setProps({ value: 'new!' });
+
+        expect(wrapper.find('[data-testid="current"]').text()).toEqual('new!');
+        expect(wrapper.find('[data-testid="previous"]').text()).toEqual('test');
+    });
+});

src/common/usePrevious.ts

@@ -0,0 +1,13 @@
+import React from 'react';
+
+// Returns the last provided value, then caches the new value via an effect
+export default function usePrevious<T>(value: T): T | undefined {
+    const ref = React.useRef<T>();
+
+    // The useEffect callback is only invoked after the parent component has rendered
+    React.useEffect(() => {
+        ref.current = value;
+    }, [value]);
+
+    return ref.current;
+}

src/components/Popups/PopupReply.tsx

@@ -1,7 +1,10 @@
-import React from 'react';
+import * as React from 'react';
+import * as Popper from '@popperjs/core';
+import * as ReactRedux from 'react-redux';
 import PopupBase from './PopupBase';
 import ReplyForm from '../ReplyForm';
-
+import usePrevious from '../../common/usePrevious';
+import { getScale, getRotation } from '../../store/options';
 import './PopupReply.scss';
 
 export type Props = {
@@ -14,13 +17,13 @@ export type Props = {
     value?: string;
 };
 
-export const options = {
+export const options: Partial<Popper.Options> = {
     modifiers: [
         {
             name: 'arrow',
             options: {
                 element: '.ba-Popup-arrow',
-                padding: 20,
+                padding: 10,
             },
         },
         {
@@ -53,8 +56,22 @@ export default function PopupReply({
     value = '',
     ...rest
 }: Props): JSX.Element {
+    const popupRef = React.useRef<PopupBase>(null);
+    const rotation = ReactRedux.useSelector(getRotation);
+    const prevRotation = usePrevious(rotation);
+    const scale = ReactRedux.useSelector(getScale);
+    const prevScale = usePrevious(scale);
+
+    React.useEffect(() => {
+        const { current: popup } = popupRef;
+
+        if (popup?.popper && (rotation !== prevRotation || scale !== prevScale)) {
+            popup.popper.update();
+        }
+    }, [popupRef, rotation, scale]); // eslint-disable-line react-hooks/exhaustive-deps
+
     return (
-        <PopupBase options={options} {...rest}>
+        <PopupBase ref={popupRef} options={options} {...rest}>
             <ReplyForm
                 isPending={isPending}
                 onCancel={onCancel}

src/components/Popups/__mocks__/PopupBase.tsx

@@ -0,0 +1,11 @@
+import * as React from 'react';
+
+export default class PopupBase extends React.Component<{ children: React.ReactNode }> {
+    name = 'PopupBaseMock';
+
+    render(): JSX.Element {
+        const { children } = this.props;
+
+        return <div>{children}</div>;
+    }
+}

src/components/Popups/__tests__/PopupReply-test.tsx

@@ -1,32 +1,55 @@
-import React from 'react';
-import { shallow, ShallowWrapper } from 'enzyme';
+import * as React from 'react';
+import * as ReactRedux from 'react-redux';
+import { mount, ReactWrapper } from 'enzyme';
+import PopupBase from '../PopupBase';
 import PopupReply, { Props } from '../PopupReply';
 import ReplyForm from '../../ReplyForm';
 
-jest.mock('../PopupBase', () => ({
-    __esModule: true,
-    default: ({ children }: { children: React.ReactNode }): JSX.Element => <div>{children}</div>,
+jest.mock('react-redux', () => ({
+    useSelector: jest.fn(),
 }));
 
-type PropsIndex = {
-    [key: string]: Function | HTMLElement | boolean;
-};
+jest.mock('../PopupBase');
+jest.mock('../../ReplyForm');
 
 describe('components/Popups/PopupReply', () => {
-    const defaults: Props & PropsIndex = {
+    const defaults: Props = {
         isPending: false,
         onCancel: jest.fn(),
         onChange: jest.fn(),
         onSubmit: jest.fn(),
         reference: document.createElement('div'),
     };
 
-    const getWrapper = (props = {}): ShallowWrapper => shallow(<PopupReply {...defaults} {...props} />);
+    const getWrapper = (props = {}): ReactWrapper => mount(<PopupReply {...defaults} {...props} />);
+
+    beforeEach(() => {
+        jest.spyOn(React, 'useEffect').mockImplementation(f => f());
+    });
+
+    describe('state changes', () => {
+        test('should call update on the underlying popper instance when the store changes', () => {
+            const popupMock = { popper: { update: jest.fn() } };
+            const reduxSpy = jest.spyOn(ReactRedux, 'useSelector').mockImplementation(() => false);
+            const refSpy = jest.spyOn(React, 'useRef').mockImplementation(() => ({ current: popupMock }));
+            const wrapper = getWrapper();
+
+            reduxSpy.mockReturnValueOnce(true);
+            wrapper.setProps({ value: '1' });
+
+            reduxSpy.mockReturnValueOnce(false);
+            wrapper.setProps({ value: '2' });
+
+            expect(refSpy).toHaveBeenCalled();
+            expect(popupMock.popper.update).toHaveBeenCalledTimes(3);
+        });
+    });
 
     describe('render()', () => {
         test('should render the ReplyForm', () => {
             const wrapper = getWrapper();
 
+            expect(wrapper.exists(PopupBase)).toBe(true);
             expect(wrapper.exists(ReplyForm)).toBe(true);
         });
     });

src/components/ReplyForm/__mocks__/ReplyForm.tsx

@@ -0,0 +1,11 @@
+import * as React from 'react';
+
+export default class ReplyForm extends React.Component<{ children: React.ReactNode }> {
+    name = 'ReplyFormMock';
+
+    render(): JSX.Element {
+        const { children } = this.props;
+
+        return <div>{children}</div>;
+    }
+}

src/components/ReplyForm/__mocks__/ReplyFormContainer.tsx

@@ -0,0 +1 @@
+export { default } from './ReplyForm';

src/components/ReplyForm/__mocks__/index.ts

@@ -0,0 +1 @@
+export { default } from './ReplyForm';