Skip to content

Commit

Permalink
implemented code review changes and removed unnecessary uses of reque…
Browse files Browse the repository at this point in the history 
…stAnimationFrame
  • Loading branch information
@jadutter
jadutter committed Oct 13, 2023
1 parent 97368f8 commit 8af4b6b
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 36 deletions.
6 changes: 2 additions & 4 deletions src/components/modal/v2/lib/providers/transition.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
4 changes: 1 addition & 3 deletions src/library/zoid/message/component.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
});

Expand Down
26 changes: 11 additions & 15 deletions src/library/zoid/modal/containerTemplate.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
});
});
};
Expand All @@ -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);
};

Expand Down
49 changes: 35 additions & 14 deletions src/library/zoid/modal/prerenderTemplate.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
};

Expand All @@ -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;
});

Expand All @@ -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">
Expand Down

0 comments on commit 8af4b6b

Please sign in to comment.