Skip to content

Conversation

jelbourn
Copy link
Member

R: @kara @robertmesserle

  • Completely rewrite radio-button unit tests to be much simpler (and also actually run several tests that weren't running before).
  • Fix ngControl always being set to dirty (vs. pristine initially)
  • Fix ngControl never being able to become touched
  • Fix setting checked directly on a radio button not updating the group's value.
  • Fix firing a change event on initial setting of value at application load.
  • Mark methods as @internal

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 13, 2016

groupDebugElement = fixture.debugElement.query(By.directive(MdRadioGroup));
groupNativeElement = groupDebugElement.nativeElement;
groupInstance = groupDebugElement.injector.get(MdRadioGroup);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the same as groupDebugElement.componentInstance?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope; radio-group is a @Directive rather than a @Component, so it doesn't have a componentInstance.

@jelbourn
Copy link
Member Author

@robertmesserle addressed comments

/** Updates the `selected` radio button from the internal _value state. */
private _updateSelectedRadioFromValue(): void {
// Update selected if different from current value.
// If the value already matches the selected radio, no dothing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean "do nothing"?

@jelbourn jelbourn force-pushed the radio-test-refactor branch 2 times, most recently from 703129a to 98c0ac2 Compare May 16, 2016 23:24
@jelbourn jelbourn mentioned this pull request May 17, 2016
4 tasks
* Initialize properties once content children are available.
* This allows us to propagate relevant attributes to associated buttons.
*/
ngAfterContentInit() {
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, shouldn't this method be @internal?

@jelbourn jelbourn force-pushed the radio-test-refactor branch from 98c0ac2 to 63a9fd9 Compare May 18, 2016 02:50
@jelbourn jelbourn force-pushed the radio-test-refactor branch from 63a9fd9 to 672e205 Compare May 18, 2016 15:19
@jelbourn
Copy link
Member Author

Verbal lgtm from @robertmesserle

@jelbourn jelbourn merged commit a25a8da into angular:master May 18, 2016
@jelbourn jelbourn deleted the radio-test-refactor branch May 19, 2016 16:57
andrewseguin pushed a commit to andrewseguin/components that referenced this pull request Oct 15, 2018
@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 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

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.

5 participants