phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

Agreement between AquaRadioButtonGroup and RectangularRadioButtonGroup #806

Closed veillette closed 1 year ago

veillette commented 1 year ago

I am currently converting a AquaRadioButtonGroup to RectangularRadioButtonGroup in Calculus-Grapher.

There is a nice parallel between the API of AquaRadioButtonGroup and RectangularRadioButtonGroup. Since going from one group of buttons to the next is probably a frequent design change,

It was almost a simple drop in, but I had to do some digging for the type.

For type checking in RectangularRadioButtonGroup, we have export type RectangularRadioButtonGroupOptions export type RectangularRadioButtonItem export type RectangularRadioButtonLabelAlign // Determines where the optional label appears, relative to the button

in AquaRadioButtonGroup, we have export type AquaRadioButtonGroupOptions export type AquaRadioButtonGroupItem

Because of its nature, AquaRadioButtonGroup doesn't need to have have a label align.

My proposal would be to rename RectangularRadioButtonItem to RectangularRadioButtonGroupItem and RectangularRadioButtonLabelAlign to RectangularRadioButtonGroupLabelAlign to bring them in agreement, although I could see that renaming AquaRadioButtonGroupItem to AquaRadioButtonItem would work as well.

Assigning to @pixelzoom .

pixelzoom commented 1 year ago

Agreed, the naming is inconsistent. Since we also have VerticalCheckboxGroupItem for VerticalCheckboxGroup, let's rename RectangularRadioButtonItem to RectangularRadioButtonGroupItem. I'll do the rename.

@samreid FYI, since you worked on RectangularRadioButtonGroup recently.

pixelzoom commented 1 year ago

Rename complete in the above commits. @veillette if this is what you had in mind, and I didn't miss anything, feel free to close.

veillette commented 1 year ago

Yes, this bring consistency between the types of the two classes. Closing.