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.
http://scenerystack.org/
MIT License
9 stars 6 forks source link

Preferences Dialog has pointer areas on text #935

Closed AgustinVallejo closed 1 year ago

AgustinVallejo commented 1 year ago

First discussed here: https://github.com/phetsims/keplers-laws/issues/79

Preferences dialog is showing pointer areas on some text which is not focusable. Assigning to @pixelzoom

image

AgustinVallejo commented 1 year ago

On the same note, the Sun Node doesn't hide touch areas even though it has draggable, pickable and focusable all set to false in keplers-laws/js/common/view/KeplersLawsScreenView.ts:89

pixelzoom commented 1 year ago

For the Preferences dialog... The "Increase color contrast..." string is displayed by ProjectorModeToggleSwitch.ts:

37    const projectorModeDescription = new VoicingText( JoistStrings.preferences.tabs.visual.projectorModeDescriptionStringProperty, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS );

The "Add visual highlights..." string is displayed by VisualPreferencesPanel.ts:

      const interactiveHighlightsEnabledSwitchVoicingText = new VoicingText( interactiveHighlightsDescriptionStringProperty, merge( {}, PreferencesDialog.PANEL_SECTION_CONTENT_OPTIONS, {
        readingBlockNameResponse: highlightsReadingBlockNameResponsePatternStringProperty
      } ) );

Both of these are in joist, and were authored by @jessegreenberg. So let's assign this to him to comment/address. For the purposes of Kepler's Laws, you can ignore it.

pixelzoom commented 1 year ago

For sunNode on line 90 of KeplersLawsScreenView.ts... You're explicilty setting pointer areas (and adding input listeners) in BodyNode.ts, regardless of whether the BodyNode is interactive. So that's why you're seeing pointer areas. If you wanted to remove the pointer areas, you'd need to add those things conditionally based on the value of BodyNodeOptions.draggable. But I don't feel like you need to do that - what you're doing now is fine, and we can just ignore the pointer areas.

pixelzoom commented 1 year ago

@AgustinVallejo After you've read through the above comments, please transfer this issue to joist, since the Preferences dialog issue is relevant for all sims.

jessegreenberg commented 1 year ago

Hey @AgustinVallejo, this text is interactive in some modes so the mouse/touch areas are expected. VoicingText is for the Voicing feature, and this text is clickable when Voicing is enabled. For example, see https://phet.colorado.edu/sims/html/quadrilateral/latest/quadrilateral_all.html?voicingInitiallyEnabled.