search

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

Reconsider placement of accessibility statement in Preferences dialog. #838

Open pixelzoom opened 1 year ago

pixelzoom commented 1 year ago

Now that we're moving sim-specific options to the Preferences dialog in #837 ....

Here's the General tab for Geometric Optics:

screenshot_1807

In this context, the 2 paragraphs at the bottom seem out of place, and potentially confusing. These controls have nothing to do with accessibility, and the paragraphs would seem to be a description of those controls.

Discussing this with @jessegreenberg, these paragraphs were placed in the General tab when the Preferences dialog was accessibility-centric. Now that we're using it for preferences that are outside the scope of accessibility, perhaps we should reconsider where these important paragraphs live. One possibility would be on a new Accesibility tab (similar to where such preferences are presented in macOS, Windows, and many applications.)

One other note... The last paragraph is:

"To find other simulations with inclusive features, look for Access and Inclusion on the simulation filter page and filter by inclusive feature."

For someone who is using a downloaded version of the sim, this will be confusing. What is "the simulation filter page"? This paragraph should probably say something about "on the PhET website".

jessegreenberg commented 1 year ago

For someone who is using a downloaded version of the sim, this will be confusing

This makes sense to me!

In this context, the 2 paragraphs at the bottom seem out of place, and potentially confusing.

Each tab can generally include features related to access and inclusion so these paragraphs describe the dialog as a whole. Now that we have more examples of sim-specific controls in the general tab, I can see how the mix of content may not be best. I don't think "General" is the best tab name for simulation specific controls.

I wanted to propose a layout like this where we have a landing tab that describes access and inclusion, and a "Simulation" tab for simulation controls.

https://phet-dev.colorado.edu/html/jg-tests/preferences/geometric-optics_en_phet.html

@terracoda @arouinfar @emily-phet @kathy-phet can you please review the recommendations in this issue and comment on what should be done next?

