Material-ui: [SwitchBase] Ignores FormControl disabled state

Created on 18 Jan 2020  路  5Comments  路  Source: mui-org/material-ui

  • [x] The issue is present in the latest release.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 馃槸

When the [FormControl] component is rendered with the disabled={true} prop, the children [Checkbox], [Radio], and [Switch] components remains enabled

Expected Behavior 馃

All the children components of [FormControl] that use [FormControlContext] should be disabled if the [FormControl] components is disabled

Steps to Reproduce 馃暪

This sandbox demonstrates the issue that appeared from v4.6.1 due to this pull request.

This sandbox clearly shows that everything was working correctly in v4.6.0

Context 馃敠

The [SwitchBase] component checks whether it has own disabled prop. If it doesn't have it, the disabled from [FormControlContext] is used (please see source code)

The issue is that the [Checkbox], [Radio], and [Switch] components were configured to use the disabled prop set to false by default (please see checkbox, radio, and switch changes).

Therefore, the [SwitchBase] component never receives the disabled={undefined} prop, which leads to non-usage of the disabled value from [FormControlContext]

Your Environment 馃寧

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.8.3 |
| React | v16.12.0 |

bug 馃悰 good first issue

Most helpful comment

IIRC, it should be possible to add it to each component's propValues in framerConfig.js. Adding it to the component directly was intended to fix the problem "at source", while also allowing the default to be documented in the prop docs; but clearly it backfired in this instance!

All 5 comments

Speaking of this issue, I can see three options I could fix this issue

  1. Change the [SwitchBase] logic to first check if the [FormControl] component is disabled. If not, fallback to own disabled prop
  let disabled = muiFormControl && muiFormControl.disabled || disabledProp;
  1. We could also reset default disabled prop back to undefined in the [Checkbox], [Radio], and [Switch] components, but I'm good with disabled = false by default
  2. I have a hard time imagining someone uses [Checkbox], [Radio], and [Switch] inside a form without [FormControlLabel], which doesn't have this issue, bacause it takes [FormControlContext], desides which disabled prop to use and then passes it directly to the control component. What I'm getting at is that we might get rid of the muiFormControl.disabled check inside the [SwitchBase]

Before I get started working on this issue, I really wanna hear your guys opinion

  1. This change was intentional in support of the Framer components that require a default (albeit with this unintended consequence).

  2. Me too, but we do document it as an option. @oliviertassinari what is the intended use-case for unlabelled selection controls?

  • I think that InputBase, FormControlLabel, Radio, Checkbox, Switch, and SwitchBase should use the same approach. Hence, revert the default prop of the Checkbox, Radio, and Switch (option 1). However, we could consider a different approach to v5.
  • For Framer, can we force the default prop outside of the code we ship to the users?
  • Regarding the question around using a radio/switch/checkbox without the FormControlLabel component. The initial motivation was to maximize composition and flexibility. However, a radio, directly accepting a child as the label could make a simpler API.

IIRC, it should be possible to add it to each component's propValues in framerConfig.js. Adding it to the component directly was intended to fix the problem "at source", while also allowing the default to be documented in the prop docs; but clearly it backfired in this instance!

If you don't mind, I'll take it over

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zabojad picture zabojad  路  3Comments

reflog picture reflog  路  3Comments

rbozan picture rbozan  路  3Comments

sys13 picture sys13  路  3Comments

newoga picture newoga  路  3Comments