Sun buttons should discover a default for accessibleName.

[pixelzoom]

Over in MOTHA phetsims/models-of-the-hydrogen-atom#140, I discovered that sun buttons do not discover a default accessibleName.

This could be handled in ButtonNode by adding:

    // If no accessibleName is provided, look for one in the content Node
    if ( !options.accessibleName && this.content ) {
      this.accessibleName = findStringProperty( this.content );
    }

I also see the code below in RectangularRadioButtonGroup. This should be removed -- it definitely not be handled by the group, it should have been handled in RectangularRadioButton. And by making the change above for ButtonNode (superclass of RectangularRadioButton), it should be totally unnecessary.

      // pdom - If the radio button was not assigned an accessibleName, search for a default one
      // in the button. It may come from the button content or its label.
      if ( !radioButton.accessibleName ) {
        radioButton.accessibleName = findStringProperty( button );
      }

It would be great to address this before MOTHA 1.0 RC testing, which will likely begin the week of 3/3/2025.

[pixelzoom]

We're running into this problem again in MOTHA, see phetsims/models-of-the-hydrogen-atom#148.

[jessegreenberg]

This patch completes this issue - it moves the findStringProperty into ButtonNode and out of subclasses. This leaves the following usages of findStringProperty in sun, and I think these need to stay:

• RectangularRadioButtonGroup - Rectangular radio buttons have a feature where the visual label can be placed outside of the button Node. So this findStringProperty should stay to catch that, and it will only run if the button Node doesn't already have a name.
• BooleanRoundToggleButton/BooleanRectangularToggleButton - accessibleName is managed in a specific way for round/rectangular toggle buttons. There is one name for the on state and another for the off state. These buttons have different visual content Nodes for each state too, and findStringProperty is run on each.

Ill apply this patch when I can test more carefully:

Details

Subject: [PATCH] Rename alertStableDelay -> alertDelay, see https://github.com/phetsims/scenery/issues/1782
---
Index: js/buttons/ButtonNode.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buttons/ButtonNode.ts b/js/buttons/ButtonNode.ts
--- a/js/buttons/ButtonNode.ts	(revision e3f4f55e51341a0015539b0cbae97742a5158902)
+++ b/js/buttons/ButtonNode.ts	(date 1773698132696)
@@ -17,6 +17,7 @@
 import Dimension2 from '../../../dot/js/Dimension2.js';
 import optionize, { combineOptions } from '../../../phet-core/js/optionize.js';
 import type StrictOmit from '../../../phet-core/js/types/StrictOmit.js';
+import { findStringProperty } from '../../../scenery/js/accessibility/pdom/findStringProperty.js';
 import { DescriptionResponseOptions } from '../../../scenery/js/accessibility/pdom/ParallelDOM.js';
 import ResponseGroup from '../../../scenery/js/accessibility/pdom/ResponseGroup.js';
 import Voicing, { type VoicingOptions } from '../../../scenery/js/accessibility/voicing/Voicing.js';
@@ -210,6 +211,10 @@
       tandem: options.tandem?.createTandem( 'pressListener' )
     }, options.listenerOptions );
 
+    if ( !options.accessibleName && options.content ) {
+      options.accessibleName = findStringProperty( options.content );
+    }
+
     assert && options.enabledProperty && assert( options.enabledProperty === buttonModel.enabledProperty,
       'if options.enabledProperty is provided, it must === buttonModel.enabledProperty' );
     options.enabledProperty = buttonModel.enabledProperty;
Index: js/buttons/RectangularButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buttons/RectangularButton.ts b/js/buttons/RectangularButton.ts
--- a/js/buttons/RectangularButton.ts	(revision e3f4f55e51341a0015539b0cbae97742a5158902)
+++ b/js/buttons/RectangularButton.ts	(date 1773698489407)
@@ -14,7 +14,6 @@
 import Shape from '../../../kite/js/Shape.js';
 import optionize from '../../../phet-core/js/optionize.js';
 import type PickRequired from '../../../phet-core/js/types/PickRequired.js';
-import { findStringProperty } from '../../../scenery/js/accessibility/pdom/findStringProperty.js';
 import LayoutConstraint from '../../../scenery/js/layout/constraints/LayoutConstraint.js';
 import Node from '../../../scenery/js/nodes/Node.js';
 import Path from '../../../scenery/js/nodes/Path.js';
@@ -155,12 +154,6 @@
 
     super( buttonModel, buttonBackground, interactionStateProperty, options );
 
-
-    // If no accessibleName is provided, look for it in the content.
-    if ( !options.accessibleName && options.content ) {
-      this.accessibleName = findStringProperty( options.content );
-    }
-
     this.buttonNodeConstraint = new RectangularButtonNodeConstraint( this, this.layoutSizeProperty, {
       content: options.content ?? null,
       size: options.size,
Index: js/buttons/BooleanRoundToggleButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buttons/BooleanRoundToggleButton.ts b/js/buttons/BooleanRoundToggleButton.ts
--- a/js/buttons/BooleanRoundToggleButton.ts	(revision e3f4f55e51341a0015539b0cbae97742a5158902)
+++ b/js/buttons/BooleanRoundToggleButton.ts	(date 1773698099368)
@@ -46,8 +46,6 @@
     const toggleNode = new BooleanToggleNode( booleanProperty, trueNode, falseNode );
     options.content = toggleNode;
 
-    super( booleanProperty, false, true, options );
-
     // If no accessibleName is provided, the default behavior finds the accessibleName from the content Nodes.
     // If a content Node does not have text content or if you need to customize the accessibleName,
     // you can provide the accessibleNameOn and/or accessibleNameOff options.
@@ -58,6 +56,8 @@
       options.accessibleNameOff = findStringProperty( falseNode );
     }
 
+    super( booleanProperty, false, true, options );
+
     this.disposeBooleanRoundToggleButton = function() {
       toggleNode.dispose();
     };

[jessegreenberg]

I applied the patch. I ran the change through a before/after comparison tool and do not see any changed or missing accessible names in sims that support description. Testing in the sun PushButtons demo, I can see lots of buttons with an accessible Name that did not have one before (mostly the round buttons because that subclass did not previously have a findStringProperty for the accessible name.

[jessegreenberg]

CT is good with this change, closing.

[phet-dev]

Reopening because there is a TODO marked for this issue.

[jessegreenberg]

Both TODOs are in models-of-the-hydrogen-atom. I will confirm that they can be removed before deleting them. Neither case uses a11y strings, so there will be no strings to clean up/remove.