-
Notifications
You must be signed in to change notification settings - Fork 65
Reactive radio #1714
Reactive radio #1714
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small non-blocking question. @Blackbaud-SteveBrush this looks good to me but is big enough to probably need your eyes at this point as I might not know of any oddities here or there. Thanks!
src/modules/radio/radio.component.ts
Outdated
|
||
/** | ||
* Provider Expression that allows sky-checkbox to register as a ControlValueAccessor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we are getting rid of the comments here and on nextId
above? Seems like there wouldn't be hurt in leaving them there for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added back in 😄
Resolves #1401 |
Created #1942 to document changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Original contribution: #1692
Resolves: #1401