From abd497985ba1acb2415ac9dbfb7f9aa4f76f0054 Mon Sep 17 00:00:00 2001 From: jadutter <4691511+jadutter@users.noreply.github.com> Date: Mon, 16 Oct 2023 10:48:33 -0400 Subject: [PATCH] implemented code review changes --- .../modal/v2/lib/providers/transition.js | 7 +------ src/library/zoid/message/component.js | 17 ++--------------- src/library/zoid/modal/containerTemplate.jsx | 17 ++++++----------- src/library/zoid/modal/prerenderTemplate.jsx | 17 +++++++---------- 4 files changed, 16 insertions(+), 42 deletions(-) diff --git a/src/components/modal/v2/lib/providers/transition.js b/src/components/modal/v2/lib/providers/transition.js index e1f1b9cad6..17c5d0d966 100644 --- a/src/components/modal/v2/lib/providers/transition.js +++ b/src/components/modal/v2/lib/providers/transition.js @@ -41,7 +41,6 @@ export const TransitionStateProvider = ({ children }) => { if (entry.isIntersecting) { // Removes .modal-closed class from modal iframe body when modal is open. document.body.classList.remove('modal-closed'); - document.body.classList.add('modal-open'); setState(STATUS.OPEN); onShow(); @@ -53,10 +52,7 @@ export const TransitionStateProvider = ({ children }) => { * The .modal-closed class is added via useTransitionState. If this class is not on the modal iframe body, * we know the modal is open and should not trigger the hook to reset the modal to the primary view. */ - if ( - document.body.classList.contains('modal-closed') || - !document.body.classList.contains('modal-open') - ) { + if (document.body.classList.contains('modal-closed')) { setState(STATUS.CLOSED); } }, TRANSITION_DELAY); @@ -89,7 +85,6 @@ export const useTransitionState = () => { linkName => { // Appends a class to the modal iframe body when handleClose is fired. document.body.classList.add('modal-closed'); - document.body.classList.remove('modal-open'); onClose({ linkName }); if (window === window.top && typeof close === 'function') { diff --git a/src/library/zoid/message/component.js b/src/library/zoid/message/component.js index 626c09c1ba..cacec5cab4 100644 --- a/src/library/zoid/message/component.js +++ b/src/library/zoid/message/component.js @@ -134,17 +134,7 @@ export default createGlobalVariableGetter('__paypal_credit_message__', () => const { onClick } = props; return ({ meta }) => { - const { - modal, - index, - account, - merchantId, - currency, - amount, - buyerCountry, - onApply, - getContainer - } = props; + const { modal, index, account, merchantId, currency, amount, buyerCountry, onApply } = props; const { offerType, offerCountry, messageRequestId } = meta; // Avoid spreading message props because both message and modal @@ -160,10 +150,7 @@ export default createGlobalVariableGetter('__paypal_credit_message__', () => offerCountry, refId: messageRequestId, refIndex: index, - src: 'message_click', - onClose: () => { - getContainer().querySelector('iframe').focus(); - } + src: 'message_click' }); logger.track({ diff --git a/src/library/zoid/modal/containerTemplate.jsx b/src/library/zoid/modal/containerTemplate.jsx index 5a124524b8..e9d206a942 100644 --- a/src/library/zoid/modal/containerTemplate.jsx +++ b/src/library/zoid/modal/containerTemplate.jsx @@ -17,8 +17,7 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon // cannot overlay across the entire screen if (context === 'popup') return undefined; - let renderedModal = false; - let previousFocus = document.activeElement; + state.previousFocus = document.activeElement; const [hijackViewport, replaceViewport] = viewportHijack(); const CLASS = { @@ -32,7 +31,7 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon const handleShow = () => { state.open = true; - previousFocus = document.activeElement; + state.previousFocus = document.activeElement; wrapper.classList.remove(CLASS.HIDDEN); hijackViewport(); // Browser needs to repaint otherwise the transition happens immediately @@ -40,7 +39,7 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon requestAnimationFrame(() => { requestAnimationFrame(() => { overlay.classList.add(CLASS.MODAL_SHOW); - if (renderedModal) { + if (state.renderedModal) { frame.focus(); } else if (window.document.activeElement !== prerenderFrame) { prerenderFrame.focus(); @@ -55,22 +54,18 @@ export default ({ uid, frame, prerenderFrame, doc, event, state, props: { cspNon replaceViewport(); setTimeout(() => { wrapper.classList.add(CLASS.HIDDEN); - if (renderedModal) { - frame.blur(); - } else { - previousFocus.focus(); - } + state.previousFocus.focus(); }, TRANSITION_DELAY); }; const handleEscape = evt => { - if (state.open && (`${evt?.key}`.toLowerCase().startsWith('esc') || evt.charCode === 27)) { + if (state.open && (evt?.key === 'Escape' || evt?.key === 'Esc' || evt.charCode === 27)) { handleHide(); } }; const handleTransition = () => { - renderedModal = true; + state.renderedModal = true; ZalgoPromise.delay(TRANSITION_DELAY) .then(() => overlay.classList.add(CLASS.TRANSITION)) .then(() => ZalgoPromise.delay(TRANSITION_DELAY)) diff --git a/src/library/zoid/modal/prerenderTemplate.jsx b/src/library/zoid/modal/prerenderTemplate.jsx index 79909d12f3..4362f97e2f 100644 --- a/src/library/zoid/modal/prerenderTemplate.jsx +++ b/src/library/zoid/modal/prerenderTemplate.jsx @@ -2,7 +2,6 @@ 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; @@ -120,7 +119,6 @@ export default ({ doc, props, event, state }) => { } `; - let renderedModal = false; let closeBtn; const checkForErrors = element => { @@ -128,7 +126,7 @@ export default ({ doc, props, event, state }) => { const modalStatus = element.querySelector('#modal-status'); // if we have a place to put our status message, // and we have not heard the 'zoid-rendered' event for the modal yet - if (modalStatus && !renderedModal) { + if (modalStatus && !state.renderedModal) { // assign variable to state and access in UI modalStatus.style.display = 'block'; modalStatus.textContent = 'Error loading Modal'; @@ -148,14 +146,17 @@ export default ({ doc, props, event, state }) => { }; const handleEscape = evt => { - if (!renderedModal && state.open && (`${evt?.key}`.toLowerCase().startsWith('esc') || evt?.charCode === 27)) { + if ( + !state.renderedModal && + state.open && + (evt?.key === 'Escape' || evt?.key === 'Esc' || evt.charCode === 27) + ) { handleClose(); } }; 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(); @@ -166,7 +167,7 @@ export default ({ doc, props, event, state }) => { }; event.on('modal-show', () => { - if (!renderedModal) { + if (!state.renderedModal) { // we need to give chrome a moment before we can focus the close button window.requestAnimationFrame(() => { window.requestAnimationFrame(() => { @@ -176,10 +177,6 @@ export default ({ doc, props, event, state }) => { } }); - event.on(EVENT.RENDERED, () => { - renderedModal = true; - }); - return (