phetsims / joist

Joist is the main framework for PhET Interactive Simulations. Joist creates and displays the simulation content, home screen, navigation bar, About dialog, enables switching between tabs, and other framework-related features.
MIT License
8 stars 6 forks source link

Preferences dialog lacks consistent UI style. #842

Open pixelzoom opened 1 year ago

pixelzoom commented 1 year ago

In #837, Options were folded into the Preferences dialog. In general, the content for the Options dialog was not changed, it was simply passed to the Preferences dialog. The end result is that there are now different UI styles in the Preferences dialog, a bit of a hodge podge.

For example, see Gas Properties. Projector Mode (handled by Preferences dialog) is set using an A/B switch, with a nice description of the preference, and looks like this:

screenshot_1818

Options specific to Gas Properties used a checkbox instead of an A/B switch, and has no description. It looks like this:

screenshot_1819

A style guide and some common-code UI components are needed.

PhET can certainly live with this for now, because it's usable. If we didn't have to worry about PhET-iO, this issue would be a "polish". But changing to a consistent UI style in the future will break the PhET-iO API, require migration rules, etc.

terracoda commented 1 year ago

Great points, @pixelzoom. I think it might be better for me to comment in the other issue where we are discussing wording of the "inclusive features".

Perhaps a temporary fix is using a "Sim-specifc Options" subheading here for these types of checkboxes that are being migrated directly from the PhET Menu to the Preferences Menu.

terracoda commented 1 year ago

This issue is also related to #838

amanda-phet commented 1 year ago

Related to this issue, we discovered that the right-aligned toggle switches can look too close to the next column and leads to a very confusing interface.

image (1)

This doesn't seem to be as problematic in the sims with voicing because there is a string lower down that makes the two columns appear farther apart, but we should still probably handle this column width thing in common code.

Overall, this is contributing to us thinking that a left-aligned toggle switch seems like it would work a lot better in the preferences dialog, and then the toggles will be aligned in every preferences dialog and their placement won't be all over, and the toggle will always be closest to the respective title/thing it is controlling.

amanda-phet commented 1 year ago

Discussed a unified design for the preferences dialog 11/17/22

Current toggle paradigm works well in a single-column format with right-aligned toggles.

Another inconsistency:

If a tab has multiple columns, should we make the column widths consistent? Rather than base it on the contents of the column?

Different tabs seem to have different types of control

Immediate concerns:

Next steps:

terracoda commented 1 year ago

Regarding the first action item, I would like for us to use a consistent language for things that require words, i.e. the text that can be recognized by screen readers or delivered via the Voicing feature.

"description" might be too general as everything from the Sim's name in the H1 to a context response that fires as changes happen is a description. It might be useful to call an interactive object's description "help text" if that is the function of the description.

pixelzoom commented 1 year ago

@marlitas After the above changes, I'm seeing something very odd in PreferencesToggleSwitch, and therefore in calculus-grapher ValueControl. PreferencesToggleSwitch is now unrelated to ToggleSwitch -- are you planning to rename PreferencesToggleSwitch? I understand that you are generalizing, but that should start with the name.

marlitas commented 1 year ago

@pixelzoom Yes the rename is happening today. Ran out of time last night. It has now been refactored to take any control in the upper right corner (or wherever we decide in terms of design in the future). This has been done to make preferences components more consistent among sims and easier for developers to use. What was the odd behavior you were seeing? I want to make sure I refactored all cases out properly.

pixelzoom commented 1 year ago

Two things:

-   labelNode?: null | Node;
+   labelNode?: Node;

- const options = optionize<PreferencesToggleSwitchOptions, SelfOptions, NodeOptions>()( {
+ const options = optionize<PreferencesToggleSwitchOptions, StrictOmit<SelfOptions, 'labelNode'>, NodeOptions>()( { 

-      labelNode: null,
marlitas commented 1 year ago

It's odd that all subcomponents of PreferencesToggleSwitch are optional. Shouldn't controlNode at least be required?

The hope is that his component will work for all the different use cases a preferences dialog may need. For example, a component may need a title and a control, but there are also moments when it may only need a title and description, or just a title, or a description and a control. Making all of these optional will provide flexibility while maintaining a consistent UI across the different usages. The name PreferencesControl is 100% up for discussion as it does not fully encompass these usages, but it felt like a step in the right direction.

marlitas commented 1 year ago

I updated NumberPlayPreferencesNode to use the new PreferencesControl API. @chrisklus the commits are above if you want to take a look and reach out to me for any questions or concerns.

chrisklus commented 1 year ago

@marlitas and I reviewed the changes in Number Play together. Thanks! Back to @marlitas.

marlitas commented 1 year ago

Reopening, as the work for preferences isn't done here, but it is done for Number Play. :-)

chrisklus commented 1 year ago

Ope, my bad! Forgot this wasn't in a Number Play repo.