radio button fixture and tests #182
Conversation
Codecov Report
@@ Coverage Diff @@
## master #182 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 29 29
Lines 884 884
Branches 169 169
=========================================
Hits 884 884 Continue to review full report at Codecov.
|
| * Get the value of the radio button at the given index. | ||
| * @param index The index of the radio button. | ||
| */ | ||
| public getRadioButtonValue(index: number): any { |
There was a problem hiding this comment.
I'd rename this to getValueByIndex to make the method name more descriptive.
Also, this method feels a bit odd to me. Is there a current customer requirement to get the value of a given radio button in a group? I feel like setting the radio group to a specific value, and retrieving the value of the radio group would suffice?
| /** | ||
| * A flag indicating if every radio button in the radio group is disabled. | ||
| */ | ||
| public get allRadioButtonsDisabled(): boolean { |
There was a problem hiding this comment.
For MVP, we should assume the consumer is using the radio group "properly", by wrapping all radio buttons within a sky-radio-group. That being the case, we could simplify all of the "disabled" properties to: get disabled and set disabled (since all radio buttons would be disabled if the parent radio group is disabled).
(We could even throw an error if a sky-radio-group isn't found.)
| * The text of the radio button at the given index. | ||
| * @param index The index of the radio button. | ||
| */ | ||
| public getRadioButtonLabelText(index: number): string { |
There was a problem hiding this comment.
Based on the feedback above, we could rename this to getLabelTextByIndex.
There was a problem hiding this comment.
Also, is there a current MVP request for this functionality?
| * Select the radio button with the given index. | ||
| * @param index The index of the radio button. | ||
| */ | ||
| public selectRadioButtonInputElByIndex(index: number): void { |
There was a problem hiding this comment.
Couldn't this method be removed and a simple set value be used instead?
No description provided.