`supportsInteractiveHighlights: true` should be the default for sims that support alt input

[pixelzoom]

From 9/15/2022 dev meeting notes, with notes by @jessegreenberg

KP: PSA - Interactive Highlights has a query parameter, and will be grouped into standard alternative input design requirement. Jesse - please explain the lift (pretty easy I think).

• Interactive Highlights are highlights that surround interactive Nodes on mouse and touch input to improve low vision access.
• To enable Interactive Highlights add this option to the Sim PreferencesModel.
• Common code components will now have this feature. Custom interactive Nodes will need to be composed with InteractiveHighlighting.ts.
• I made a quick start guide for this and InteractiveHighlighting.ts
• Let me know if you encounter issues!

PhET wants this added to all sims that support alt input. We are already indicating that alt input is supported via supportsInteractiveDescription: true in package.json. So why do we need to take the additional step of adding supportsInteractiveHighlights: true to PreferencesModel? Shouldn't that be the default for sims that support alt input?

A correct default seems preferrable to having to revisit each sim and make a change.

[kathy-phet]

@jessegreenberg - Let me know what you think about CM's suggestion above. Thanks!

[jessegreenberg]

Yes, I think this makes sense! Making this the default has a little more up front cost in enabling this for the ~25 sims that currently support alt input but do not yet support Interactive Highlights. Most will be trivial, but a few might take 15-30 minutes.

@kathy-phet is that OK? Is this something you would like me to do or should I create issues for the responsible dev of each of these sims?

[kathy-phet]

Hey Jesse, Yes, this sounds great. There are 14 sims published that are listed as supporting alt input. So maybe 10 other sims. I don't think we would eagerly republish all 14 sims right now. But if you could get it working on master, that would be great, so its ready when we republish. Sounds like its pretty straightforward. @emily-phet - Do you see any issues with having interactive highlights be the default for sims that support alt input? Kathy

[emily-phet]

This sounds wonderful!

Having interactive highlights be forever forward coupled with alt input is indeed ideal! One clarification, it would be available but "off" by default, correct? I'm guessing that's the case but just wanted to clarify.

[kathy-phet]

Yes - off by default unless you use the query parameter to start with it on - but available to turn on through the preferences dialog.

The user testing that was done with low-vision non-screen reader users on GHE suggests that this would really help them. We are going to re-do the testing in GHE with the interactive highlights available and confirm. But it definitely seems like what they were wishing for in the user feedback reports.

[jessegreenberg]

I got about halfway through the list of sims locally this afternoon, should have this finished soon.

[jessegreenberg]

This was done in the above commits and sims in perennial/ now support Interactive Highlights. Only exception are the CCK sims because alt input isn't really supported for those.

• https://github.com/phetsims/chipper/commit/1c3d1a8f73ee3afe88202b79b73c569daba19ba9 made Interactive Highlights enabled by default when supportsInteractiveDescription is true.
• Commits like https://github.com/phetsims/geometric-optics/commit/108b4a118a4c3463d3428301400ba5026ca94cff added the instrumentation for sim components.
• I also removed the old supportsInteractiveHighlights from package.jsons now that the feature is enabled by default.
• I also made updates to https://github.com/phetsims/phet-info/blob/master/doc/interactive-highlights-quickstart-guide.md.

@kathy-phet would you like QA or someone from design to spot check new highlights in these sims? Would you like a developer to review any changes?

[kathy-phet]

@pixelzoom - Can you please review the code changes here? @KatieWoe - When QA has a lull, this would be good to test. @jessegreenberg - Can you make a QA issue with what to test, and put it in the pipeline? Label it medium-priority. thx.

[pixelzoom]

I can't get to reviewing this for a least a week - I'm on vacation. This seems like should be high priority to review, since it affects many sims, and PhET is in the process of re-releasing many sims. So @kathy-phet @jessegreenberg please assign someone else to review the code.

[pixelzoom]

@kathy contacted me via Slack:

The only sims the interactive highlights infrastructure impact are ones with alt-input, which are my sims. You are in the best position to evaluate it, and it can wait until you are back.

So I'll do the review, sometime during the week of October 17.

[pixelzoom]

Requirements are not described in this issue. And (at first glance), there are a lot of commits above. So to make this review so more smoothly...

@jessegreenberg could you please summarize:

• How should this feature behave from the user's perspective? When is it on, when is it off? Are there are related query parameters, and how do they behave? (All of this would be good to note in the code, if you haven't already done so.)

• Which source code files or commits are relevant for review?

[jessegreenberg]

Certainly,

• For an intro to Interactive Highlights (what it is, how it should behave, relevant query parameters) please see https://github.com/phetsims/phet-info/blob/master/doc/interactive-highlights-quickstart-guide.md - in particular the first paragraphs.
• This commit enables Interactive Highlights by default when supportsInteractiveDescription is true. https://github.com/phetsims/chipper/commit/1c3d1a8f73ee3afe88202b79b73c569daba19ba9
• These are the commits that instrument components with Interactive Highlights:
  • https://github.com/phetsims/geometric-optics/commit/108b4a118a4c3463d3428301400ba5026ca94cff
  • https://github.com/phetsims/fourier-making-waves/commit/7296b451fa7a272131f76fa31267be26157cab12
  • https://github.com/phetsims/ph-scale/commit/9517d1cec87373d2f4265620781797f7f497016c
  • https://github.com/phetsims/mean-share-and-balance/commit/39d36d74a1fa3b065742b6596e9eca28303d8883
  • https://github.com/phetsims/faradays-law/commit/97f1d05f14d9a9e7b8c904b1061c49b092b1e6f1
  • https://github.com/phetsims/sun/commit/387764007bcf99dd67596581d0ba400d21a58f9e
  • https://github.com/phetsims/inverse-square-law-common/commit/ccd716577b8cfb330c3996a62c4c7ee14c9afcae
  • https://github.com/phetsims/wave-interference/commit/2f1e729af3c85a2fee7dc9bb94a14c706e9c13c8
  • https://github.com/phetsims/scenery-phet/commit/51dd8756ad23ad3a49a5e0f10b70ae4c2be7bdf9
