phetsims / number-suite-common

"Number Suite Common" contains code for use by sims that are part of the Number Suite
GNU General Public License v3.0
0 stars 0 forks source link

Add PageControl to preferences language carousel #71

Closed chrisklus closed 1 year ago

chrisklus commented 1 year ago

From discussion with @amanda-phet and @zepumph we realized that the locale carousel should have a page control since there are so many pages to scroll through!

Tagging https://github.com/phetsims/qa/issues/929

zepumph commented 1 year ago

We can start with this:

```diff Subject: [PATCH] factor out icon height for maintainability if asset, https://github.com/phetsims/joist/issues/919 --- Index: js/common/view/LanguageAndVoiceControl.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/LanguageAndVoiceControl.ts b/js/common/view/LanguageAndVoiceControl.ts --- a/js/common/view/LanguageAndVoiceControl.ts (revision a5f5ba8c559bc5954e0f2cec7b029ccc08636309) +++ b/js/common/view/LanguageAndVoiceControl.ts (date 1681321449211) @@ -8,7 +8,7 @@ */ import numberSuiteCommon from '../../numberSuiteCommon.js'; -import { HBox, HBoxOptions, Node, RichText, RichTextOptions, Text, TextOptions, VBox } from '../../../../scenery/js/imports.js'; +import { Color, HBox, HBoxOptions, Node, RichText, RichTextOptions, Text, TextOptions, VBox } from '../../../../scenery/js/imports.js'; import optionize, { combineOptions, EmptySelfOptions } from '../../../../phet-core/js/optionize.js'; import { Locale } from '../../../../joist/js/i18n/localeProperty.js'; import Property from '../../../../axon/js/Property.js'; @@ -21,6 +21,7 @@ import StringUtils from '../../../../phetcommon/js/util/StringUtils.js'; import NumberSuiteCommonUtteranceQueue from './NumberSuiteCommonUtteranceQueue.js'; import Multilink from '../../../../axon/js/Multilink.js'; +import PageControl from '../../../../sun/js/PageControl.js'; const LABEL_TEXT_OPTIONS = { fontWeight: 'bold', @@ -98,6 +99,17 @@ const voiceCarouselLabel = new Text( NumberSuiteCommonStrings.voiceStringProperty, textOptions ); const noVoiceDescriptionNode = new NoVoiceDescriptionNode( languageCarouselWidth, languageCarousel.height ); + // TODO: radius/color/other stuff + const pageControl = new PageControl( languageCarousel.pageNumberProperty, languageCarousel.numberOfPagesProperty, { + orientation: 'vertical', + pageFill: Color.WHITE, + pageStroke: Color.BLACK, + currentPageStroke: Color.TRANSPARENT, + interactive: true, + dotTouchAreaDilation: 4, + dotMouseAreaDilation: 4 + } ); + const voiceControlVBox = new VBox( { children: [ voiceCarouselLabel, voiceCarousel ], spacing: LABEL_Y_SPACING, @@ -109,7 +121,8 @@ options.children = [ new VBox( { - children: [ languageCarouselLabel, languageCarousel ], + // TODO: actually we want the label to align with the carousel + children: [ languageCarouselLabel, new HBox( { spacing: 5, children: [ pageControl, languageCarousel ] } ) ], spacing: LABEL_Y_SPACING, align: 'left' } ),
chrisklus commented 1 year ago

Patch applied

zepumph commented 1 year ago

For QA: A "PageControl" (dots next to a carousel was added for the language selection in this version. please confirm it works as expected, try to break it, and close if all is well. It is expected to be hidden if there is just one "page" of languages.

Nancy-Salpepi commented 1 year ago

This is working as expected in rc.2. Closing