mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.88k stars 32.26k forks source link

[Switch] doesn't respect readOnly prop #37753

Open KarafiziArtur opened 1 year ago

KarafiziArtur commented 1 year ago

Duplicates

Latest version

Steps to reproduce ๐Ÿ•น

Link to live example: https://codesandbox.io/s/recursing-matsumoto-wjv26k

Steps:

  1. You see that readOnly props have already passed to the Switch component and to inputProps
  2. Interact with the Switch component in the UI
  3. You see that the Switch doesn't respect the readOnly props as expected

Current behavior ๐Ÿ˜ฏ

Switch doesn't respect readOnly prop but it supports it as described in interface SwitchBaseProps - readOnly?: boolean; in /node_modules/@mui/material/internal/SwitchBase.d.ts:69

Screenshot 2023-06-29 at 11 56 41

Expected behavior ๐Ÿค”

In Swithc component the prop readOnly works as expected

Context ๐Ÿ”ฆ

No response

Your environment ๐ŸŒŽ

npx @mui/envinfo ``` System: OS: macOS 13.4 Binaries: Node: 18.8.0 - ~/.nvm/versions/node/v18.8.0/bin/node Yarn: 1.22.19 - ~/.yarn/bin/yarn npm: 8.18.0 - ~/.nvm/versions/node/v18.8.0/bin/npm Browsers: Chrome: 114.0.5735.198 Edge: 112.0.1722.68 Firefox: Not Found Safari: 16.5 npmPackages: @emotion/react: 11.11.1 @emotion/styled: 11.11.0 @mui/base: 5.0.0-beta.4 @mui/core-downloads-tracker: 5.13.4 @mui/icons-material: 5.11.16 @mui/lab: 5.0.0-alpha.134 @mui/material: 5.13.6 @mui/private-theming: 5.13.1 @mui/styled-engine: 5.13.2 @mui/styles: 5.13.2 @mui/system: 5.13.6 @mui/types: 7.2.4 @mui/utils: 5.13.6 @types/react: 18.2.14 => 18.2.14 react: 18.2.0 react-dom: 18.2.0 styled-components: 5.3.11 typescript: 5.1.3 => 5.1.3 ```
sai6855 commented 1 year ago

i'm picking this up

sai6855 commented 1 year ago

@KarafiziArtur This is expected behaviour, readOnly prop only prevents changing value but not interacting with field. This applies to native html input element also.

In the below sandbox, readOnly is applied to inputs with type='text' and type='checkbox'. if you try to edit content in input with type text, input doesn't allow that. but you can check and uncheck input with type checkbox.

https://codesandbox.io/s/eager-glade-z4d73m?file=/src/App.tsx

KarafiziArtur commented 1 year ago

@KarafiziArtur This is expected behaviour, readOnly prop only prevents changing value but not interacting with field. This applies to native html input element also.

Thanks. But I don't think that it's expected behaviour because the value of Switch is still changing

sai6855 commented 1 year ago

Thanks. But I don't think that it's expected behaviour because the value of Switch is still changing

It's not the value that's changing but checked prop of switch is changing

KarafiziArtur commented 1 year ago

It's not the value that's changing but checked prop of switch is changing

So the checked is not the value - that's true, but we don't use value for the checkbox, it's the checked that is used in most of the cases

sai6855 commented 1 year ago

Yup you are right, readOnly stops only value to be changed not checked. Since checked is used for switch not value, you can still toggle it as readOnly cannot control checked prop

KarafiziArtur commented 1 year ago

Yup you are right, readOnly stops only value to be changed not checked. Since checked is used for switch not value, you can still toggle it as readOnly cannot control checked prop

Thanks. So is it possible to fix that? To make readOnly prop to prevent evaluating the onChange event - because it's not expected behaviour in our team's opinion

sai6855 commented 1 year ago

you can programatically stop changing checked prop based on readOnly.

sandbox: https://codesandbox.io/s/affectionate-jerry-97rsw8?file=/src/App.tsx

KarafiziArtur commented 1 year ago

you can programatically stop changing checked prop based on readOnly. sandbox: https://codesandbox.io/s/affectionate-jerry-97rsw8?file=/src/App.tsx

Yes, we can. We also ended up with the solution to set disabled={disabled || readOnly} but it's not correct to do some workarounds on our side I'm asking about fixing that in the MUI components

sai6855 commented 1 year ago

It is default behaviour, i don't think it will be fixed in mui components

KarafiziArtur commented 1 year ago

Do you think that it's correct the default behaviour? In my opinion, even default behaviour could be not correct. Things are going to change

sai6855 commented 1 year ago

It definitely doesn't look intuitive, i agree.

berenicestr commented 9 months ago

I have the same issue. I also found other issue. The switch component doesnโ€™t focus as the next element in tab index. I have tried with the tabIndex and autoFocus props and doesnโ€™t solve it.

Code: https://stackblitz.com/edit/react-ujnl8h?file=Demo.tsx