material-web icon indicating copy to clipboard operation
material-web copied to clipboard

use formDisabledCallback to support <fieldset> disabled attribute

Open datvm opened this issue 1 year ago • 5 comments

What is affected?

Component

Description

See: https://jsfiddle.net/datvm/zdpeqc5j/1/

image

For standard components, when an acestor <fieldset> has [disabled=true], they are disabled as well. I think this behavior is not discussed yet so it's probably not a bug? Would you consider adding this feature?

Note: the screenshot above misses <md-select>.

Reproduction

https://jsfiddle.net/datvm/zdpeqc5j/1/

<fieldset disabled>
    <p>
        <md-filled-button>A Material 3 Button</md-filled-button>
        <button>Standard button</button>
    </p>
    
    <p>
        <md-outlined-text-field></md-outlined-text-field>
        <input value="Standard input" />
    </p>
    
    <p>
        <md-checkbox></md-checkbox>
        <md-switch></md-switch>
        <input type="checkbox" value="Standard input" />
    </p>
    
    <p>
        <md-radio></md-radio>
        <input type="radio" value="Standard input" />
    </p>
    
    <p>
        <md-slider></md-slider>
        <input type="range" />
    </p>
    
</fieldset>

Workaround

You have to manually set disabled to each component in the fieldset.

Is this a regression?

No or unsure. This never worked, or I haven't tried before.

Affected versions

1.0.0

Browser/OS/Node environment

datvm avatar Oct 03 '23 00:10 datvm

We need to add formDisabledCallback() to our elements, since <fieldset> can disable them

formDisabledCallback(disabled: boolean) {
  this.disabled = disabled;
}

This would be a good community PR, want to take a stab at it?

asyncLiz avatar Oct 03 '23 16:10 asyncLiz

Sure I can do that :)

datvm avatar Oct 03 '23 18:10 datvm

This is fixed now in 1.1 :)

asyncLiz avatar Dec 15 '23 04:12 asyncLiz

I not sure this is just a workaround or completely fix. The behavior is wrong due to, https://github.com/material-components/material-web/pull/5053#discussion_r1344784951. The component should not set disabled to itself. Moreover, when <fieldset> is changed back to enabled state, the <text-field> is not changed back to enabled.

mrpachara avatar Feb 11 '24 20:02 mrpachara

Interesting! After investigating, I believe the problem is that setting our own disabled attributes changes the underlying disabled state of the FACE.

Instead, we need to handle an internal disabled state and a client-facing disabled attribute state.

asyncLiz avatar Feb 14 '24 19:02 asyncLiz