Patch for that build ```patch Index: js/preferences/PreferencesPanels.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesPanels.ts b/js/preferences/PreferencesPanels.ts --- a/js/preferences/PreferencesPanels.ts (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/js/preferences/PreferencesPanels.ts (date 1661288031715) @@ -21,6 +21,7 @@ import TReadOnlyProperty from '../../../axon/js/TReadOnlyProperty.js'; import LocalizationPreferencesPanel from './LocalizationPreferencesPanel.js'; import PreferencesType from './PreferencesType.js'; +import AccessibilityPreferencesPanel from './AccessibilityPreferencesPanel.js'; type SelfOptions = EmptySelfOptions; type PreferencesPanelsOptions = SelfOptions & NodeOptions; @@ -55,6 +56,14 @@ matchVertical: false } ); + let accessibilityPreferencesPanel: Node | null = null; + if ( supportedTabs.includes( PreferencesType.ACCESSIBILITY ) ) { + accessibilityPreferencesPanel = new AccessibilityPreferencesPanel(); + const accessibilityBox = panelAlignGroup.createBox( accessibilityPreferencesPanel ); + this.addChild( accessibilityBox ); + this.content.push( new PreferencesPanelContainer( accessibilityPreferencesPanel, PreferencesType.ACCESSIBILITY ) ); + } + let generalPreferencesPanel: Node | null = null; if ( supportedTabs.includes( PreferencesType.GENERAL ) ) { generalPreferencesPanel = new GeneralPreferencesPanel( preferencesModel.generalModel, { @@ -107,6 +116,7 @@ // display the selected panel selectedTabProperty.link( tab => { + accessibilityPreferencesPanel && ( accessibilityPreferencesPanel.visible = tab === PreferencesType.ACCESSIBILITY ); generalPreferencesPanel && ( generalPreferencesPanel.visible = tab === PreferencesType.GENERAL ); visualPreferencesPanel && ( visualPreferencesPanel.visible = tab === PreferencesType.VISUAL ); audioPreferencesPanel && ( audioPreferencesPanel.visible = tab === PreferencesType.AUDIO ); @@ -117,6 +127,7 @@ this.disposePreferencesPanel = () => { panelAlignGroup.dispose(); + accessibilityPreferencesPanel && accessibilityPreferencesPanel.dispose(); generalPreferencesPanel && generalPreferencesPanel.dispose(); visualPreferencesPanel && visualPreferencesPanel.dispose(); audioPreferencesPanel && audioPreferencesPanel.dispose(); Index: js/preferences/PreferencesType.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesType.ts b/js/preferences/PreferencesType.ts --- a/js/preferences/PreferencesType.ts (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/js/preferences/PreferencesType.ts (date 1661287105228) @@ -16,6 +16,7 @@ public static AUDIO = new PreferencesType(); public static INPUT = new PreferencesType(); public static LOCALIZATION = new PreferencesType(); + public static ACCESSIBILITY = new PreferencesType(); public static enumeration = new Enumeration( PreferencesType ); } Index: js/preferences/GeneralPreferencesPanel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/GeneralPreferencesPanel.ts b/js/preferences/GeneralPreferencesPanel.ts --- a/js/preferences/GeneralPreferencesPanel.ts (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/js/preferences/GeneralPreferencesPanel.ts (date 1661288635873) @@ -7,13 +7,9 @@ * @author Jesse Greenberg (PhET Interactive Simulations) */ -import merge from '../../../phet-core/js/merge.js'; -import { Node, VBox, VBoxOptions, VoicingRichText } from '../../../scenery/js/imports.js'; -import HSeparator from '../../../sun/js/HSeparator.js'; +import { Node, VBox, VBoxOptions } from '../../../scenery/js/imports.js'; import Tandem from '../../../tandem/js/Tandem.js'; import joist from '../joist.js'; -import joistStrings from '../joistStrings.js'; -import PreferencesDialog from './PreferencesDialog.js'; import { GeneralModel } from './PreferencesModel.js'; import optionize, { EmptySelfOptions } from '../../../phet-core/js/optionize.js'; import PickRequired from '../../../phet-core/js/types/PickRequired.js'; @@ -52,19 +48,6 @@ // All panel content collected in this final array const panelContent = []; - const introParagraphs = new VBox( { spacing: 10, align: 'left' } ); - const introTextOptions = merge( {}, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS, { - - // using lineWrap instead of default maxWidth for content - maxWidth: null, - lineWrap: 600, - tagName: 'p' - } ); - introParagraphs.children = [ - new VoicingRichText( joistStrings.preferences.tabs.general.accessibilityIntroProperty, introTextOptions ), - new VoicingRichText( joistStrings.preferences.tabs.general.moreAccessibilityProperty, introTextOptions ) - ]; - // Just the provided panel content with its own spacing const providedChildren: Node[] = []; @@ -82,12 +65,7 @@ if ( providedChildren.length > 0 ) { const providedContent = new VBox( { spacing: 30, align: 'left', children: providedChildren } ); panelContent.push( providedContent ); - - // If there was provided content for this panel, separate from intro statements - panelContent.push( new HSeparator( introParagraphs.width ) ); } - - panelContent.push( introParagraphs ); this.children = panelContent; Index: js/preferences/AccessibilityPreferencesPanel.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/AccessibilityPreferencesPanel.ts b/js/preferences/AccessibilityPreferencesPanel.ts new file mode 100644 --- /dev/null (date 1661287840373) +++ b/js/preferences/AccessibilityPreferencesPanel.ts (date 1661287840373) @@ -0,0 +1,33 @@ +// Copyright 2022, University of Colorado Boulder + +/** + * @author Jesse Greenberg (PhET Interactive Simulations) + */ + +import { combineOptions } from '../../../phet-core/js/optionize.js'; +import { VBox, VoicingRichText, VoicingRichTextOptions } from '../../../scenery/js/imports.js'; +import joist from '../joist.js'; +import joistStrings from '../joistStrings.js'; +import PreferencesDialog from './PreferencesDialog.js'; + +class AccessibilityPreferencesPanel extends VBox { + public constructor() { + + super( { spacing: 10, align: 'left' } ); + const introTextOptions = combineOptions( {}, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS, { + + // using lineWrap instead of default maxWidth for content + maxWidth: null, + lineWrap: 600, + tagName: 'p' + } ); + + this.children = [ + new VoicingRichText( joistStrings.preferences.tabs.general.accessibilityIntroProperty, introTextOptions ), + new VoicingRichText( joistStrings.preferences.tabs.general.moreAccessibilityProperty, introTextOptions ) + ]; + } +} + +joist.register( 'AccessibilityPreferencesPanel', AccessibilityPreferencesPanel ); +export default AccessibilityPreferencesPanel; Index: joist-strings_en.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist-strings_en.json b/joist-strings_en.json --- a/joist-strings_en.json (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/joist-strings_en.json (date 1661287138931) @@ -141,7 +141,7 @@ "value": "Preferences" }, "preferences.tabs.general.title": { - "value": "General" + "value": "Simulation" }, "preferences.tabs.general.accessibilityIntro": { "value": "We are adding features to our simulations to make them more inclusive. Some of these features support accessibility for learners with diverse needs and within diverse environments. Explore this menu to review or change the default presentation settings." Index: js/preferences/PreferencesTabs.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesTabs.ts b/js/preferences/PreferencesTabs.ts --- a/js/preferences/PreferencesTabs.ts (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/js/preferences/PreferencesTabs.ts (date 1661287968642) @@ -51,6 +51,9 @@ const isTabSupported = ( preferencesType: PreferencesType ) => _.includes( supportedTabs, preferencesType ); + if ( isTabSupported( PreferencesType.ACCESSIBILITY ) ) { + this.content.push( new Tab( new TinyProperty( 'Accessibility' ), selectedPanelProperty, PreferencesType.ACCESSIBILITY ) ); + } if ( isTabSupported( PreferencesType.GENERAL ) ) { this.content.push( new Tab( joistStrings.preferences.tabs.general.titleProperty, selectedPanelProperty, PreferencesType.GENERAL ) ); } Index: js/preferences/PreferencesDialog.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/preferences/PreferencesDialog.ts b/js/preferences/PreferencesDialog.ts --- a/js/preferences/PreferencesDialog.ts (revision 2f941b4a549cfe4f1e53c1d749edceff9d4ed492) +++ b/js/preferences/PreferencesDialog.ts (date 1661287462946) @@ -85,7 +85,8 @@ // determine which tabs will be supported in this Dialog, true if any entry in a configuration has content const supportedTabs = []; - supportedTabs.push( PreferencesType.GENERAL ); // There is always a "General" tab + supportedTabs.push( PreferencesType.ACCESSIBILITY ); // There is always an access and inclusion blurb + preferencesModel.supportsGeneralPreferences() && supportedTabs.push( PreferencesType.GENERAL ); preferencesModel.supportsVisualPreferences() && supportedTabs.push( PreferencesType.VISUAL ); preferencesModel.supportsAudioPreferences() && supportedTabs.push( PreferencesType.AUDIO ); preferencesModel.supportsInputPreferences() && supportedTabs.push( PreferencesType.INPUT ); @@ -93,7 +94,7 @@ assert && assert( supportedTabs.length > 0, 'Trying to create a PreferencesDialog with no tabs, check PreferencesModel' ); // the selected PreferencesType, indicating which tab is visible in the Dialog - const selectedTabProperty = new EnumerationProperty( PreferencesType.GENERAL, { + const selectedTabProperty = new EnumerationProperty( PreferencesType.ACCESSIBILITY, { validValues: supportedTabs, tandem: options.tandem.createTandem( 'selectedTabProperty' ), phetioState: false, ```
terracoda commented 1 year ago

