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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Dialog so that it allows providing a TransitionComponent #957

Merged
merged 5 commits into from
Apr 17, 2023
Merged
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
136 changes: 92 additions & 44 deletions src/components/feedback/Dialog.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import classnames from 'classnames';
import type { RefObject } from 'preact';
import { useEffect, useLayoutEffect, useRef } from 'preact/hooks';
import { Fragment } from 'preact';
import {
useCallback,
useEffect,
useLayoutEffect,
useMemo,
useRef,
useState,
} from 'preact/hooks';

import { useClickAway } from '../../hooks/use-click-away';
import { useFocusAway } from '../../hooks/use-focus-away';
import { useKeyPress } from '../../hooks/use-key-press';
import { useSyncedRef } from '../../hooks/use-synced-ref';
import { useUniqueId } from '../../hooks/use-unique-id';
import type { PresentationalProps } from '../../types';
import type { PresentationalProps, TransitionComponent } from '../../types';
import { downcastRef } from '../../util/typing';
import Panel from '../layout/Panel';
import type { PanelProps } from '../layout/Panel';
Expand All @@ -33,6 +41,11 @@ type ComponentProps = {
* Restore focus to previously-focused element when unmounted/closed
*/
restoreFocus?: boolean;

/**
* Optional TransitionComponent for open (mount) and close (unmount) transitions
*/
transitionComponent?: TransitionComponent;
};

