material-components / material-web

Material Design Web Components
https://material-web.dev
Apache License 2.0
9.31k stars 890 forks source link

use formDisabledCallback to support <fieldset> disabled attribute #5049

Open datvm opened 1 year ago

datvm commented 1 year ago

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

-

asyncLiz commented 1 year ago

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?

datvm commented 1 year ago

Sure I can do that :)

asyncLiz commented 10 months ago

This is fixed now in 1.1 :)

mrpachara commented 8 months ago

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.

asyncLiz commented 8 months ago

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.