I agree that there needs to be some changes to the language. There is room for improvement.

Before we start adjusting the words and the placement of the words, let's agree to not put some features or options in an "accessibility" category.

Some features may be required to make a sim accessible to a particular learner, but the same features are almost never exclusively for accessibility.

While PhET sims have always been flexible in many ways, we are creating sims that are more flexible than ever! With options in the Preferences Menu our sims can bend and stretch to meet the needs of more learners and more learning contexts.

I would like a Preferences Menu that is clear and simple and does not have a proliferation of tabs. I am confident we can get there.

terracoda commented 1 year ago

@jessegreenberg, I think you are onto something, but there is room for improvement.

kathy-phet commented 1 year ago

I wanted to propose a layout like this where we have a landing tab that describes access and inclusion, and a "Simulation" tab for simulation controls.

@jessegreenberg - I was also thinking this could work really well, but would propose the tab be called "Overview", not Accessibility. And the next tab be called Simulation or Sim-Specific.

e.g. starts with something like ... "Here you can set a range of preferences for your sim. " ...

pixelzoom commented 1 year ago

In https://github.com/phetsims/joist/issues/838#issue-1348168341, I showed the sim-specific preferences for Geometric Optics. In discussion with @kathy-phet, she proposed that I might think of those preferences as being loosely related to inclusion, because they help tailor the sim for a specific age group. I think that's a stretch, but I can imagine that for those specific controls. But I don't think that's true of sim-specific controls in general.