// This component forwards a number of props on to `Panel` but always sets the
Expand All @@ -53,46 +66,44 @@ const DialogNext = function Dialog({
children,
initialFocus = 'auto',
restoreFocus = false,
transitionComponent: TransitionComponent,

classes,
elementRef,

// Forwarded to Panel
buttons,
icon,
onClose,
onClose = noop,
paddingSize = 'md',
scrollable = true,
title,

...htmlAttributes
}: DialogProps) {
const modalRef = useSyncedRef(elementRef);
const closeHandler = onClose ?? noop;
const restoreFocusEl = useRef<HTMLElement | null>(
document.activeElement as HTMLElement | null
);
const [transitionComponentVisible, setTransitionComponentVisible] =
useState(false);

const closeHandler = useCallback(() => {
if (TransitionComponent) {
// When a TransitionComponent is provided, the actual "onClose" will be
// called by that component, once the "out" transition has finished
setTransitionComponentVisible(false);
} else {
onClose();
}
}, [onClose, TransitionComponent]);

useClickAway(modalRef, closeHandler, {
enabled: closeOnClickAway,
});

useKeyPress(['Escape'], closeHandler, {
enabled: closeOnEscape,
});

useFocusAway(modalRef, closeHandler, {
enabled: closeOnFocusAway,
});

const dialogDescriptionId = useUniqueId('dialog-description');

useEffect(() => {
const initializeDialog = useCallback(() => {
if (initialFocus === 'manual') {
return;
}
if (initialFocus === 'auto') {
// An explicit `initialFocus` has not be set, so use automatic focus
// An explicit `initialFocus` has not been set, so use automatic focus
Copy link
Contributor

Choose a reason for hiding this comment

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

Hee hee, thanks.

// handling. Modern accessibility guidance is to focus the dialog itself
// rather than trying to be smart about focusing a particular control
// within the dialog.
Expand All @@ -109,7 +120,39 @@ const DialogNext = function Dialog({
} else {
// Fall back to focusing the modal itself
modalRef.current?.focus();
return;
}
}, [initialFocus, modalRef]);

const onTransitionEnd = (direction: 'in' | 'out') => {
if (direction === 'in') {
initializeDialog();
} else {
onClose();
}
};

useClickAway(modalRef, closeHandler, {
enabled: closeOnClickAway,
});

useKeyPress(['Escape'], closeHandler, {
enabled: closeOnEscape,
});

useFocusAway(modalRef, closeHandler, {
enabled: closeOnFocusAway,
});

const dialogDescriptionId = useUniqueId('dialog-description');
const Wrapper = useMemo(
() => TransitionComponent ?? Fragment,
[TransitionComponent]
);

useEffect(() => {
setTransitionComponentVisible(true);
if (!TransitionComponent) {
initializeDialog();
}

// We only want to run this effect once when the dialog is mounted.
Expand Down Expand Up @@ -152,31 +195,36 @@ const DialogNext = function Dialog({
);

return (
<div
data-component="Dialog"
tabIndex={-1}
// NB: Role can be overridden with an HTML attribute; this is purposeful
role="dialog"
{...htmlAttributes}
className={classnames(
// Column-flex layout to constrain content to max-height
'flex flex-col',
classes
)}
ref={downcastRef(modalRef)}
<Wrapper
visible={transitionComponentVisible}
onTransitionEnd={onTransitionEnd}
>
<Panel
buttons={buttons}
fullWidthHeader={true}
icon={icon}
onClose={onClose}
paddingSize={paddingSize}
title={title}
scrollable={scrollable}
<div
data-component="Dialog"
tabIndex={-1}
// NB: Role can be overridden with an HTML attribute; this is purposeful
role="dialog"
{...htmlAttributes}
className={classnames(
// Column-flex layout to constrain content to max-height
'flex flex-col',
classes
)}
ref={downcastRef(modalRef)}
>
{children}
</Panel>
</div>
<Panel
buttons={buttons}
fullWidthHeader={true}
icon={icon}
onClose={closeHandler}
paddingSize={paddingSize}
title={title}
scrollable={scrollable}
>
{children}
</Panel>
</div>
</Wrapper>
);
};

Expand Down
58 changes: 58 additions & 0 deletions src/components/feedback/test/Dialog-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { mount } from 'enzyme';
import { createRef } from 'preact';

import { delay } from '../../../test-util/wait.js';
import { testPresentationalComponent } from '../../test/common-tests';
import Dialog from '../Dialog';
import { $imports } from '../Dialog';
Expand All @@ -13,6 +14,15 @@ const createComponent = (Component, props = {}) => {
);
};

/**
* @type {import('../../../next.js').TransitionComponent}
*/
const ComponentWithTransition = ({ children, visible, onTransitionEnd }) => {
// Fake a 50ms transition time
setTimeout(() => onTransitionEnd?.(visible ? 'in' : 'out'), 50);
return <div>{children}</div>;
};

describe('Dialog', () => {
let fakeUseClickAway;
let fakeUseFocusAway;
Expand Down Expand Up @@ -110,6 +120,30 @@ describe('Dialog', () => {
wrapper.find('[role="dialog"]').getDOMNode()
);
});

context('when a TransitionComponent is provided', () => {
it('focuses dialog only after "in" transition has ended', async () => {
const wrapper = mount(
<Dialog
title="My dialog"
transitionComponent={ComponentWithTransition}
/>,
{ attachTo: container }
);

// The Dialog is still not focused immediately after mounting it
assert.notEqual(
document.activeElement,
wrapper.find('[role="dialog"]').getDOMNode()
);
// Once the transition has ended, the Dialog should be focused
await delay(60); // Transition finishes after 6ms
assert.equal(
document.activeElement,
wrapper.find('[role="dialog"]').getDOMNode()
);
});
});
});

describe('restoring focus', () => {
Expand Down Expand Up @@ -224,6 +258,30 @@ describe('Dialog', () => {

assert.deepEqual(fakeUseFocusAway.lastCall.args[2], { enabled: true });
});

context('when a TransitionComponent is provided', () => {
it('closes the Dialog only after "out" transition has ended', async () => {
const wrapper = mount(
<Dialog
title="Test dialog"
onClose={onClose}
transitionComponent={ComponentWithTransition}
/>,
{
attachTo: container,
}
);

// We simulate closing the Dialog's Panel
wrapper.find('Panel').prop('onClose')();

// The onClose callback is not immediately invoked
assert.notCalled(onClose);
// Once the transition has ended, the callback should have been called
await delay(60); // Transition finishes after 50ms
assert.called(onClose);
});
});
});

describe('aria-describedby', () => {
Expand Down
1 change: 1 addition & 0 deletions src/next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export type {
CompositeProps,
IconComponent,
PresentationalProps,
TransitionComponent,
} from './types';

export type {
Expand Down
35 changes: 35 additions & 0 deletions src/pattern-library/components/patterns/FadeComponent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import classnames from 'classnames';
import { useCallback } from 'preact/hooks';

import type { TransitionComponent } from '../../../types';

/**
* This component is used just to demonstrate the `TransitionComponent`
* functionality on components supporting it, like `Dialog`.
*/
const FadeComponent: TransitionComponent = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a docstring/comment here to explain the purpose of this component (which is, as I understand it, to demonstrate the TransitionComponent functionality?)?

children,
visible,
onTransitionEnd,
}) => {
const handleTransitionEnd = useCallback(
() => onTransitionEnd?.(visible ? 'in' : 'out'),
[visible, onTransitionEnd]
);

return (
<div
className={classnames('transition-opacity duration-500', {
'opacity-100': visible,
'opacity-0': !visible,
})}
// @ts-expect-error preact uses "ontransitionend" rather than "onTransitionEnd".
// eslint-disable-next-line react/no-unknown-property
ontransitionend={handleTransitionEnd}
>
{children}
</div>
);
};

export default FadeComponent;
32 changes: 32 additions & 0 deletions src/pattern-library/components/patterns/feedback/DialogPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
Scroll,
} from '../../../../next';
import Library from '../../Library';
import FadeComponent from '../FadeComponent';
import { LoremIpsum, nabokovNovels } from '../samples';

const nabokovRows = nabokovNovels();
Expand Down Expand Up @@ -322,6 +323,37 @@ export default function DialogPage() {
</Dialog_>
</Library.Demo>
</Library.Example>

<Library.Example title="transitionComponent">
<p>
It allows to provide a component which supports transitions, but
keeping the internal behavior (initial focus, closing, etc)
transparent to consumers.
</p>
<p>
The only requirement is that provided component needs to conform
to the next props:
</p>
<Library.Code
content={`type TransitionComponentProps = {
visible: boolean;
onTransitionEnd?: (direction: 'in' | 'out') => void;
}`}
/>
<Library.Demo
title="Dialog with TransitionComponent example"
withSource
>
<Dialog_
title="Dialog with TransitionComponent example"
transitionComponent={FadeComponent}
>
<p>
This dialog has a fade in/out transition when opened/closed.
</p>
</Dialog_>
</Library.Demo>
</Library.Example>
</Library.Pattern>

<Library.Pattern title="Forwarded Props: Panel">
Expand Down
9 changes: 9 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,12 @@ export type BaseProps = PresentationalProps & { unstyled?: boolean };
* valid `<svg>` element attribute as props.
*/
export type IconComponent = FunctionComponent<JSX.SVGAttributes<SVGSVGElement>>;

/**
* A component that describes an `in` and an `out` transition, typically to
* animate the mounting and unmounting of a child component.
*/
export type TransitionComponent = JSX.ElementType<{
visible: boolean;
onTransitionEnd?: (direction: 'in' | 'out') => void;
}>;