Skip to content

Conversation

@rafaelss95
Copy link
Contributor

@rafaelss95 rafaelss95 commented Jul 14, 2017

Closes #1656

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 14, 2017
@rafaelss95
Copy link
Contributor Author

rafaelss95 commented Jul 15, 2017

I have added required attribute in radio-button, but I just noticed that it could cause some inconsistencies, as in this example:

<md-radio-group required="true" name="any">
  <md-radio-button value="any" required="false">any</md-radio-button>
</md-radio-group>

Note that in this case it is possible to set required="true" to md-radio-group and at the same time required="false" on one or even all the radio buttons inside it.

But in this case, the required from md-radio-group will override any required attribute in inside radio-buttons due to the condition:

get required(): boolean {
  return this._required || (this.radioGroup && this.radioGroup.required);
}

It means that if the md-radio-group is required, all md-radio-button's inside it, will be required, no matter what you've set in the required attribute in md-radio-button's.

I'm just wondering if it's the correct approach. Any thoughts @jelbourn?

Copy link
Contributor

@tinayuangao tinayuangao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks, I think your approach is correct (same as disabled behavior)

@jelbourn
Copy link
Member

@rafaelss95 could you rebase this PR?

@rafaelss95
Copy link
Contributor Author

rafaelss95 commented Jul 21, 2017

@jelbourn Done.

Note that the failed checks are unrelated to this PR. Btw, it should be fixed in #5945.

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jul 24, 2017
@andrewseguin andrewseguin merged commit f06fe11 into angular:master Jul 25, 2017
@rafaelss95 rafaelss95 deleted the radio-group branch July 29, 2017 18:16
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[md-radio] required attribute doesn't work for md-radio-group or md-radio-button

5 participants