fix(popupreply): Adjust positioning for IE11 (#609)

* fix(popupreply): Adjust positioning for IE11

* chore: address pr comments

* chore: remove unneeded dev dependency

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Author: Conrad Chan

jest.config.js

@@ -21,7 +21,7 @@ module.exports = {
     },
     restoreMocks: true,
     roots: ['src'],
-    setupFiles: ['jest-canvas-mock'],
+    setupFiles: ['jest-canvas-mock', '<rootDir>/test/jest/envWindow.js'],
     setupFilesAfterEnv: ['<rootDir>/scripts/jest/enzyme-adapter.js'],
     snapshotSerializers: ['enzyme-to-json/serializer'],
     testEnvironment: 'jest-environment-jsdom-sixteen',

src/components/Popups/PopupBase.scss

@@ -11,11 +11,17 @@
         @include ba-PopupArrow(12px, $popup-content-bg, $popup-content-bg, $popup-footer-bg, $popup-content-bg, $popup-border-color);
 
         z-index: 1; // Make sure popup is showing above Preview controls and left/right arrows
+    }
+
+    // The following media query is specific to IE10+
+    @media all and (-ms-high-contrast: none), (-ms-high-contrast: active) {
+        .ba-Popup {
+            z-index: 0; // Changing z-index causes page to flicker in IE11
+        }
 
-        // Changing z-index causes page to flicker in IE11
-        // The following media query is specific to IE10+
-        @media all and (-ms-high-contrast: none), (-ms-high-contrast: active) {
-            z-index: 0;
+        .ba-ItemList {
+            // stylelint-disable-next-line declaration-no-important
+            max-height: 140px !important; // Prevent list overflow onto subsequent pages in IE11
         }
     }
 

src/components/Popups/PopupReply.tsx

@@ -18,6 +18,11 @@ export type Props = {
     value?: string;
 };
 
+const isIE = (): boolean => {
+    const { userAgent } = navigator;
+    return userAgent.indexOf('Trident/') > 0;
+};
+
 export const options: Partial<Popper.Options> = {
     modifiers: [
         {
@@ -64,11 +69,20 @@ export default function PopupReply({
     ...rest
 }: Props): JSX.Element {
     const popupRef = React.useRef<PopupBase>(null);
+    const popupOptions = React.useRef<Partial<Popper.Options> | null>(null);
     const rotation = ReactRedux.useSelector(getRotation);
     const prevRotation = usePrevious(rotation);
     const scale = ReactRedux.useSelector(getScale);
     const prevScale = usePrevious(scale);
 
+    // Keep the options reference the same between renders
+    if (!popupOptions.current) {
+        popupOptions.current = {
+            ...options,
+            placement: isIE() ? 'top' : 'bottom',
+        };
+    }
+
     React.useEffect(() => {
         const { current: popup } = popupRef;
 
@@ -78,7 +92,7 @@ export default function PopupReply({
     }, [popupRef, rotation, scale]); // eslint-disable-line react-hooks/exhaustive-deps
 
     return (
-        <PopupBase ref={popupRef} data-resin-component="popupReply" options={options} {...rest}>
+        <PopupBase ref={popupRef} data-resin-component="popupReply" options={popupOptions.current} {...rest}>
             <ReplyForm
                 isPending={isPending}
                 onCancel={onCancel}

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

@@ -52,5 +52,37 @@ describe('PopupReply', () => {
             expect(wrapper.exists(PopupBase)).toBe(true);
             expect(wrapper.exists(ReplyForm)).toBe(true);
         });
+
+        test('should maintain referential integrity of the PopupBase options object across renders', () => {
+            const wrapper = getWrapper();
+
+            const popupOptions = wrapper.find(PopupBase).prop('options');
+
+            expect(wrapper.exists(PopupBase)).toBe(true);
+            expect(wrapper.find(ReplyForm).prop('value')).toBe('');
+
+            wrapper.setProps({ value: '1' });
+
+            expect(wrapper.find(PopupBase).prop('options')).toStrictEqual(popupOptions);
+            expect(wrapper.find(ReplyForm).prop('value')).toBe('1');
+        });
+    });
+
+    describe('Popup options', () => {
+        test.each`
+            userAgent                                                                  | expectedPlacement
+            ${'Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; rv:11.0) like Gecko'} | ${'top'}
+            ${'Other'}                                                                 | ${'bottom'}
+        `(
+            'should set placement option as $expectedPlacement based on userAgent=$userAgent',
+            ({ userAgent, expectedPlacement }) => {
+                global.window.navigator.userAgent = userAgent;
+
+                const wrapper = getWrapper();
+
+                expect(window.navigator.userAgent).toEqual(userAgent);
+                expect(wrapper.find(PopupBase).prop('options').placement).toBe(expectedPlacement);
+            },
+        );
     });
 });

test/jest/envWindow.js

@@ -0,0 +1,12 @@
+Object.defineProperty(
+    window.navigator,
+    'userAgent',
+    (value => ({
+        get() {
+            return value;
+        },
+        set(v) {
+            value = v;
+        },
+    }))(window.navigator.userAgent),
+);