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.

Sign up for GitHub

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

Jump to bottom

use formDisabledCallback to support <fieldset> disabled attribute #5049

Open
datvm opened this issue Oct 3, 2023 · 5 comments · May be fixed by #5565
Open

use formDisabledCallback to support <fieldset> disabled attribute #5049

datvm opened this issue Oct 3, 2023 · 5 comments · May be fixed by #5565
Assignees
@asyncLiz

Comments

@datvm
Copy link
Contributor

datvm commented Oct 3, 2023

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
Copy link
Collaborator

asyncLiz commented Oct 3, 2023

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 asyncLiz changed the title Button and Input components do not honor <fieldset> disabled attribute use formDisabledCallback to support <fieldset> disabled attribute Oct 3, 2023
@datvm
Copy link
Contributor Author

datvm commented Oct 3, 2023

Sure I can do that :)

@datvm datvm mentioned this issue Oct 3, 2023
Closed
@asyncLiz asyncLiz self-assigned this Oct 3, 2023
@asyncLiz
Copy link
Collaborator

This is fixed now in 1.1 :)

@mrpachara
Copy link

I not sure this is just a workaround or completely fix. The behavior is wrong due to, #5053 (comment). 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
Copy link
Collaborator

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 asyncLiz reopened this Feb 14, 2024
This was referenced Apr 3, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@asyncLiz asyncLiz

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@datvm @mrpachara @asyncLiz