phetsims / a11y-research

a repository to track PhETs research into accessibility, or "a11y" for short
MIT License
3 stars 0 forks source link

Consider using a JSX-style representation #99

Open samreid opened 6 years ago

samreid commented 6 years ago

Currently a11y is hard-coded to use 4 peers per Node, specified like so:

        var a11yNode = new scenery.Node( {

          // a11y
          tagName: 'button',
          innerContent: 'Grab Magnet',
          labelTagName: 'h3',
          labelContent: 'Magnet',
          descriptionTagName: 'p',
          descriptionContent: 'Magnet has a grey south pole on the left and a red north pole on the right.',
          containerTagName: 'section'
        } );

I wonder if it would give us more control/flexibility/readability/maintainability to use a JSX style representation in the simulation code:

var a11yNode = new scenery.Node({
  pdom: <section>
          <h3>Magnet</h3>
          <p>Magnet has a grey south pole on the left and a red north pole on the right.</p>
          <button>Grab Magnet </button>
        </section>
});

I'm presuming scenery would still assign and maintain tabindex and id.

Potential drawbacks of a JSX approach are:

  1. Needing a new transformation step during builds or requirejs running
  2. Perhaps it is too flexible and it is better to provide more structure for ourselves
  3. Development team would need to understand JSX as a language and its constraints.

Potential advantages:

  1. More flexibility in creation and maintenance of PDOM
  2. More similar to vanilla HTML, hopefully easier to read/test/debug

Please do not mistake this issue for strong advocacy to jump in to a radically different strategy--I'm mainly wondering if it (or something like it) has been discussed before or if there are other pros/cons to consider.

jessegreenberg commented 6 years ago

Thanks @samreid, JSX is not something I was aware of and we had not considered it. I would like to think about it more.

I agree with your pros/cons list. Before the Accessibility trait, we had syntax that was kind of similar to this (though a lot more verbose), things looked something like:

var a11yNode = new scenery.Node( {
  accessibleContent: {
    function: createPeer() {
      var section = document.createElement( 'section' );
      var heading = document.createElement( 'h3' );
      var paragraph = document.createElement( 'p' );
      var button = document.createElement( 'button' );

      section.appendChild( heading );
      section.appendChild( paragraph );
      section.appendChild( button );

      return new AccessiblePeer( section );
    }
  }
} );

which gave lots of flexibility, similar to the JSX style.

We discovered that this is generally as complex as the structure gets for description content for a Node. And for more complicated HTML it is more straight forward to leverage the scene graph.

Another benefit of the current approach is having state variables and getters/setters for properties of the possible HTML elements. After initial definition with the current approach, we could update the description element directly with

a11yNode.descriptionContent = 'Some new description';

instead of recreating the whole collection of PDOM content.

The current approach is also nice for inheritance.

function SuperType( options ) {
  options = _.extend( {
    // a11y
    tagName: 'button'
  }, options );
  ...
}
function SubType( options ) {
  options  =_.extend( {

    // a11y
    innerContent: 'Accessible name'
  } );
  SuperType.call( this, options );
  ...
}

We could probably come up with a solution for this that works with JSX syntax, but this conforms nicely with the options convention that is used by the project.

jessegreenberg commented 6 years ago

Adding to a11y dev meeting agenda.

zepumph commented 6 years ago

to be discussed further in the meeting.

mbarlow12 commented 6 years ago

Here are a few more considerations:

mbarlow12 commented 6 years ago

I found a great, brief explanation of JSX. https://jasonformat.com/wtf-is-jsx/

samreid commented 6 years ago

I mentioned this in https://github.com/phetsims/color-vision/issues/121#issuecomment-388251479 but will clarify from a perspective more specific to this issue:

