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

ComboBox: add align option #419

Closed
pixelzoom opened this issue Nov 19, 2018 · 5 comments
Closed

ComboBox: add align option #419

pixelzoom opened this issue Nov 19, 2018 · 5 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 19, 2018

ComboBox items are left aligned, with no option to do otherwise.

When the items are numbers, it would better to right align them. For example, here's how the Collision Counter currently looks in Gas Properties:

screenshot_892

Add align option, with values 'left' (default), 'right', 'center'.

align should apply to both the popup list and the current value displayed on the button.

@pixelzoom pixelzoom self-assigned this Nov 19, 2018
@pixelzoom pixelzoom changed the title add ComboBox option to align items in popup list ComboBox: add align option Nov 19, 2018
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Nov 27, 2018

This will also fix the alignment of temperature choices in Gas Properties. This combo box is currently composed of NumberDisplay items. The NumberDisplay items have align: 'right', but they have different widths, so the items are not right aligned in the popup list.

screenshot_893

pixelzoom added a commit to phetsims/gas-properties that referenced this issue Nov 27, 2018
pixelzoom added a commit that referenced this issue Nov 27, 2018
@pixelzoom
Copy link
Contributor Author

The align option was added in the above commits, and align: 'right' is used in Gas Properties. What I thought was going to be trivial was downright scary — ComboBox has gotten complicated.

I tested with ?a11y and the focus highlight still looks correct:

screenshot_895

Randomly assigned to @jessegreenberg to review, which works out nicely since my primary concern is breaking a11y.

@jessegreenberg
Copy link
Contributor

Changes look good @pixelzoom. I tested a few other comboboxes, the option worked as expected, focus highlights were correct and screen reader output was not changed.

image

Closing.

@pixelzoom
Copy link
Contributor Author

Reopening. This feature was broken during #430.

@pixelzoom pixelzoom reopened this Jan 8, 2019
@pixelzoom pixelzoom assigned pixelzoom and unassigned jessegreenberg Jan 8, 2019
pixelzoom added a commit to phetsims/balancing-chemical-equations that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/masses-and-springs-basics that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/least-squares-regression that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/fluid-pressure-and-flow that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/masses-and-springs that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/molecule-polarity that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/projectile-motion that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/states-of-matter that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/molecule-shapes that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/gas-properties that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/bending-light that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/beers-law-lab that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/pendulum-lab that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/unit-rates that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/wave-interference that referenced this issue Jan 15, 2019
pixelzoom added a commit to phetsims/scenery-phet that referenced this issue Jan 15, 2019
pixelzoom added a commit that referenced this issue Jan 15, 2019
@pixelzoom
Copy link
Contributor Author

pixelzoom commented Jan 15, 2019

Alignment was fixed in b47526a. Will continue further work in #430. Closing.

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

2 participants