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.

Sign up for GitHub

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

Jump to bottom

[useButton] Modernize implementation #643

Merged
merged 3 commits into from
Oct 2, 2024
Merged

[useButton] Modernize implementation #643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5e277f3
Move types to namespaces and remove focus-visible polyfill
michaldudak Sep 22, 2024
dc4ec99
Rename rootRef to buttonRef
michaldudak Sep 30, 2024
8e90c80
root -> button
michaldudak Oct 1, 2024
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
5 changes: 2 additions & 3 deletions packages/mui-base/src/Menu/CheckboxItem/MenuCheckboxItem.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,17 @@ describe('<Menu.CheckboxItem />', () => {
fireEvent.keyDown(menuItems[0], { key: 'ArrowDown' }); // highlights '2'

// React renders twice in strict mode, so we expect twice the number of spy calls
// Also, useButton's focusVisible polyfill causes an extra render when focus is gained/lost.

await waitFor(
() => {
expect(renderItem1Spy.callCount).to.equal(4); // '1' rerenders as it loses highlight
expect(renderItem1Spy.callCount).to.equal(2); // '1' rerenders as it loses highlight
},
{ timeout: 1000 },
);

await waitFor(
() => {
expect(renderItem2Spy.callCount).to.equal(4); // '2' rerenders as it receives highlight
expect(renderItem2Spy.callCount).to.equal(2); // '2' rerenders as it receives highlight
},
{ timeout: 1000 },
);
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-base/src/Menu/Item/MenuItem.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ describe('<Menu.Item />', () => {

await waitFor(
() => {
expect(renderItem1Spy.callCount).to.equal(4); // '1' rerenders as it loses highlight
expect(renderItem1Spy.callCount).to.equal(2); // '1' rerenders as it loses highlight
},
{ timeout: 1000 },
);
await waitFor(
() => {
expect(renderItem2Spy.callCount).to.equal(4); // '2' rerenders as it receives highlight
expect(renderItem2Spy.callCount).to.equal(2); // '2' rerenders as it receives highlight
},
{ timeout: 1000 },
);
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-base/src/Menu/Item/useMenuItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ export function useMenuItem(params: useMenuItem.Parameters): useMenuItem.ReturnV
typingRef,
} = params;

const { getRootProps: getButtonProps, rootRef: mergedRef } = useButton({
const { getButtonProps, buttonRef: mergedRef } = useButton({
disabled,
focusableWhenDisabled: true,
rootRef: externalRef,
buttonRef: externalRef,
});

const getRootProps = React.useCallback(
Expand Down
5 changes: 2 additions & 3 deletions packages/mui-base/src/Menu/RadioItem/MenuRadioItem.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,17 @@ describe('<Menu.RadioItem />', () => {
fireEvent.keyDown(menuItems[0], { key: 'ArrowDown' }); // highlights '2'

// React renders twice in strict mode, so we expect twice the number of spy calls
// Also, useButton's focusVisible polyfill causes an extra render when focus is gained/lost.

await waitFor(
() => {
expect(renderItem1Spy.callCount).to.equal(4); // '1' rerenders as it loses highlight
expect(renderItem1Spy.callCount).to.equal(2); // '1' rerenders as it loses highlight
},
{ timeout: 1000 },
);

await waitFor(
() => {
expect(renderItem2Spy.callCount).to.equal(4); // '2' rerenders as it receives highlight
expect(renderItem2Spy.callCount).to.equal(2); // '2' rerenders as it receives highlight
},
{ timeout: 1000 },
);
Expand Down
10 changes: 5 additions & 5 deletions packages/mui-base/src/Menu/Trigger/useMenuTrigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr

const mergedRef = useForkRef(externalRef, triggerRef);

const { getRootProps: getButtonRootProps, rootRef: buttonRootRef } = useButton({
const { getButtonProps, buttonRef } = useButton({
disabled,
focusableWhenDisabled: false,
rootRef: mergedRef,
buttonRef: mergedRef,
});

const handleRef = useForkRef(buttonRootRef, setTriggerElement);
const handleRef = useForkRef(buttonRef, setTriggerElement);
const ignoreNextClick = React.useRef(false);

const getRootProps = React.useCallback(
Expand Down Expand Up @@ -73,10 +73,10 @@ export function useMenuTrigger(parameters: useMenuTrigger.Parameters): useMenuTr
}
},
},
getButtonRootProps(),
getButtonProps(),
);
},
[getButtonRootProps, handleRef, open, setOpen, setClickAndDragEnabled],
[getButtonProps, handleRef, open, setOpen, setClickAndDragEnabled],
);

return React.useMemo(
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/Tabs/Tab/useTab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ function useTab(parameters: UseTabParameters): UseTabReturnValue {
item: value,
});

const { getRootProps: getButtonProps, rootRef: buttonRefHandler } = useButton({
const { getButtonProps, buttonRef: buttonRefHandler } = useButton({
disabled,
focusableWhenDisabled: true,
type: 'button',
Expand Down
1 change: 0 additions & 1 deletion packages/mui-base/src/useButton/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export { useButton } from './useButton';
export * from './useButton.types';
200 changes: 17 additions & 183 deletions packages/mui-base/src/useButton/useButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,212 +1,46 @@
import * as React from 'react';
import { createRenderer, fireEvent } from '@mui/internal-test-utils';
import { createRenderer } from '@mui/internal-test-utils';
import { expect } from 'chai';
import { spy } from 'sinon';
import { useButton } from '@base_ui/react/useButton';

describe('useButton', () => {
const { render } = createRenderer();

describe('state: active', () => {
describe('when using a button element', () => {
it('is set when triggered by mouse', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return <button {...getRootProps()} className={active ? 'active' : ''} />;
}

const { getByRole } = render(<TestComponent />);
const button = getByRole('button');
fireEvent.mouseDown(button);
expect(button).to.have.class('active');
fireEvent.mouseUp(button);
expect(button).not.to.have.class('active');
});

it('is set when triggered by keyboard', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return <button {...getRootProps()} className={active ? 'active' : ''} />;
}

const { getByRole } = render(<TestComponent />);
const button = getByRole('button');
button.focus();
fireEvent.keyDown(button, { key: ' ' });
expect(button).to.have.class('active');
fireEvent.keyUp(button, { key: ' ' });
expect(button).not.to.have.class('active');
});

it('is set when clicked on an element inside the button', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return (
<button {...getRootProps()} className={active ? 'active' : ''}>
<span>Click here</span>
</button>
);
}

const { getByText, getByRole } = render(<TestComponent />);
const span = getByText('Click here');
const button = getByRole('button');
fireEvent.mouseDown(span);
expect(button).to.have.class('active');
});

it('is unset when mouse button is released above another element', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return (
<div data-testid="parent">
<button {...getRootProps()} className={active ? 'active' : ''} />
</div>
);
}

const { getByRole, getByTestId } = render(<TestComponent />);
const button = getByRole('button');
const background = getByTestId('parent');
fireEvent.mouseDown(button);
expect(button).to.have.class('active');
fireEvent.mouseUp(background);
expect(button).not.to.have.class('active');
});
});

describe('when using a span element', () => {
it('is set when triggered by mouse', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return <span {...getRootProps()} className={active ? 'active' : ''} />;
}

const { getByRole } = render(<TestComponent />);
const button = getByRole('button');
fireEvent.mouseDown(button);
expect(button).to.have.class('active');
fireEvent.mouseUp(button);
expect(button).not.to.have.class('active');
});

it('is set when triggered by keyboard', () => {
function TestComponent() {
const buttonRef = React.useRef(null);
const { active, getRootProps } = useButton({ rootRef: buttonRef });

return <span {...getRootProps()} className={active ? 'active' : ''} />;
}

const { getByRole } = render(<TestComponent />);
const button = getByRole('button');
button.focus();
fireEvent.keyDown(button, { key: ' ' });
expect(button).to.have.class('active');
fireEvent.keyUp(button, { key: ' ' });
expect(button).not.to.have.class('active');
});
});

describe('event handlers', () => {
interface WithClickHandler {
onClick: React.MouseEventHandler;
}

it('calls them when provided in props', () => {
function TestComponent(props: WithClickHandler) {
const ref = React.useRef(null);
const { getRootProps } = useButton({ ...props, rootRef: ref });
return <button {...getRootProps()} />;
}

const handleClick = spy();

const { getByRole } = render(<TestComponent onClick={handleClick} />);
fireEvent.click(getByRole('button'));

expect(handleClick.callCount).to.equal(1);
});

it('calls them when provided in getRootProps()', () => {
const handleClick = spy();

function TestComponent() {
const ref = React.useRef(null);
const { getRootProps } = useButton({ rootRef: ref });
return <button {...getRootProps({ onClick: handleClick })} />;
}

const { getByRole } = render(<TestComponent />);
fireEvent.click(getByRole('button'));

expect(handleClick.callCount).to.equal(1);
});

it('calls the one provided in getRootProps() when both props and getRootProps have ones', () => {
const handleClickExternal = spy();
const handleClickInternal = spy();

function TestComponent(props: WithClickHandler) {
const ref = React.useRef(null);
const { getRootProps } = useButton({ ...props, rootRef: ref });
return <button {...getRootProps({ onClick: handleClickInternal })} />;
}

const { getByRole } = render(<TestComponent onClick={handleClickExternal} />);
fireEvent.click(getByRole('button'));

expect(handleClickInternal.callCount).to.equal(1);
expect(handleClickExternal.callCount).to.equal(0);
});
});
});

describe('tabIndex', () => {
it('does not return tabIndex in getRootProps when host component is BUTTON', () => {
it('does not return tabIndex in getButtonProps when host component is BUTTON', () => {
function TestComponent() {
const ref = React.useRef(null);
const { getRootProps } = useButton({ rootRef: ref });
const buttonRef = React.useRef(null);
const { getButtonProps } = useButton({ buttonRef });

expect(getRootProps().tabIndex).to.equal(undefined);
expect(getButtonProps().tabIndex).to.equal(undefined);

return <button {...getRootProps()} />;
return <button {...getButtonProps()} />;
}

const { getByRole } = render(<TestComponent />);
expect(getByRole('button')).to.have.property('tabIndex', 0);
});

it('returns tabIndex in getRootProps when host component is not BUTTON', () => {
it('returns tabIndex in getButtonProps when host component is not BUTTON', () => {
function TestComponent() {
const ref = React.useRef(null);
const { getRootProps } = useButton({ rootRef: ref });
const buttonRef = React.useRef(null);
const { getButtonProps } = useButton({ buttonRef });

expect(getRootProps().tabIndex).to.equal(ref.current ? 0 : undefined);
expect(getButtonProps().tabIndex).to.equal(buttonRef.current ? 0 : undefined);

return <span {...getRootProps()} />;
return <span {...getButtonProps()} />;
}

const { getByRole } = render(<TestComponent />);
expect(getByRole('button')).to.have.property('tabIndex', 0);
});

it('returns tabIndex in getRootProps if it is explicitly provided', () => {
it('returns tabIndex in getButtonProps if it is explicitly provided', () => {
const customTabIndex = 3;
function TestComponent() {
const ref = React.useRef(null);
const { getRootProps } = useButton({ rootRef: ref, tabIndex: customTabIndex });
return <button {...getRootProps()} />;
const buttonRef = React.useRef(null);
const { getButtonProps } = useButton({ buttonRef, tabIndex: customTabIndex });
return <button {...getButtonProps()} />;
}

const { getByRole } = render(<TestComponent />);
Expand All @@ -218,8 +52,8 @@ describe('useButton', () => {
it('are passed to the host component', () => {
const buttonTestId = 'button-test-id';
function TestComponent() {
const { getRootProps } = useButton({});
return <button {...getRootProps({ 'data-testid': buttonTestId })} />;
const { getButtonProps } = useButton({});
return <button {...getButtonProps({ 'data-testid': buttonTestId })} />;
}

const { getByRole } = render(<TestComponent />);
Expand Down
Loading