Today, @jonathanolson and @pixelzoom pointed out that what will serve us best is a higher-level abstraction, where the client code doesn't code tags or HTML structure, but rather a more "domain specific" representation/API. For instance, one example that @jonathanolson gave is that we wouldn't code explicit h2, h3 tags, but rather that we would "push" or "pop" a level of nesting (my words, not @jonathanolson's), and scenery would automatically translate that into the appropriate level of headers.

I originally advocated for using HTML as the API directly, but @jonathanolson and @pixelzoom convinced me that a higher-level abstraction/API will serve us best (as it has for scenery graphics). However, the individual components (such as ComboBox or RadioButton) will still need to efficiently be able to encode HTML, and for this it is still possible that a more flexible representation (rather than primaryTag/labelTag/descriptionTag/otherTag) will be most productive.

To clarify: I suspect what will work best are component-specific APIs (like ComboBox will have a ComboBox-specific a11y API, and RadioButton will have a RadioButton-specific a11y API), but for the internal implementations of ComboBox a11y and RadioButton a11y, we could use (a) primaryTag/labelTag/descriptionTag/otherTag as we have now or (b) fully general HTML using document.createElement or (c) fully general HTML using JSX or similar. Whatever implementation strategy we use for creating the HTML shouldn't "leak" into the component-specific APIs. This also makes it a little less critical which of these strategies we settle on (since we are now deciding an isolated implementation detail rather than an overarching API that will be widely used).

zepumph commented 6 years ago

The a11y dev team talked about this during the a11y sprint a lot, and it seems like we are learning more towards a system that takes the html writing out of the devs' hands. I'm not yet ready to close this issue entirely, but I'm going to unassign for now, still marked for a11y dev meeting.

mbarlow12 commented 6 years ago

@zepumph I agree. There seem to be some solid advantages like being able to very specifically target a given HTML structure (radio buttons and comobox are good examples where our API is somewhat cumbersome and a JSX/React style templating language would be pretty straightforward).

Since we are on the verge of transitioning to ES6+, rolling this feature into the build steps that the update would require could be relatively easy. I think it behooves us to adequately vet this option and make a decision prior to the ES6 (whenever that may be).

jessegreenberg commented 6 years ago

Removing assignment since we will talk about this more at an upcoming meeting.

mbarlow12 commented 6 years ago

@zepumph @jessegreenberg do either of you think this is still under consideration? I would vote to close and create a new issue if the topic arises again.

zepumph commented 6 years ago

Sounds good to me. I think that for now, we don't have the resources to adopt such large scale pivot. Closing.

samreid commented 1 year ago

I mentioned this again today in slack:

Do you want to use React to write code like this?

    // opening paragraph for the simulation
    const openingSummaryNode = new Node( { tagName: 'p', innerContent: simOpeningString } );
    this.addChild( openingSummaryNode );

    // list of dynamic description content that will update with the state of the simulation
    const listNode = new Node( { tagName: 'ul' } );
    const roomObjectsNode = new Node( { tagName: 'li' } );
    const objectPositionsNode = new Node( { tagName: 'li', innerContent: initialObjectPositionsString } );
    const balloonChargeNode = new Node( { tagName: 'li' } );
    const sweaterWallChargeNode = new Node( { tagName: 'li' } );
    const inducedChargeNode = new Node( { tagName: 'li' } );

    // structure the accessible content
    this.addChild( listNode );
    listNode.addChild( roomObjectsNode );
    listNode.addChild( objectPositionsNode );
    listNode.addChild( balloonChargeNode );
    listNode.addChild( sweaterWallChargeNode );
    listNode.addChild( inducedChargeNode );
    this.addChild( new Node( { tagName: 'p', innerContent: grabBalloonToPlayString } ) );

@jessegreenberg said:

Code like that is one of the more clumsy parts of the API. Interested in thinking about how React or another strategy might improve it. Its clumsy because a single place needs a really custom/complicated HTML structure for accessibility. Most use cases are represented by a single element.

I wrote:

We can use TSX like this, if we want:

const openingSummaryNode = <Node tagName="p" innerContent={simOpeningString}>
  <Node tagName="ul"/>
  <Node tagName="li"/>

  // opening paragraph for the simulation
  <Node tagName="li" innerContent={initialObjectPositionsString}/>
  <Node tagName="li"/>
  <Node tagName="li"/>
</Node>;

Might align it more with conventional aria implementation strategy?

@jessegreenberg replied:

Yea, looks much more like a typical front-end framework.

samreid commented 1 year ago

When this issue was written in 2018, we did not have a transpiler step. Hence "Needing a new transformation step during builds or requirejs running" was a much larger barrier. Also, JSX has evolved and matured since then, and now TSX is also supported. Our team got experience working with JSX in the website (@mattpen @chrisklus) as well as in PhET-iO as part of https://github.com/phetsims/chipper/issues/1268. Therefore, I think it is appropriate to reopen this issue for consideration. I'll also mark it epic to indicate the effort level.

jessegreenberg commented 1 year ago

This could be a useful change and better in some ways/cases. Though what we currently have is more aligned with scenery style development. This would be good to explore but removing assignments for now since there are other issues currently that are more important for enhancing scalability of accessibility in scenery.