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

Set pdomOrder for pdomPlayAreaNode and pdomControlAreaNode #291

Open
pixelzoom opened this issue Sep 5, 2024 · 4 comments
Open

Set pdomOrder for pdomPlayAreaNode and pdomControlAreaNode #291

pixelzoom opened this issue Sep 5, 2024 · 4 comments

Comments

@pixelzoom
Copy link
Contributor

Related to the description tool prototype...

Another thing that probably needs to be addressed in ph-scale/ph-scale-basics is pdomOrder. We are not currently setting pdomOrder for the play area and control area (pdomPlayAreaNode and pdomControlAreaNode respectively). We’re still using the old-style approach of setting pdomOrder on a single “root node” (screenViewRootNode) that’s a child of the ScreenView.

@arouinfar please let me know how you’d like things allocated to play area vs control area.

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Sep 5, 2024

Here's the current pdomOrder:

For the Macro screen:

      screenViewRootNode.pdomOrder = [
        pHMeterNode,
        soluteComboBox,
        dropperNode,
        waterFaucetNode,
        drainFaucetNode,
        resetAllButton
      ];

For the Micro screen:

    screenViewRootNode.pdomOrder = [
      pHAccordionBox,
      soluteComboBox,
      dropperNode,
      waterFaucetNode,
      drainFaucetNode,
      beakerControlPanel,
      graphNode,
      resetAllButton
    ];

For the My Solution screen:

    screenViewRootNode.pdomOrder = [
      pHAccordionBox,
      beakerControlPanel,
      graphNode,
      resetAllButton
    ];

@pixelzoom
Copy link
Contributor Author

Instead of setting screenViewRootNode.pdomOrder as shown in #291 (comment), I am now setting this.pdomPlayAreaNode.pdomOrder. So for example in MySolutionScreenView:

    // Play Area focus order
    this.pdomPlayAreaNode.pdomOrder = [
      pHAccordionBox,
      beakerControlPanel,
      graphNode,
      resetAllButton
    ];

    // Control Area focus order
    this.pdomControlAreaNode.pdomOrder = [
      //TODO https://github.com/phetsims/ph-scale/issues/291
    ];

@pixelzoom
Copy link
Contributor Author

pixelzoom commented Sep 24, 2024

@arouinfar and I have paused work on this issue, because we discovered that the description plugin is setting pdomOrder, and breaks if the sim code sets pdomOrder.

In MacroScreenView.ts, a guard was added around setting pdomOrder in 420ac61, because it apparently conflicts with the description plugin:

    if ( !phet.chipper.queryParameters.supportsDescriptionPlugin ) {

      // Play Area focus order
      this.pdomPlayAreaNode.pdomOrder = [
        ...
      ];

      // Control Area focus order
      this.pdomControlAreaNode.pdomOrder = [
        ...
      ];
    }

And in ph-scale-basics-description-logic.sh, the plugin is setting the pdomOrder for macroScreenView:

        /*********************************************
         * PDOM Order
         *********************************************/

        // Play Area
        context.nodeSet( macroScreenView.pdomPlayAreaNode, 'pdomOrder', [
          beakerNode,
          phMeterHeading,
          controlsHeading,
          soluteComboBox,
          dropperNodeButton,
          phMeterProbeNode,
          waterFaucetNode,
          drainFaucetNode
        ] );

        // Control Area
        context.nodeSet( macroScreenView.pdomControlAreaNode, 'pdomOrder', [
          resetAllButtonNode
        ] );

@jessegreenberg @jonathanolson Questions related to pdomOrder and the description plugin:

  • Will the description plugin be the way that pdomOrder is designed and implemented?

  • Does the description plugin also handle (or will it need to handle) pdomOrder that's not at the ScreenView level? For example, ph-scale has several other places where pdomOrder is set (search for pdomOrder =) in components that are not used by ph-scale-basics. Will those cases also conflict with the description plugin + ph-scale?

  • Will the if ( !phet.chipper.queryParameters.supportsDescriptionPlugin ) in MacroScreenView eventually be removed, and the correct pdomOrder set therein? Or will ph-scale-basics-description-logic.sh now be responsibile for setting pdomOrder in the published sim? And somehow bundled into the published sim?

  • How will code that appears in *-logic.sh be reused across a suite of sims? For example (and related to this issue), the pdomOrder in ph-scale-basics-description-logic.sh is for the Macro screen, which lives in ph-scale and is used by both ph-scale and ph-scale-basics. That pdomOrder should certainly not be duplicated in ph-scale-description-logic.sh and ph-scale-basics-description-logic.sh. And this probably applies to most of what is in ph-scale-basis-description-logic.sh -- it should live in ph-scale-description-logic.sh, and the bits that are relevant to ph-scale-basics should be reused in that sim.

@pixelzoom
Copy link
Contributor Author

At 10/10/24 iteration planning meeting, this work was put on hold.

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

4 participants