Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to set pdomOrder in CalculusGrapherScreenView #123

Closed
pixelzoom opened this issue Dec 5, 2022 · 6 comments
Closed

Make it possible to set pdomOrder in CalculusGrapherScreenView #123

pixelzoom opened this issue Dec 5, 2022 · 6 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Dec 5, 2022

So that this sim can easily support alternative input in the future....

In CalculusGrapherScreenView.ts, replace this:

    this.addChild( this.graphsNode );
    this.addChild( controlPanel );
    this.addChild( toolsCheckboxGroup );
    this.addChild( resetAllButton );

... with:

    const screenViewRootNode = new Node( {
      children: [
        this.graphsNode,
        controlPanel,
        toolsCheckboxGroup,
        resetAllButton
      ]
    } );
   this.addChild( screenViewRootNode );

The reason for this is covered in detail in "Approach 2" of https://github.com/phetsims/phet-info/blob/master/doc/alternative-input-quickstart-guide.md#traversal-order. But in a nutshell, focus traversal (aka "tab order") must to be set via the pdomOrder property of a Node, and it cannot be set directly on the ScreenView. So (eventually, you don't need to do this now) the pdomOrder will be specified in CalculusGrapherScreenView.ts like this:

screenViewRootNode.pdomOrder = [
  this.graphsNode,
  controlPanel,
  toolsCheckboxGroup,
  resetAllButton
];

Let me know if you have any questions.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Dec 5, 2022

I see that addChild( graphSetRadioButtonGroup ) is also called in subclasses AdvancedScreenView and LabScreenView. So in CalculusGrapherScreenView, you'll need to add:

protected readonly screenViewRootNode: Node

... and in the subclasses:

this.screenViewRootNode.addChild( graphSetRadioButtonGroup )

@pixelzoom
Copy link
Contributor Author

I'll do this while working on #106.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Dec 5, 2022

This was addressed in the above commit. @veillette ready for review.

Note that responsibility for conditionally creating graphSetRadioButtonGroup was moved to CalculusGrapherScreenView in 305f35e while working on #106.

@veillette
Copy link
Contributor

Let's discussed today if you should set a flag to support alternative input in package.json or deferred to a later time.

@pixelzoom
Copy link
Contributor Author

Yes, let's discuss. But I don't think that we should enable alternative input support yet, because of #125.

@veillette
Copy link
Contributor

On 12/6, we discussed the PDOM and how we can test the PDOM in phetBrand . In #125, we will addressed the general question of how to support alternative input for the drag handler. The work done here sets the stage to make it easier to set the PDOM in CalculusGrapherScreenView, without actually implementing a PDOM order. We can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants