bug: radio button disabled prop doesn't handle undefined

[nmspackman]

Prerequisites

• [X] I have read the Contributing Guidelines.
• [X] I agree to follow the Code of Conduct.
• [X] I have searched for existing issues that already report this problem, without success.

Ionic Framework Version

v7.x

Current Behavior

An IonRadio component with undefined passed as a value to the disabled prop causes the radio to not trigger the onIonChange function when selected. The visual effect is the previous selection and the current selection (with disabled set to undefined) will both be selected.'

Note: Cherries is the radio that has disabled set to undefined.

Expected Behavior

The radio button should be selected and the radio group value should update.

Steps to Reproduce

1. Create an IonRadioGroup with 3 IonRadio components nested (values are arbitrary)
2. Set one of the IonRadio's disabled prop to undefined
3. When viewing the UI, click one of the radios without the disabled prop set, then click the radio with it set

Code Reproduction URL

https://stackblitz.com/edit/f4avhg?file=src%2Fmain.tsx

Ionic Info

N/A

Additional Information

To work around this issue, I set the disabled prop using strict equals for comparing the variable passed in. It looks something like this:

<IonRadio value="cherries" disabled={options?.isDisabled === true}>
    Cherries
</IonRadio>

[liamdebeasi]

Thanks for the report. Can you help me understand why you need to pass undefined instead of false to disabled? As per the docs, the disabled property accepts a boolean: https://ionicframework.com/docs/api/radio#properties This behavior is consistent with the other disabled properties in Ionic.

[nmspackman]

Thanks for the quick response!

I wouldn't say need, but that's how we previously were using the component without issue. Typically the value would be true or false, but it was possible for that value to not be set (undefined). We upgraded, then caught this behavior weeks later. So, it appeared to be a bug.

The only confusing thing as a TS developer is that (despite the type being boolean) being optional allows for undefined to be passed in. Passing in any other falsy value of course will be caught, but undefined is the only exception. My personal expectation is that any prop set to undefined would function as if the prop was never passed at all.

At the end of the day, it's not a big deal. We now know, but others may benefit from it.

[liamdebeasi]

The only confusing thing as a TS developer is that (despite the type being boolean) being optional

Out of curiosity, where are you seeing that this property is marked as optional? The intent of the disabled property is that it is required, but maybe there's a type mixup somewhere.

[nmspackman]

@ionic/core/dist/types/components.d.ts:

Which shows in the VSCode tooltip as:

[HudsonPWesel]

You could create a wrapper (either for each IonRadio or an IonGroup like, like I've done below) and force the type boolean like this: ` interface IonProps { value: string; disabled: boolean; } const CustomIonRadioGroup: React.FC = (props: IonProps) => { return ( <>

Current Value: {value}

    <IonRadioGroup
      value={value}
      onIonChange={(e) => {
        console.log(value);
        setValue(e.detail.value);
      }}
    >
      <IonRadio value="cherries" disabled={props.disabled}>
        Cherries
      </IonRadio>
      <br />
      <IonRadio value="grapes">Grapes</IonRadio>
      <br />
      <IonRadio value="strawberries">Strawberries</IonRadio>
      <br />
      <IonRadio value="pineapple">Pineapple</IonRadio>
      <br />
    </IonRadioGroup>
  </>
);

}; return ;`

[1pandeyji]

I can solve this issue.

[liamdebeasi]

@ionic/core/dist/types/components.d.ts:

Which shows in the VSCode tooltip as:

Thanks for the info! I need to check with the team to see if having the disabled prop be optional is intentional. The team's course of action may change based on that discussion. I'll post here when I have more to share.

[liamdebeasi]

Hi everyone,

~I spoke with the team and having disabled be optional does not appear to be intentional. However, most of our components that use disabled are built in such a way that they support the optional use case. Additionally, making this property required would mean we'd need to change the types for 28 components, meaning 28 breaking changes. We don't think that is a good tradeoff to make, so we'll focus on updating radio to resolve this issue.~

Update: The compiled types in @ionic/core show that the property is required. However, the React types show that it is optional. There's something missing here that I'm not fully understanding. I can reproduce the optional types in React, but I need to figure out why that's being set.

[mapsandapps]

Thanks for submitting this issue! This has been fixed in #28712 and will be released in an upcoming version of Ionic Framework.

[ionitron-bot[bot]]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.