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

[Radio] Radio's ButtonBase has role="button" when using styled-components #44185

Open
Lukas-Kullmann opened this issue Oct 22, 2024 · 3 comments
Assignees
Labels
component: ButtonBase The React component. component: radio This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it package: styled-engine-sc Specific to styled-components

Comments

@Lukas-Kullmann
Copy link

Lukas-Kullmann commented Oct 22, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/devbox/nervous-oskar-xx7s2l

Steps:

  1. Use mui together with styled-components (i.e. aliasing @mui/styled-engine with @mui/styled-engine-sc)
  2. Use a <Radio /> component

Current behavior

The radio's ButtonBase has a role="button". This leads to an accessibility violation since there is a radio inside of a button.
The HTML basically looks like this:

<span 
  class="MuiButtonBase-root MuiRadio-root [...]" 
  role="button">
    <input class="sc-egkSDF jccrsl PrivateSwitchBase-input" type="radio">
    <span class="sc-fAUdSK cyWAma">
      <svg>[...]</svg>
    </span>
    <span class="MuiTouchRipple-root-hfoSaC iZusHU MuiTouchRipple-root"></span>
</span>

Note that there is a role="button" on the outermost <span>. This should not be there and is in fact not there if you use the "regular" emotion adapter for @mui/styled-engine.

Expected behavior

There is no role="button" on the ButtonBase for a <Radio />.

Context

I think the problem is that <SwitchBase /> passes role={undefined} to the <SwitchBaseRoot /> (which is a styled <ButtonBase />) here: https://github.com/mui/material-ui/blob/master/packages/mui-material/src/internal/SwitchBase.js#L177
And I guess that styled-components just swallows properties with undefined values (since version 6?).
It used to work with mui v5 and styled-components v5.

This bug also affects other components that use <SwitchBase /> (like <Checkbox /> or <Switch />).

Your environment

npx @mui/envinfo
  System:
    OS: Linux 6.1 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
  Binaries:
    Node: 20.11.0 - /usr/local/bin/node
    npm: 10.2.4 - /usr/local/bin/npm
    pnpm: 8.15.1 - /usr/local/share/npm-global/bin/pnpm
  Browsers:
    Chrome: Not Found
  npmPackages:
    @mui/core-downloads-tracker:  6.1.5 
    @mui/envinfo: 2.0.28 => 2.0.28 
    @mui/material: ^6.1.5 => 6.1.5 
    @mui/private-theming:  6.1.5 
    @mui/styled-engine:  6.1.5 
    @mui/styled-engine-sc: ^6.1.5 => 6.1.5 
    @mui/system:  6.1.5 
    @mui/types:  7.2.18 
    @mui/utils:  6.1.5 
    @types/react: 18.3.11 => 18.3.11 
    react: 18.3.1 => 18.3.1 
    react-dom: 18.3.1 => 18.3.1 
    styled-components: ^6.1.13 => 6.1.13 

Search keywords: radio styled-components button

@Lukas-Kullmann Lukas-Kullmann added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 22, 2024
@zannager zannager added component: radio This is the name of the generic UI component, not the React module! component: ButtonBase The React component. labels Oct 22, 2024
@Lukas-Kullmann
Copy link
Author

As a workaround, you can pass role="" to the radio: <Radio role="" />

@mj12albert mj12albert added external dependency Blocked by external dependency, we can’t do anything about it and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 23, 2024
@aarongarciah
Copy link
Member

As you pointed out, this only happens with styled-components:

Do you have any idea why this is happening, @mnajdova?

@aarongarciah aarongarciah added the package: styled-engine-sc Specific to styled-components label Oct 23, 2024
@mnajdova
Copy link
Member

@Lukas-Kullmann thanks for investigating this further. I ended up on the same issue as you - styled-components stopped propagating props that have undefined as a value to the underlying component, so we don't have a way to "remove" the role prop. This is the simplest reproduction: https://codesandbox.io/p/devbox/radio-styled-components-forked-9j9fpz?workspaceId=02ae2942-9632-4b06-9a20-6ccadc8dcb97

We can't support the attrs API as we try to normalize and use APIs that are available both for Emotion and styled-components. I am surprised that the issue in styled-components didn't get much more traction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ButtonBase The React component. component: radio This is the name of the generic UI component, not the React module! external dependency Blocked by external dependency, we can’t do anything about it package: styled-engine-sc Specific to styled-components
Projects
None yet
Development

No branches or pull requests

5 participants