Skip to content

Commit

Permalink
[WNMGDS-2495] Fix dropdown menus getting cut off at dialog window bou…
Browse files Browse the repository at this point in the history
…ndaries (#2669)

* Redefine the virtual boundaries of our dialogs through CSS

Hopefully this shouldn't change anything visually. I know I need to take a look at the code for clicking outside the dialog closing it

* Update the "click outside exits" logic

* Update unit test snapshots

* COMMIT FOR TESTING THE FIX

* Revert "COMMIT FOR TESTING THE FIX"

This reverts commit 6b9361a.

* Fix forced-colors-mode outline

* Scroll to a consistent position in our VRTs before taking the screenshots

* Update VRT refs that show the FCM dialog border

* It's not the window that's scrolled; it's the dialog :(
  • Loading branch information
pwolfert authored Sep 11, 2023
1 parent bee9773 commit 5eefa5c
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 28 deletions.
17 changes: 15 additions & 2 deletions packages/design-system/src/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,21 @@ export const Dialog = (props: DialogProps) => {
}, []);

return (
<NativeDialog className={dialogClassNames} showModal exit={onExit} {...modalProps} id={rootId}>
<div role="document" ref={containerRef} tabIndex={-1} aria-labelledby={headingId}>
<NativeDialog
className={dialogClassNames}
showModal
exit={onExit}
{...modalProps}
id={rootId}
boundingBoxRef={containerRef}
>
<div
className="ds-c-dialog__window"
role="document"
ref={containerRef}
tabIndex={-1}
aria-labelledby={headingId}
>
<header className={headerClassNames}>
{heading && (
<h1 className="ds-h2" id={headingId} ref={headingRef}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`Dialog close button text and variation can be changed 1`] = `
<div
aria-labelledby="static-id__heading"
class="ds-c-dialog__window"
role="document"
tabindex="-1"
>
Expand Down Expand Up @@ -50,6 +51,7 @@ exports[`Dialog close button text and variation can be changed 1`] = `
exports[`Dialog renders with additional classNames and size 1`] = `
<div
aria-labelledby="static-id__heading"
class="ds-c-dialog__window"
role="document"
tabindex="-1"
>
Expand Down
29 changes: 18 additions & 11 deletions packages/design-system/src/components/NativeDialog/NativeDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface NativeDialogProps extends Omit<DialogHTMLAttributes<HTMLElement
* Pass `true` to have the dialog close when its backdrop pseudo-element is clicked
*/
backdropClickExits?: boolean;
boundingBoxRef?: React.MutableRefObject<any>;
/**
* Function called to close dialog.
*/
Expand All @@ -31,6 +32,7 @@ export const NativeDialog = ({
exit,
showModal,
backdropClickExits,
boundingBoxRef,
...dialogProps
}: NativeDialogProps) => {
const dialogRef = useRef(null);
Expand All @@ -50,18 +52,29 @@ export const NativeDialog = ({
};
}, [showModal]);

// Bind and unbind event listeners on mount and unmount
// Bind and unbind cancel event listeners on mount and unmount
useEffect(() => {
const dialogNode = dialogRef.current;

const handleCancel = (event) => {
event.preventDefault();
exit();
};
dialogNode.addEventListener('cancel', handleCancel);
return () => {
dialogNode.removeEventListener('cancel', handleCancel);
};
}, [exit]);

// Bind and unbind click event listeners on mount and unmount
useEffect(() => {
if (!backdropClickExits) {
return;
}

const dialogNode = dialogRef.current;
const handleClick = (event) => {
const rect = dialogNode.getBoundingClientRect();
const boundingNode = boundingBoxRef?.current ?? dialogRef.current;
const rect = boundingNode.getBoundingClientRect();
const isInDialog =
rect.top <= event.clientY &&
event.clientY <= rect.top + rect.height &&
Expand All @@ -71,15 +84,9 @@ export const NativeDialog = ({
exit();
}
};
if (backdropClickExits) {
dialogNode.addEventListener('click', handleClick);
}

dialogNode.addEventListener('click', handleClick);
return () => {
dialogNode.removeEventListener('cancel', handleCancel);
if (backdropClickExits) {
dialogNode.removeEventListener('click', handleClick);
}
dialogNode.removeEventListener('click', handleClick);
};
}, [exit, backdropClickExits]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ exports[`ThirdPartyExternalLink ThirdPartyExternalLink dialog renders external l
>
<div
aria-labelledby="dialog--6__heading"
class="ds-c-dialog__window"
role="document"
tabindex="-1"
>
Expand Down
42 changes: 27 additions & 15 deletions packages/design-system/src/styles/components/_Dialog.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,47 +36,59 @@ dialog.fixed {
// End polyfill styles

.ds-c-dialog {
background: transparent;
border: none;
height: auto;
inset: 0;
margin: 0;
max-height: none;
max-width: none;
padding: 0;
width: auto;
}

.ds-c-dialog__window {
background-color: var(--dialog__background-color);
border: none;
box-shadow: var(--shadow-base);
box-sizing: border-box;
color: var(--color-base); // Needed to override user-agent styles `canvasText`
display: inline-block;
margin-top: $spacer-6;
display: block;
margin: auto;
margin-block-start: $spacer-6;
max-width: var(--measure-base);
padding: 0;
padding: var(--dialog__padding);
text-align: start;
width: 95%; // provide space for the background layer to peek through

@media (-ms-high-contrast: active), (forced-colors: active) {
outline: $spacer-2 solid WindowText;
}

> [role='document'] {
padding: var(--dialog__padding);
}

// We set default focus to the role="document" div, but we don't want
// to show the default focus halo
> [role='document']:focus {
&:focus {
outline: none;
}

@media (-ms-high-contrast: active), (forced-colors: active) {
&,
&:focus {
outline: $spacer-2 solid WindowText;
}
}
}

.ds-c-dialog::backdrop,
.ds-c-dialog + .backdrop {
background-color: var(--dialog-overlay__background-color);
}

.ds-c-dialog--narrow {
.ds-c-dialog--narrow .ds-c-dialog__window {
max-width: var(--measure-narrow);
}

.ds-c-dialog--wide {
.ds-c-dialog--wide .ds-c-dialog__window {
max-width: var(--measure-wide);
}

.ds-c-dialog--full {
.ds-c-dialog--full .ds-c-dialog__window {
max-width: var(--site-max-width);
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 5eefa5c

Please sign in to comment.