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

Remove custom initial focus logic #2910

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
7 changes: 4 additions & 3 deletions packages/circuit-ui/components/Dialog/Dialog.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,16 @@ describe('Dialog', () => {
});

it('should focus a given element when provided', async () => {
const ref = createRef<HTMLButtonElement>();
render(
<Dialog {...props} open initialFocusRef={ref}>
<Dialog {...props} open>
{() => (
<div>
<button type="button" name="btn">
Button
</button>
<button ref={ref} type="button" name="btn">
{/* eslint-disable react/no-unknown-property */}
{/* @ts-expect-error React purposefully breaks the `autoFocus` property. Using the lowercase DOM attribute name instead forces it to be added to the DOM but will produce a console warning that can be safely ignored. https://github.com/facebook/react/issues/23301 */}
<button type="button" name="btn" autofocus="true">
Special button
</button>
</div>
Expand Down
40 changes: 4 additions & 36 deletions packages/circuit-ui/components/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import { useEscapeKey } from '../../hooks/useEscapeKey/index.js';
import { useLatest } from '../../hooks/useLatest/index.js';
import { useI18n } from '../../hooks/useI18n/useI18n.js';

import { getFirstFocusableElement } from './DialogService.js';
import classes from './Dialog.module.css';
import { translations } from './translations/index.js';

Expand Down Expand Up @@ -72,11 +71,6 @@ export interface PublicDialogProps
* Defaults to `navigator.language` in supported environments.
*/
locale?: Locale;
/**
* Enables focusing a particular element in the dialog content and overrides the default behavior.
* @default false.
*/
initialFocusRef?: RefObject<HTMLElement>;
/**
* A `ReactNode` or a function that returns the content of the modal dialog.
*/
Expand Down Expand Up @@ -127,7 +121,6 @@ export const Dialog = forwardRef<HTMLDialogElement, DialogProps>(
onCloseEnd,
closeButtonLabel,
className,
initialFocusRef,
preventOutsideClickRefs,
preventOutsideClickClose = false,
hideCloseButton = false,
Expand All @@ -144,31 +137,6 @@ export const Dialog = forwardRef<HTMLDialogElement, DialogProps>(
const lastFocusedElementRef = useRef<HTMLElement | null>(null);

// Focus Management
useEffect(() => {
const dialogElement = dialogRef.current;
let timeoutId: NodeJS.Timeout;
if (open && dialogElement) {
timeoutId = setTimeout(() => {
if (initialFocusRef?.current) {
initialFocusRef?.current?.focus({ preventScroll: true });
} else {
const firstFocusableElement = getFirstFocusableElement(
dialogElement,
!hideCloseButton,
);
if (firstFocusableElement) {
firstFocusableElement?.focus({ preventScroll: true });
} else {
dialogElement.focus();
}
}
}, animationDurationRef.current);
}
return () => {
clearTimeout(timeoutId);
};
}, [open, initialFocusRef, hideCloseButton, animationDurationRef.current]);

useEffect(() => {
// save the opening element to restore focus after the dialog closes
if (open) {
Expand Down Expand Up @@ -377,15 +345,15 @@ export const Dialog = forwardRef<HTMLDialogElement, DialogProps>(
}}
{...rest}
>
{open &&
(typeof children === 'function'
? children?.({ onClose: onCloseEnd })
: children)}
{!hideCloseButton && (
<CloseButton onClick={handleDialogClose} className={classes.close}>
{closeButtonLabel}
</CloseButton>
)}
{open &&
(typeof children === 'function'
? children?.({ onClose: onCloseEnd })
: children)}
</dialog>
</>
);
Expand Down
113 changes: 0 additions & 113 deletions packages/circuit-ui/components/Dialog/DialogService.spec.tsx

This file was deleted.

48 changes: 0 additions & 48 deletions packages/circuit-ui/components/Dialog/DialogService.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

'use client';

import { type FC, type ReactNode, type SVGProps, useId, useRef } from 'react';
import { type FC, type ReactNode, type SVGProps, useId } from 'react';

import type { ClickEvent } from '../../types/events.js';
import { Image, type ImageProps } from '../Image/index.js';
Expand Down Expand Up @@ -93,14 +93,12 @@ export const NotificationModal = ({
}

const headlineId = useId();
const initialFocusRef = useRef<HTMLButtonElement>(null);
const dialogProps = {
className: clsx(className, classes.base),
closeButtonLabel,
'aria-labelledby': headlineId,
preventClose,
onClose,
initialFocusRef,
...props,
};

Expand Down Expand Up @@ -134,8 +132,12 @@ export const NotificationModal = ({
},
secondary: actions.secondary && {
...actions.secondary,
// @ts-expect-error ref is a valid prop on a button
ref: initialFocusRef,
// @ts-expect-error React purposefully breaks the `autoFocus`
// property. Using the lowercase DOM attribute name instead
// forces it to be added to the DOM but will produce a console
// warning that can be safely ignored.
// https://github.com/facebook/react/issues/23301
autofocus: 'true',
onClick: wrapOnClick(actions.secondary.onClick),
},
}}
Expand Down
Loading