Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: POC for isHidden prop on ModalDialog #2431

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Modal/ModalContext.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
});

function ModalContextProvider({
onClose, isOpen, isBlocking, children,
onClose, isOpen, isHidden, isBlocking, children,

Check failure on line 9 in src/Modal/ModalContext.jsx

View workflow job for this annotation

GitHub Actions / tests

'isHidden' is missing in props validation
}) {
const modalContextValue = useMemo(
() => ({ onClose, isOpen, isBlocking }),
[onClose, isOpen, isBlocking],
() => ({ onClose, isOpen, isHidden, isBlocking }),

Check failure on line 12 in src/Modal/ModalContext.jsx

View workflow job for this annotation

GitHub Actions / tests

Expected a line break after this opening brace

Check failure on line 12 in src/Modal/ModalContext.jsx

View workflow job for this annotation

GitHub Actions / tests

Expected a line break before this closing brace
[onClose, isOpen, isHidden, isBlocking],
);

return (
Expand Down
3 changes: 2 additions & 1 deletion src/Modal/ModalDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
children,
title,
isOpen,
isHidden,

Check failure on line 23 in src/Modal/ModalDialog.jsx

View workflow job for this annotation

GitHub Actions / tests

'isHidden' is missing in props validation
onClose,
size,
variant,
Expand All @@ -34,7 +35,7 @@
const isMobile = useMediaQuery({ query: '(max-width: 767.98px)' });
const showFullScreen = (isFullscreenOnMobile && isMobile);
return (
<ModalLayer isOpen={isOpen} onClose={onClose} isBlocking={isBlocking} zIndex={zIndex}>
<ModalLayer isOpen={isOpen} isHidden={isHidden} onClose={onClose} isBlocking={isBlocking} zIndex={zIndex}>
<div
role="dialog"
aria-label={title}
Expand Down
22 changes: 12 additions & 10 deletions src/Modal/ModalLayer.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
* component is that if a modal object is visible then it is "enabled"
*/
function ModalLayer({
children, onClose, isOpen, isBlocking, zIndex,
children, onClose, isOpen, isHidden, isBlocking, zIndex,

Check failure on line 42 in src/Modal/ModalLayer.jsx

View workflow job for this annotation

GitHub Actions / tests

'isHidden' is missing in props validation
}) {
useEffect(() => {
if (isOpen) {
Expand All @@ -52,29 +52,31 @@
};
}, [isOpen]);

if (!isOpen) {
if (!isOpen && !isHidden) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[consideration] It's possible this conditional could remain unchanged given isOpen would need to be truthy for isHidden to work anyways.

return null;
}

const onClickOutside = !isBlocking ? onClose : null;

return (
<ModalContextProvider onClose={onClose} isOpen={isOpen} isBlocking={isBlocking}>
<ModalContextProvider onClose={onClose} isOpen={isOpen} isHidden={isHidden} isBlocking={isBlocking}>
<Portal>
<FocusOn
allowPinchZoom
scrollLock
enabled={isOpen}
enabled={isOpen && !isHidden}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FocusOn is used in ModalDialog to handle the accessibility of auto-focus on the first interactive element in the modal and handling the Esc keypress or backdrop click.

onEscapeKey={onClose}
onClickOutside={onClickOutside}
className={classNames(
'pgn__modal-layer',
zIndex ? `zindex-${zIndex}` : '',
)}
className={classNames({
'pgn__modal-layer': isOpen && !isHidden,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] Conditionally include the pgn__modal-layer class name only when the modal contents and the backdrop should actually be visible. Without making this class name conditional, the resulting div from FocusOn acts a non-clickable overlay.

[`zindex-${zIndex}`]: !!zIndex,
})}
>
<ModalContentContainer>
<ModalBackdrop onClick={onClickOutside} />
{children}
{!isHidden && <ModalBackdrop onClick={onClickOutside} />}
<div className={classNames({ 'd-none': isHidden })}>
Copy link
Member Author

@adamstankiewicz adamstankiewicz Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we should avoid using CSS utility classes with !important in Paragon components; define a custom .pgn__ namespaced class name to use instead with display: none; to be more theme-able.

{children}
</div>
</ModalContentContainer>
</FocusOn>
</Portal>
Expand Down
50 changes: 50 additions & 0 deletions src/Modal/modal-dialog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,53 @@ label for the dialog element.
)
}
```

## With hidden modal contents

By default, ``ModalDialog`` only render its modal contents to the DOM when the modal is explicitly marked as open via the `isOpen` prop. However, in some cases, needing to re-render the contents of the modal body each and every time the modal closes and re-opens can be expensive and/or a poor user experience (UX), e.g. if the modal contents are dynamic and loaded from an external source like some iframed content or a slow loading API request.

To mitigate these performance concerns, ``ModalDialog`` supports an optional ``isHidden`` prop that when used in conjunction with ``isOpen``, would result in the modal contents and the backdrop still being kept in the React Portal DOM, but visually hidden in the UI.

```jsx live
() => {
const [isHidden, hideModal, showModal] = useToggle(true);
return (
<>
<div className="d-flex">
<Button variant="outline-primary" onClick={showModal}>Open standard modal dialog</Button>
</div>
<ModalDialog
title="My dialog"
isOpen /* always "open" */
isHidden={isHidden}
onClose={hideModal}
hasCloseButton
isFullscreenOnMobile
>
<ModalDialog.Header>
<ModalDialog.Title>
I'm a dialog box
</ModalDialog.Title>
</ModalDialog.Header>

<ModalDialog.Body>
<p>
I'm baby palo santo ugh celiac fashion axe. La croix lo-fi venmo whatever. Beard man braid migas single-origin coffee forage ramps. Tumeric messenger bag bicycle rights wayfarers, try-hard cronut blue bottle health goth. Sriracha tumblr cardigan, cloud bread succulents tumeric copper mug marfa semiotics woke next level organic roof party +1 try-hard.
</p>
</ModalDialog.Body>

<ModalDialog.Footer>
<ActionRow>
<ModalDialog.CloseButton variant="tertiary">
Cancel
</ModalDialog.CloseButton>
<Button variant="primary">
Continue
</Button>
</ActionRow>
</ModalDialog.Footer>
</ModalDialog>
</>
)
}
```
Loading