Skip to content

Commit

Permalink
Remove custom initial focus logic (#2910)
Browse files Browse the repository at this point in the history
  • Loading branch information
connor-baer authored and sirineJ committed Jan 28, 2025
1 parent fbeb421 commit 96e789e
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 205 deletions.
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

0 comments on commit 96e789e

Please sign in to comment.