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

Create a ScreenSummaryContent class for ScreenView.screenSummaryContent #987

Closed pixelzoom closed 1 month ago

pixelzoom commented 1 month ago

For https://github.com/phetsims/models-of-the-hydrogen-atom/issues/67 ...

@kathy-phet requested a static screen summary for each of the screen in MOTHA. @jessegreenberg pointed me to ScreenViewOptions.screenSummaryContent as the way to do this. The documentation in ScreenView.ts says:

      // {Node|null} the Node with screen summary content to be added to the ScreenSummaryNode, and into PDOM above
      // the Play Area. This Node is added as a child to the ScreenSummaryNode.
      screenSummaryContent: null,

To test, I added this to SpectraScreenView:

  public constructor( model: SpectraModel, tandem: Tandem ) {

    super( {

      // ScreenViewOptions
      isDisposable: false,
      tandem: tandem,
+     screenSummaryContent: new Text( 'This is the Spectra screen summary.', {
+       font: new PhetFont( 24 )
+     } )
    } );

But I do not see anything related to this appearing in A11y View. Am I doing something wrong? Or is A11y View not aware of screenSummaryContent?

pixelzoom commented 1 month ago

Looking at FaradaysLawScreenView, I discovered that screenSummaryContent is not just any Node. It has to specifically be a Node, and with low-level HTML options.

      screenSummaryContent: new Node( {
        tagName: 'p',
        innerContent: 'This is the screen summary for the Spectra screen.'
      } )

And then it shows up in A11y View like this:

screenshot_3549

It seems like a better API would be something like:

screenSummaryContent: ScreenSummaryContent

where ScreenSummaryContent handles the low-level details internally, instead of duplicating those details at the sim level. For example:

class ScreenSummaryContent extends Node {
  public constructor( summaryStringProperty: TReadOnlyProperty<string> ) {
    super( {
      tagName: 'p',
      innerContent: summaryStringProperty
    } );
  }
}
jessegreenberg commented 1 month ago

This is because Text has no accessible content. Something like this should work:

    const contentString = 'This is a screen view for the Spectra screen.';

    super( {

      // ScreenViewOptions
      isDisposable: false,
      tandem: tandem,

      screenSummaryContent: new phet.scenery.Text( contentString, {
        tagName: 'p',
        accessibleName: contentString
      } )
    } );
jessegreenberg commented 1 month ago

It seems like a better API would be something like:

That sounds good, lets do it. ScreenSummaryContent will be a good superclass, and can be extended for the more custom content (like GravityForceLabScreenSummaryNode)

pixelzoom commented 1 month ago

Sounds good. I pushed to MOTHA, but added 2 TODOs that look like this:

//TODO https://github.com/phetsims/joist/issues/987 change this to ScreenSummaryContent
jessegreenberg commented 1 month ago

In https://github.com/phetsims/models-of-the-hydrogen-atom/issues/72 we need more than one element for the summary so our class should support that in a simple way.

jessegreenberg commented 1 month ago

I created ScreenSummaryContent and used it everywhere. @pixelzoom can you please review and take a look at how I used it in MOTHA? Here is the main commit: https://github.com/phetsims/joist/commit/2dca61efb7a4f26e5ef58870f5b5f3060a8988c8.

pixelzoom commented 1 month ago

Looks great, this is a nice improvement. I also reviewed a subclass example in GFLScreenSummaryNode. Closing.