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

should RadioButtonGroup support grid layout? #513

Open
veillette opened this issue May 29, 2019 · 7 comments
Open

should RadioButtonGroup support grid layout? #513

veillette opened this issue May 29, 2019 · 7 comments
Assignees

Comments

@veillette
Copy link
Contributor

veillette commented May 29, 2019

In vector-addition, the design document specifies a 2x2 grid of radio buttons (see phetsims/vector-addition#6). Currently the layout of the RadioGroupButton supports only two orientations, namely vertical and horizontal.

Is it worth generalizing the layout of RadioButtonGroup?

In a Slack discussion, CM suggested:
You could also rework/rewrite RadioButtonGroup to have options:

rows: {number},
columns: {number},
majorOrder: {Enumeration} // RowMajor, ColumnMajor

A second approach would be to go around the problem by creating two vertical RadioButtonGroups that are arranged in HBox. However, this might be a source of problems for PhET-iO and a11y.

A third approach would be to implement a custom layout by leveraging RadioGroupButtonMember and RectangularButtonView. There is no other precedent of a grid like layout for radio buttons but there is some precedent for a grid like layout of a combox box. Although it is no supported by SUN (see #363), there exists a custom grid like layout of the combo box in Area-Model ( GenericLayoutSelectionNode)

FYI, I counted 33 instances of 'new RadioButtonGroup' in the code base.

@veillette
Copy link
Contributor Author

The kludge approach (2nd one) does not work since RadioButtonGroup requires that one of the property value is present in the content array, such that at least one of the button is selected

@samreid
Copy link
Member

samreid commented May 30, 2019

Would it be better to create a more general layout suite that applies to any set of Node (instead of just hardcoded to work in RadioButtonGroup or isolated modules?)

@jbphet
Copy link
Contributor

jbphet commented Jun 6, 2019

Discussed in the 6/6/2019 dev meeting, and @jonathanolson is going to work with @veillette and et al and will work to improve it.

@pixelzoom will add some other thoughts to this issue which will potentially be folded in to this effort.

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 7, 2019

My 2 cents...

  • Grid layout (perhaps in LayoutBox?) should be independent of any "group" that uses it.
  • VBox and HBox should be conveniences that are special cases of "grid layout".
  • RadioButtonGroup should responsible for layout (using general "grid layout") and convenient specification of radio buttons, but nothing more (similar to VerticalAquaRadioButtonGroup).
  • RadioButtonGroupMember should be reworked (and renamed) to be similar to AquaRadioButton, a UI component that is independent of any parent "group". Consider RectangularRadioButton or something similar as the new name.
  • RadioButtonGroup should replace VerticalRadioButtonGroup, with an option to specify what kind of
    radio buttons you want (e.g. 'aqua', 'rectangular')

(EDIT: "should" in the above is a recommendation, not an order)

@ariel-phet
Copy link

@jessegreenberg will work on this issue after ESP is published. It may makes sense to bring back to developer meeting before doing the work.

@veillette
Copy link
Contributor Author

GridBox in Scenery supports a grid layout that can be applied to a radio button group. See for instance an application of it in CurveManipulationModeRadioButtonGroup in Calculus-Grapher.

Unassigning.

@veillette veillette removed their assignment Mar 16, 2023
@jonathanolson
Copy link
Contributor

I'm unclear why RadioButtonGroup would handle grid layout? I just refactored AquaRadioButtonGroup, and it definitely uses FlowBox. We could rip out some parts of it so that it could just provide a list of Nodes, and the user could do their own layout. Then we could have a convenient way of including them in any layout, OR just using the the default FlowBox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants