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

[core] Remove useIsFocusVisible util #42467

Merged
merged 9 commits into from
Jul 1, 2024
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: 6 additions & 1 deletion packages/mui-base/src/Menu/Menu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,12 @@ describe('<Menu />', () => {
});
});

it('perf: does not rerender menu items unnecessarily', async () => {
it('perf: does not rerender menu items unnecessarily', async function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const renderItem1Spy = spy();
const renderItem2Spy = spy();
const renderItem3Spy = spy();
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-base/src/Select/Select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,12 @@ describe('<Select />', () => {
expect(selectButton).to.have.text('1, 2');
});

it('perf: does not rerender options unnecessarily', async () => {
it('perf: does not rerender options unnecessarily', async function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const renderOption1Spy = spy();
const renderOption2Spy = spy();
const renderOption3Spy = spy();
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-base/src/Slider/Slider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,12 @@ describe('<Slider />', () => {
expect(screen.getByTestId('value-label')).to.have.text('20');
});

it('should provide focused state to the slotProps.thumb', () => {
it('should provide focused state to the slotProps.thumb', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const { getByTestId } = render(
<Slider
defaultValue={[20, 40]}
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-base/src/useButton/useButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,12 @@ describe('useButton', () => {
expect(handleClickExternal.callCount).to.equal(0);
});

it('handles onFocusVisible and does not include it in the root props', () => {
it('handles onFocusVisible and does not include it in the root props', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

interface WithFocusVisibleHandler {
onFocusVisible: React.FocusEventHandler;
}
Expand Down
22 changes: 4 additions & 18 deletions packages/mui-base/src/useButton/useButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import * as React from 'react';
import {
unstable_useForkRef as useForkRef,
unstable_useIsFocusVisible as useIsFocusVisible,
unstable_isFocusVisible as isFocusVisible,
} from '@mui/utils';
import {
UseButtonParameters,
Expand Down Expand Up @@ -38,22 +38,11 @@ export function useButton(parameters: UseButtonParameters = {}): UseButtonReturn

const [active, setActive] = React.useState<boolean>(false);

const {
isFocusVisibleRef,
onFocus: handleFocusVisible,
onBlur: handleBlurVisible,
ref: focusVisibleRef,
} = useIsFocusVisible();

const [focusVisible, setFocusVisible] = React.useState(false);
if (disabled && !focusableWhenDisabled && focusVisible) {
setFocusVisible(false);
}

React.useEffect(() => {
isFocusVisibleRef.current = focusVisible;
}, [focusVisible, isFocusVisibleRef]);

const [rootElementName, updateRootElementName] = useRootElementName({
rootElementName: rootElementNameProp ?? (href || to ? 'a' : undefined),
componentName: 'Button',
Expand All @@ -68,9 +57,7 @@ export function useButton(parameters: UseButtonParameters = {}): UseButtonReturn
};

const createHandleBlur = (otherHandlers: EventHandlers) => (event: React.FocusEvent) => {
handleBlurVisible(event);

if (isFocusVisibleRef.current === false) {
if (!isFocusVisible(event.target)) {
setFocusVisible(false);
}

Expand All @@ -84,8 +71,7 @@ export function useButton(parameters: UseButtonParameters = {}): UseButtonReturn
buttonRef.current = event.currentTarget;
}

handleFocusVisible(event);
if (isFocusVisibleRef.current === true) {
if (isFocusVisible(event.target)) {
setFocusVisible(true);
otherHandlers.onFocusVisible?.(event);
}
Expand Down Expand Up @@ -176,7 +162,7 @@ export function useButton(parameters: UseButtonParameters = {}): UseButtonReturn
}
};

const handleRef = useForkRef(updateRootElementName, externalRef, focusVisibleRef, buttonRef);
const handleRef = useForkRef(updateRootElementName, externalRef, buttonRef);

interface AdditionalButtonProps {
type?: React.ButtonHTMLAttributes<HTMLButtonElement>['type'];
Expand Down
17 changes: 4 additions & 13 deletions packages/mui-base/src/useSlider/useSlider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
unstable_useEnhancedEffect as useEnhancedEffect,
unstable_useEventCallback as useEventCallback,
unstable_useForkRef as useForkRef,
unstable_useIsFocusVisible as useIsFocusVisible,
unstable_isFocusVisible as isFocusVisible,
visuallyHidden,
clamp,
} from '@mui/utils';
Expand Down Expand Up @@ -265,32 +265,23 @@ export function useSlider(parameters: UseSliderParameters): UseSliderReturnValue

const marksValues = (marks as Mark[]).map((mark: Mark) => mark.value);

const {
isFocusVisibleRef,
onBlur: handleBlurVisible,
onFocus: handleFocusVisible,
ref: focusVisibleRef,
} = useIsFocusVisible();
const [focusedThumbIndex, setFocusedThumbIndex] = React.useState(-1);

const sliderRef = React.useRef<HTMLSpanElement>();
const handleFocusRef = useForkRef(focusVisibleRef, sliderRef);
const handleRef = useForkRef(ref, handleFocusRef);
const handleRef = useForkRef(ref, sliderRef);

const createHandleHiddenInputFocus =
(otherHandlers: EventHandlers) => (event: React.FocusEvent) => {
const index = Number(event.currentTarget.getAttribute('data-index'));
handleFocusVisible(event);
if (isFocusVisibleRef.current === true) {
if (isFocusVisible(event.target)) {
setFocusedThumbIndex(index);
}
setOpen(index);
otherHandlers?.onFocus?.(event);
};
const createHandleHiddenInputBlur =
(otherHandlers: EventHandlers) => (event: React.FocusEvent) => {
handleBlurVisible(event);
if (isFocusVisibleRef.current === false) {
if (!isFocusVisible(event.target)) {
setFocusedThumbIndex(-1);
}
setOpen(-1);
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-base/src/useSwitch/useSwitch.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@ describe('useSwitch', () => {
expect(handleChange.callCount).to.equal(1);
});

it('should call focus event handlers if focus events are fired', () => {
it('should call focus event handlers if focus events are fired', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const handleBlur = spy();
const handleFocus = spy();
const handleFocusVisible = spy();
Expand Down
22 changes: 4 additions & 18 deletions packages/mui-base/src/useSwitch/useSwitch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from 'react';
import {
unstable_useControlled as useControlled,
unstable_useForkRef as useForkRef,
unstable_useIsFocusVisible as useIsFocusVisible,
unstable_isFocusVisible as isFocusVisible,
} from '@mui/utils';
import { UseSwitchParameters, UseSwitchReturnValue } from './useSwitch.types';

Expand Down Expand Up @@ -51,22 +51,11 @@ export function useSwitch(props: UseSwitchParameters): UseSwitchReturnValue {
otherProps.onChange?.(event);
};

const {
isFocusVisibleRef,
onBlur: handleBlurVisible,
onFocus: handleFocusVisible,
ref: focusVisibleRef,
} = useIsFocusVisible();

const [focusVisible, setFocusVisible] = React.useState(false);
if (disabled && focusVisible) {
setFocusVisible(false);
}

React.useEffect(() => {
isFocusVisibleRef.current = focusVisible;
}, [focusVisible, isFocusVisibleRef]);

const inputRef = React.useRef<HTMLInputElement | null>(null);

const createHandleFocus =
Expand All @@ -77,8 +66,7 @@ export function useSwitch(props: UseSwitchParameters): UseSwitchReturnValue {
inputRef.current = event.currentTarget;
}

handleFocusVisible(event);
if (isFocusVisibleRef.current === true) {
if (isFocusVisible(event.target)) {
setFocusVisible(true);
onFocusVisible?.(event);
}
Expand All @@ -90,17 +78,15 @@ export function useSwitch(props: UseSwitchParameters): UseSwitchReturnValue {
const createHandleBlur =
(otherProps: React.InputHTMLAttributes<HTMLInputElement>) =>
(event: React.FocusEvent<HTMLInputElement>) => {
handleBlurVisible(event);

if (isFocusVisibleRef.current === false) {
if (!isFocusVisible(event.target)) {
setFocusVisible(false);
}

onBlur?.(event);
otherProps.onBlur?.(event);
};

const handleInputRef = useForkRef(focusVisibleRef, inputRef);
const handleInputRef = useForkRef(inputRef);
aarongarciah marked this conversation as resolved.
Show resolved Hide resolved

const getInputProps: UseSwitchReturnValue['getInputProps'] = (otherProps = {}) => ({
checked: checkedProp,
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-joy/src/Link/Link.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,12 @@ describe('<Link />', () => {
});

describe('keyboard focus', () => {
it('should add the focusVisible class when focused', () => {
it('should add the focusVisible class when focused', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const { container } = render(<Link href="/">Home</Link>);
const anchor = container.querySelector('a');

Expand Down
18 changes: 4 additions & 14 deletions packages/mui-joy/src/Link/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ import { unstable_composeClasses as composeClasses } from '@mui/base';
import { OverridableComponent } from '@mui/types';
import {
unstable_capitalize as capitalize,
unstable_useForkRef as useForkRef,
unstable_useIsFocusVisible as useIsFocusVisible,
unstable_isFocusVisible as isFocusVisible,
unstable_isMuiElement as isMuiElement,
} from '@mui/utils';
import { unstable_extendSxProp as extendSxProp } from '@mui/system';
Expand Down Expand Up @@ -216,26 +215,17 @@ const Link = React.forwardRef(function Link(inProps, ref) {

const level = nesting || inheriting ? inProps.level || 'inherit' : levelProp;

const {
isFocusVisibleRef,
onBlur: handleBlurVisible,
onFocus: handleFocusVisible,
ref: focusVisibleRef,
} = useIsFocusVisible();
const [focusVisible, setFocusVisible] = React.useState<boolean>(false);
const handleRef = useForkRef(ref, focusVisibleRef) as React.Ref<HTMLAnchorElement>;
const handleBlur = (event: React.FocusEvent<HTMLAnchorElement>) => {
handleBlurVisible(event);
if (isFocusVisibleRef.current === false) {
if (!isFocusVisible(event.target)) {
setFocusVisible(false);
}
if (onBlur) {
onBlur(event);
}
};
const handleFocus = (event: React.FocusEvent<HTMLAnchorElement>) => {
handleFocusVisible(event);
if (isFocusVisibleRef.current === true) {
if (isFocusVisible(event.target)) {
setFocusVisible(true);
}
if (onFocus) {
Expand Down Expand Up @@ -263,7 +253,7 @@ const Link = React.forwardRef(function Link(inProps, ref) {
onBlur: handleBlur,
onFocus: handleFocus,
},
ref: handleRef,
ref,
className: classes.root,
elementType: LinkRoot,
externalForwardedProps,
Expand Down
7 changes: 6 additions & 1 deletion packages/mui-joy/src/ListItemButton/ListItemButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ describe('Joy <ListItemButton />', () => {
});

describe('prop: focusVisibleClassName', () => {
it('should have focusVisible classes', () => {
it('should have focusVisible classes', function test() {
if (/jsdom/.test(window.navigator.userAgent)) {
// JSDOM doesn't support :focus-visible
this.skip();
}

const { getByRole } = render(<ListItemButton />);
const button = getByRole('button');

Expand Down
Loading