feat: add disabled property to radio-group

[wslaghekke]

Bug Report

Ionic version:

[x] 4.x

Current behavior:

Add the disabled attribute to a ion-radio-group, and some child ion-radio components

Expected behavior:

The child ion-radio components should be disabled

Steps to reproduce:

Related code:

insert short code snippets here

Other information: I suspect this also causes the radios not to be disabled in the issue https://github.com/ionic-team/ionic/issues/18383 but i've only tested the ionic/core component for now

Ionic info:

insert the output from ionic info here

[infacto]

I'm wondering why the existing pull request isn't being merged or rather why the developers aren't writing a single word about it. Anything is better than no response. Consider to merge or close it with any reason you want. Anything else is a little rude in my opinion. But anyway... In case anyone is looking for workarounds: You just need to declare some global styles.

global.scss

ion-radio-group[disabled]:not([disabled='false']) {
  opacity: 0.3;
  pointer-events: none;
}

some.component.ts

<ion-radio-group [attr.disabled]="true"></ion-radio-group>

Since attribute is not a known property on ion-radio-group, you cannot bind it directly in Angular. Bypass with attr works. But you can do anything you want. A custom class or a data attribute like data-disabled, etc. This may not work with (reactive) forms. You may need to do extra stuff like binding the form control disabled property to the elements attr.disabled. Just not yet tested. This is just a first shot of a workaround and fits my current requirements. But it would be great if the ion-radio-groups acts like ion-item disabled attr. Not sure if the pull-reqeust works in all cases with forms etc. Maybe I missed something. But the Ionic devs can certainly tell more about that. It would definitely be interesting to have an opinion on this.

[sean-perkins]

@infacto the existing pull request cannot be merged as-is, as it is out of date with main and has a number of merge conflicts and no test coverage.

If you are curious to the status on an older issue or feature, it is appropriate to ask on the thread, especially if no one has recently responded to the issue. It is always best to be kind in replies. Ionic Framework is free and open source. You can always fork the repository if you need to make modifications required for your projects.

The team has actively reviewed pending community pull requests from 50+ to the current 28 that exist (while a number of those are recent PRs from the Core team) over the past few months. This feature and community pull request are in our queue to review. The team will share an update when that review takes place.

[infacto]

np, I think I was a bit annoyed because it's an input element, which should have a disabled attribute. I'm fine with my workaround. I'm not sure if the current pull request meets Ionic Team's requirements. What is the proper way to implement disabled-attribute for a group? Do we already have comparable implementations? Radio group is maybe a special case. Anyway...

[sean-perkins]

The existence of a "radio group" component in other design systems is rare. It tends to be a container element with the role="radiogroup" or using fieldset.

e.g.:

<fieldset>
    <legend>Select a maintenance drone:</legend>

    <div>
      <input type="radio" id="huey" name="drone" value="huey" checked>
      <label for="huey">Huey</label>
    </div>

    <div>
      <input type="radio" id="dewey" name="drone" value="dewey">
      <label for="dewey">Dewey</label>
    </div>

    <div>
      <input type="radio" id="louie" name="drone" value="louie">
      <label for="louie">Louie</label>
    </div>
</fieldset>

The initial concerns I have with adding disabled to ion-radio-group is what happens when disabled is set on the ion-radio-group and then set on the individual ion-radio elements?

e.g.:

<ion-radio-group disabled="true">
  <ion-radio disabled="false"></ion-radio>
  <ion-radio disabled="true"></ion-radio>
</ion-radio-group>

To screen readers, the group would be announced as disabled, but then controls inside of it could potentially be enabled.

Comparing with Android (native), they require you iterate over each radio child and manually disable it, when needing to disable a group. I think a similar situation may apply here.

[wslaghekke]

The reason why my PR implements it on the ion-radio-group instead of on the ion-radio element is because in Angular when you use Reactive forms and you put the formControl on the group it doesnt disable the radio items when you disable the control from the angular FormControl. Binding the control to the radio items is not an option because those emit True/False based on if they are checked, not the specific option value that is selected.

[sean-perkins]

The team is still interested in this feature and we plan on supporting a similar API solution for both ion-accordion-group and ion-radio-group.

I posted a brief message around why we are not accepting the proposed PR at the moment: https://github.com/ionic-team/ionic-framework/pull/19521#issuecomment-1271770529

I will follow-up after the team is able to move that discovery work in a sprint or we may just open a pull request depending on the scope.

Appreciate the discussion here + added context!

[muuvmuuv]

Hey, I see there is a PR merged, does that include disabled? I cannot see an attribute set, and I am unable to properly set it manually. Any guidance?