From 8af4b6bdbc636ae0feec8b1c42254ef10ce7b588 Mon Sep 17 00:00:00 2001
From: jadutter <4691511+jadutter@users.noreply.github.com>
Date: Fri, 13 Oct 2023 15:25:26 -0400
Subject: [PATCH] implemented code review changes and removed unnecessary uses
 of requestAnimationFrame

---
 .../modal/v2/lib/providers/transition.js      |  6 +--
 src/library/zoid/message/component.js         |  4 +-
 src/library/zoid/modal/containerTemplate.jsx  | 26 +++++-----
 src/library/zoid/modal/prerenderTemplate.jsx  | 49 +++++++++++++------
 4 files changed, 49 insertions(+), 36 deletions(-)

diff --git a/src/components/modal/v2/lib/providers/transition.js b/src/components/modal/v2/lib/providers/transition.js
index 991eef5af6..e1f1b9cad6 100644
--- a/src/components/modal/v2/lib/providers/transition.js
+++ b/src/components/modal/v2/lib/providers/transition.js
@@ -27,10 +27,8 @@ export const TransitionStateProvider = ({ children }) => {
      * Particularly useful for those using screen readers and other accessibility functions.
      */
     const focusCloseBtnOnModalOpen = () => {
-        // give the document time to update before trying to focus the close button
-        window.requestAnimationFrame(() => {
-            document.querySelector('.close')?.focus();
-        });
+        // focus the close button
+        document.querySelector('.close')?.focus();
     };
 
     useEffect(() => {
diff --git a/src/library/zoid/message/component.js b/src/library/zoid/message/component.js
index 49aa5118c8..626c09c1ba 100644
--- a/src/library/zoid/message/component.js
+++ b/src/library/zoid/message/component.js
@@ -162,9 +162,7 @@ export default createGlobalVariableGetter('__paypal_credit_message__', () =>
                             refIndex: index,
                             src: 'message_click',
                             onClose: () => {
-                                window.requestAnimationFrame(() => {
-                                    getContainer().querySelector('iframe').focus();
-                                });
+                                getContainer().querySelector('iframe').focus();
                             }
                         });
 
diff --git a/src/library/zoid/modal/containerTemplate.jsx b/src/library/zoid/modal/containerTemplate.jsx
index e4785bde4f..5a124524b8 100644
--- a/src/library/zoid/modal/containerTemplate.jsx
+++ b/src/library/zoid/modal/containerTemplate.jsx
@@ -16,6 +16,7 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon
     // In this scenario we can skip creating container elements and transitions since we
     // cannot overlay across the entire screen
     if (context === 'popup') return undefined;
+
     let renderedModal = false;
     let previousFocus = document.activeElement;
     const [hijackViewport, replaceViewport] = viewportHijack();
@@ -39,13 +40,11 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon
             requestAnimationFrame(() => {
                 requestAnimationFrame(() => {
                     overlay.classList.add(CLASS.MODAL_SHOW);
-                    requestAnimationFrame(() => {
-                        if (renderedModal) {
-                            frame.focus();
-                        } else if (window.document.activeElement !== prerenderFrame) {
-                            prerenderFrame.focus();
-                        }
-                    });
+                    if (renderedModal) {
+                        frame.focus();
+                    } else if (window.document.activeElement !== prerenderFrame) {
+                        prerenderFrame.focus();
+                    }
                 });
             });
         };
@@ -56,14 +55,11 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon
             replaceViewport();
             setTimeout(() => {
                 wrapper.classList.add(CLASS.HIDDEN);
-
-                requestAnimationFrame(() => {
-                    if (renderedModal) {
-                        frame.blur();
-                    } else {
-                        previousFocus.focus();
-                    }
-                });
+                if (renderedModal) {
+                    frame.blur();
+                } else {
+                    previousFocus.focus();
+                }
             }, TRANSITION_DELAY);
         };
 
diff --git a/src/library/zoid/modal/prerenderTemplate.jsx b/src/library/zoid/modal/prerenderTemplate.jsx
index 1cc6c7ad54..79909d12f3 100644
--- a/src/library/zoid/modal/prerenderTemplate.jsx
+++ b/src/library/zoid/modal/prerenderTemplate.jsx
@@ -2,6 +2,7 @@
 import { node, dom } from '@krakenjs/jsx-pragmatic/src';
 import { Spinner } from '@paypal/common-components';
 import { ZalgoPromise } from '@krakenjs/zalgo-promise/src';
+import { EVENT } from '@krakenjs/zoid/src';
 
 export default ({ doc, props, event, state }) => {
     const ERROR_DELAY = 15000;
@@ -138,10 +139,7 @@ export default ({ doc, props, event, state }) => {
 
     const focusCloseButton = () => {
         if (closeBtn) {
-            window.requestAnimationFrame(() => {
-                // TODO: determine how to get this to re-focus if the prerender is dismissed and re-opened
-                closeBtn.focus();
-            });
+            closeBtn.focus();
         }
     };
 
@@ -158,12 +156,27 @@ export default ({ doc, props, event, state }) => {
     const handleRender = element => {
         closeBtn = element.querySelector('#prerender-close-btn');
         focusCloseButton();
+        // we need to give chrome a moment before we can focus the close button
+        window.requestAnimationFrame(() => {
+            focusCloseButton();
+        });
         ZalgoPromise.delay(ERROR_DELAY).then(() => {
             return checkForErrors(element);
         });
     };
 
-    event.on('zoid-rendered', () => {
+    event.on('modal-show', () => {
+        if (!renderedModal) {
+            // we need to give chrome a moment before we can focus the close button
+            window.requestAnimationFrame(() => {
+                window.requestAnimationFrame(() => {
+                    focusCloseButton();
+                });
+            });
+        }
+    });
+
+    event.on(EVENT.RENDERED, () => {
         renderedModal = true;
     });
 
@@ -174,16 +187,24 @@ export default ({ doc, props, event, state }) => {
                 <meta name="viewport" content="width=device-width, initial-scale=1" />
             </head>
             <style nonce={props.cspNonce}>{styles}</style>
-            <body onRender={handleRender}>
+            {/* 
+                disable jsx-a11y/no-static-element-interactions
+                    because we need handleEscape to work regardless of which element has focus,
+                    and Safari currently forbids an iframe from setting focus within its document
+                    until the user interacts with the contents of the iframe 
+            */}
+            {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
+            <body onRender={handleRender} onKeyUp={handleEscape}>
                 <div class="modal" aria-errormessage="modal-status">
-                    {/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */}
-                    <div
-                        class="overlay"
-                        role="dialog"
-                        onClick={handleClose}
-                        onKeyUp={handleEscape}
-                        aria-keyshortcuts="escape"
-                    />
+                    {/* 
+                        disable jsx-a11y/click-events-have-key-events 
+                            because although the overlay does not have a keyup listener, the body does 
+                        disable jsx-a11y/no-static-element-interactions
+                            because if we give it `role="button"`, then it will require the overlay be 
+                            focusable, which is unnecessary given the `#prerender-close-btn`
+                    */}
+                    {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions, jsx-a11y/click-events-have-key-events */}
+                    <div class="overlay" onClick={handleClose} aria-keyshortcuts="escape" />
                     <div class="top-overlay" />
                     <div class="modal-content">
                         <div class="close-button">