So here's another example, the sim-specific preferences from Molecule Polarity (run with ?realMolecules). The controls here choose between different conventions (standards) for representing dipole charge and isosurface. In this case, I really can't find a way to relate the paragraphs to these controls. And I think it does a disservice to both the controls and the paragraphs (both of which are important) by combining them in this odd manner.

screenshot_1823
terracoda commented 1 year ago

Ok, here's a sample re-write that may be general enough for an opening Tab, whatever it gets called

Proposed Wording: P: The Preferences Menu provides access to all available presentation settings and features for a given simulation. Some features make the simulation more inclusive and can support accessibility for learners with diverse needs and within diverse environments. Explore this menu to review or change the default settings.

P: To find simulations with inclusive features, look for the Access and Inclusion filter on the PhET website.

ADDING We could drop "diverse environments" and assume people get that idea on their own. Also changed "default" to "available" presentation settings.

P: The Preferences Menu provides access to all available presentation settings and features for a given simulation. Some features make the simulation more inclusive and can support accessibility for learners with diverse needs. Explore this menu to review or change the available presentation settings.

P: To find simulations with inclusive features, look for Access and Inclusion filter on the PhET website.

terracoda commented 1 year ago

Since some simulations have several options, we may indeed indeed need an extra opening tab, and an aptly-named sim-specific tab.

The place that gets fuzzy for me, is when sim-specific settings cross-over with the other tabs (i.e., the "Visual Tab"). But maybe this kind of cross-over can be worked out relatively easily by the sim designers.

terracoda commented 1 year ago

Brainstorming ideas for Sim Options or Sim Settings Note that "tab" is spoken by a screen reader and included in the voiced tab name, though not visually displayed.

I am not familiar with how the sim-specific settings are used. This is a brain storm.

arouinfar commented 1 year ago

Notes from the 8/25/22 design meeting:

There are still discussions to be had regarding the blurb's wording and perhaps further discussion on sim-by-sim basis of which set of controls belongs in "Simulation". However, I think there is enough of a consensus to move forward with these changes:

Assigning to @jessegreenberg since he already did some work on this, and co-assigning @zepumph as the responsible dev for this repo.

jessegreenberg commented 1 year ago

There are a lot of classes throughout now with GeneralPreferences in the name. They should be renamed now.

jessegreenberg commented 1 year ago

OK, the "General" tab has been removed and replaced with "Overview" and "Simulation" tabs with some related code cleanup.

The blurb needs some wordsmithing. Options to consider

@arouinfar back to you for this part or to reassign to anyone else.

@zepumph can you please spot check? Most things were straight forward, but I used Simulation in class/option/type names. Was slightly worried about confusion because "Simulation" is overloaded for us. But thought it was best to match other preferences naming since that is the name of the tab.

zepumph commented 1 year ago

You are so very talented. This is amazing. I liked above issues that I think are high priority that I found while reviewing this, but I have no recommendations here. Let me know if you need anything else.

zepumph commented 1 year ago

At this time I do not think this word smithing is blocking any sim release. Please mark this with the appropriate label and tag me if that changes.

arouinfar commented 1 year ago

This issue remains open because there has been some discussion about improving the wording of the "Overview" blurb, but we did not reach a conclusion. Some options appear in https://github.com/phetsims/joist/issues/838#issuecomment-1230732910.

When work resumes on the Preferences Dialog (https://github.com/orgs/phetsims/projects/61), the team should decide how to proceed. For now, I'm unassigning myself.