Skip to content

Commit

Permalink
implemented code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@jadutter
jadutter committed Oct 16, 2023
1 parent 8af4b6b commit abd4979
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 42 deletions.
7 changes: 1 addition & 6 deletions src/components/modal/v2/lib/providers/transition.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
Expand Down Expand Up @@ -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') {
Expand Down
17 changes: 2 additions & 15 deletions src/library/zoid/message/component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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({
Expand Down
17 changes: 6 additions & 11 deletions src/library/zoid/modal/containerTemplate.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -32,15 +31,15 @@ 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
// Firefox requires 2 RAFs due to where they are called in the event loop
requestAnimationFrame(() => {
requestAnimationFrame(() => {
overlay.classList.add(CLASS.MODAL_SHOW);
if (renderedModal) {
if (state.renderedModal) {
frame.focus();
} else if (window.document.activeElement !== prerenderFrame) {
prerenderFrame.focus();
Expand All @@ -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))
Expand Down
17 changes: 7 additions & 10 deletions src/library/zoid/modal/prerenderTemplate.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -120,15 +119,14 @@ export default ({ doc, props, event, state }) => {
}
`;
let renderedModal = false;
let closeBtn;

const checkForErrors = element => {
ZalgoPromise.delay(ERROR_DELAY).then(() => {
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';
Expand All @@ -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();
Expand All @@ -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(() => {
Expand All @@ -176,10 +177,6 @@ export default ({ doc, props, event, state }) => {
}
});

event.on(EVENT.RENDERED, () => {
renderedModal = true;
});

return (
<html lang="en">
<head>
Expand Down

0 comments on commit abd4979

Please sign in to comment.