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.

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

fix(radio): deselect button if value doesn't match up with the group anymore #14734

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jan 5, 2019

Handles the case where a radio button is selected and then its value is changed to something that doesn't match up with the group anymore.

@crisbeto crisbeto added the target: patch This PR is targeted for the next patch release label Jan 5, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 5, 2019
devversion
devversion previously approved these changes Jan 5, 2019
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM

@devversion devversion added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jan 5, 2019
jelbourn
jelbourn previously approved these changes Jan 8, 2019
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

crisbeto added a commit to crisbeto/material2 that referenced this pull request Jan 11, 2019
Along the same lines as angular#14734. Deselects a selected list option, if its value is changed to something that doesn't match up with the group.
vivian-hu-zz pushed a commit that referenced this pull request Jan 17, 2019
Along the same lines as #14734. Deselects a selected list option, if its value is changed to something that doesn't match up with the group.
s2-abdo pushed a commit to s2-abdo/material2 that referenced this pull request Jan 18, 2019
Along the same lines as angular#14734. Deselects a selected list option, if its value is changed to something that doesn't match up with the group.
vivian-hu-zz pushed a commit that referenced this pull request Jan 18, 2019
Along the same lines as #14734. Deselects a selected list option, if its value is changed to something that doesn't match up with the group.
@mmalerba mmalerba added aaa and removed aaa labels Apr 25, 2019
…anymore

Handles the case where a radio button is selected and then its value is changed to something that doesn't match up with the group anymore.
@crisbeto crisbeto force-pushed the radio-selected-button-value-change branch from 645e63d to 5a29a88 Compare June 20, 2019 20:57
@crisbeto crisbeto added the P4 A relatively minor issue that is not relevant to core functions label Jun 20, 2019
@mmalerba mmalerba removed the lgtm label Jul 31, 2020
@andrewseguin andrewseguin removed the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 29, 2021
@andrewseguin
Copy link
Contributor

This change depends on the order Angular sets the button's checked and value properties. When checked is set before value, both the group and button will have a null value and never be selected. However, when value is set first, the evaluation shows that group's value is null and the button has a value so it sets it.

You can test it by comparing the group value between these two cases:

<!-- No button will be selected since the checked setter was called first and it found that the group and buttons all have null values -->
<mat-radio-group>
  <mat-radio-button checked value="1"> Option 1 </mat-radio-button>
  <mat-radio-button value="2"> Option 2 </mat-radio-button>
  <mat-radio-button value="3"> Option 3 </mat-radio-button>
</mat-radio-group>

<!-- The first button will be selected because checked will update the group based on the first button's value -->
<mat-radio-group>
  <mat-radio-button value="1" checked> Option 1 </mat-radio-button>
  <mat-radio-button value="2"> Option 2 </mat-radio-button>
  <mat-radio-button value="3"> Option 3 </mat-radio-button>
</mat-radio-group>

@andrewseguin andrewseguin removed the action: merge The PR is ready for merge by the caretaker label Feb 22, 2022
@andrewseguin andrewseguin removed the P4 A relatively minor issue that is not relevant to core functions label Mar 28, 2022
@andrewseguin andrewseguin dismissed stale reviews from jelbourn and devversion via 5a29a88 May 2, 2022 20:33
@andrewseguin andrewseguin assigned amysorto and unassigned devversion Jun 5, 2023
@josephperrott josephperrott requested a review from a team as a code owner December 18, 2024 17:40
@josephperrott josephperrott requested review from amysorto and mmalerba and removed request for a team December 18, 2024 17:40
@mmalerba mmalerba removed their request for review February 20, 2025 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants