[ButtonBase] Fix native button detection by mj12albert · Pull Request #47985 · mui/material-ui (original) (raw)

The "is native button" check only checked component !== 'button'. When component={StyledButton} where <StyledButton> renders a <button> element, event handlers incorrectly apply the logic for non-native buttons

I have followed (at least) the PR section of the contributing guide.

Comment on lines +259 to +269

const handleClick = useEventCallback((event) => {
if (disabled) {
event.preventDefault();
return;
}

if (onClick) {
onClick(event);
}
});

Choose a reason for hiding this comment

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

I think there is a risk to intercept the click event here given that the ButtonBase is used by a lot of components. What's the issue it fixes? or is this change align with Base UI?

Choose a reason for hiding this comment

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

This possibly relates more to #47989 than this PR

Currently when disabled we are relying on CSS pointer-events none to "disable" the click. However I want to extract these event handlers to a hook so that new ChipButton/ChipDelete can reuse the logic, but may not necessarily want to set pointer-events: none on them @siriwatknp

mj12albert changed the title~~[ButtonBase] Fix native button detection and disabled state~~ [ButtonBase] Fix native button detection

Mar 16, 2026


const button = screen.getByRole('button');

await act(async () => {

Choose a reason for hiding this comment

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

Would user.tab() work here instead?

Choose a reason for hiding this comment

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

This is kind of an edge case to begin with since it's a disabled non-native button, because it's disabled a Tab wouldn't land focus there to begin with, only programmatic focus is possible (added a comment in the test)

button.focus();
});

fireEvent.keyUp(button, { key: ' ' });

Choose a reason for hiding this comment

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

having just a keyUp is definitely not what is going to happen in production, even though the test logic is sound. would it work if we use user.keyboard? Again, the test makes sense, it's fine, I'm just asking if we can mimic the user behavior better here.

Choose a reason for hiding this comment

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

👍 changed to user.keyboard(' ')

Cherry-pick PRs will be created targeting branches: v7.x

siriwatknp pushed a commit to siriwatknp/material-ui that referenced this pull request

Mar 18, 2026

mj12albert added a commit that referenced this pull request

Mar 18, 2026

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})