• The rest of the commits removed supportsInteractiveHighlights from package.jsons where they are now enabled by default or updated documentation.

[pixelzoom]

This looks great, and is behaving as advertised. I tested Preferences and query parameter in pH Scale, but not other sims.

A few suggestions/questions ...

• [ ] Rename interactiveHighlightsInitiallyEnabled to interactiveHighlightsEnabled. PhET has many other query parameters that set an initial value, and we don't include "initial" in their name.

• [ ] Where should documentation live? interactive-highlights-quickstart-guide.md is very nice. But I'm wondering why it's an entirely separate document, and not just an additional section in alternative-input-quickstart-guide.md -- because alternative-input-quickstart-guide.md currently says nothing about this topic, which is now essential. There's also some redundancy (much of the package.json section) in interactive-highlights-quickstart-guide.md that's already covered in alternative-input-quickstart-guide.md. So... I'd prefer to see interactive-highlights-quickstart-guide.md folded into alternative-input-quickstart-guide.md. But if you feel strongly that it needs to be a standalone document for some reason, I guess it would be OK to add an "Interactive Highlights" to section to alternative-input-quickstart-guide.md that consists solely of a link to interactive-highlights-quickstart-guide.md .

• [ ] Should InteractiveHighlighting be resonspible for setting focusable? In OpticalObjectNode (for example) I now see:

export default class OpticalObjectNode extends InteractiveHighlighting( Node ) {
...
    const options = optionize<OpticalObjectNodeOptions, SelfOptions, NodeOptions>()( {

      // NodeOptions
      tagName: 'div',
      focusable: true,

Should focusable: true (and a default tagName?) be handled by InteractiveHighlighting? If these options are not present, it seems like InteractiveHighlighting will not work, because you can't highlight something that is not focusable.

• [ ] Use of mixins/traits. I'm a little concerned about how much a11y features (in general) use mixins/traits. Elsewhere, PhET's policy is to use mixins/traits sparingly. What is the advantage of using a trait in this case? What are the disadvantages? Did you investigate other approaches, such as subclassing, composition, Node options, etc?

• [ ] The API is getting complicated. To properly add alt input to a custom Node, I need to remember to do a bunch of things that are not tied together in any way. In OpticalObjectNode (for example), I needed to:

• add extends InteractiveHighlighting( Node )

• add options focusable: true and tagName: 'div'

• add a KeyboardDragListener in subclasses

• ...and I'm probably forgetting something by trying to do this from memory :)

  Is there any way to tie this all together, so that there are fewer separate pieces to adding alt input, and less likely that something will be forgotten? This is admittedly a bigger/broader issue that probably deserves its own GitHub issue.

Back to @jessegreenberg.

[jessegreenberg]

Thank you for reviewing!

Rename interactiveHighlightsInitiallyEnabled to interactiveHighlightsEnabled

Currently it is matching the similar query parameters for extra sound and voicing extraSoundInitiallyEnabled, voicingInitiallyEnabled.

Where should documentation live? ... I'm wondering why it's an entirely separate document

PhET is going to enable both at the same time by default, but these features are really different in how they behave and how they are added. I do not think of them as the same feature set. I think if someone is looking for info on "Interactive Highlights" they would first look for a quick start guide just about that feature (and likewise for alt input). I think the "quickstart" guides are most useful when short so I currently have a preference to keep them separate.

To alternative-input-quickstart-guide.md I added

3. Adding `"supportsInteractiveDescription": true` will by default also enable Interactive Highlights.
   See https://github.com/phetsims/phet-info/blob/master/doc/interactive-highlights-quickstart-guide.md for more
   information about this feature.

Should InteractiveHighlighting be resonspible for setting focusable? The API is getting complicated. To properly add alt input to a custom Node, I need to remember to do a bunch of things that are not tied together in any way

Combining these points because I see them as related. I think Interactive Highlighting less related to alt input and more related to mouse/touch input. I could see making scenery more responsible for this feature by automatically adding highlights for Nodes with a input listeners. I think almost every commit in this issue using InteractiveHighlighting surrounds a Node with a PressListener? Ill make a new issue for this.

Use of mixins/traits. I'm a little concerned about how much a11y features (in general) use mixins/traits. Elsewhere, PhET's policy is to use mixins/traits sparingly. What is the advantage of using a trait in this case?

I think there are cases where it simplifies adding multiple sets of features to a component. It is sort of like multiple inheritance. There are also syntax advantages. For example, AmplitudeNumberDisplay.js -

class AmplitudeNumberDisplay extends InteractiveHighlighting( VBox ) {

The superclass is clear and you can easily see that AmplitudeNumberDisplay is in vertical layout. It also "inherits" the features of interactive highlighting. You could change the VBox superclass if you need without changing InteractiveHighlighting. To do this with single inheritance, you would have to change the superclass to InteractiveHighlightingNode, then add the VBox as a child with composition further down in the constructor. That is more disruptive and less clear IMO.

[pixelzoom]

👍🏻 closing.