switch from boolean to string union type, phetsims/density-buoyancy-c…

…ommon#168

Signed-off-by: Michael Kauzmann <[email protected]>

js/accessibility/pdom/PDOMPeer.js

@@ -497,7 +497,7 @@ class PDOMPeer {
 
       // type conversion for DOM spec
       const valueString = `${this.node.inputValue}`;
-      this.setAttributeToElement( 'value', valueString, { asProperty: true } );
+      this.setAttributeToElement( 'value', valueString, { type: 'property' } );
     }
   }
 
@@ -540,7 +540,7 @@ class PDOMPeer {
       namespace: null,
 
       // set as a javascript property instead of an attribute on the DOM Element.
-      asProperty: false,
+      type: 'attribute',
 
       elementName: PRIMARY_SIBLING, // see this.getElementName() for valid values, default to the primary sibling
 
@@ -563,13 +563,13 @@ class PDOMPeer {
     if ( attribute === DISABLED_ATTRIBUTE_NAME && !this.display.interactive ) {
 
       // The presence of the `disabled` attribute means it is always disabled.
-      this._preservedDisabledValue = options.asProperty ? attributeValueWithoutMarks : true;
+      this._preservedDisabledValue = options.type === 'property' ? attributeValueWithoutMarks : true;
     }
 
     if ( options.namespace ) {
       element.setAttributeNS( options.namespace, attribute, attributeValueWithoutMarks );
     }
-    else if ( options.asProperty ) {
+    else if ( options.type === 'property' ) {
       element[ attribute ] = attributeValueWithoutMarks;
     }
     else {

js/accessibility/pdom/ParallelDOM.ts

@@ -318,7 +318,7 @@ export type Association = {
 
 type SetPDOMAttributeOptions = {
   namespace?: string | null;
-  asProperty?: boolean;
+  type?: 'attribute' | 'property'; // javascript Property instead of AXON/Property
   elementName?: string;
 };
 
@@ -2500,7 +2500,7 @@ export default class ParallelDOM extends PhetioObject {
       this._pdomChecked = checked;
 
       this.setPDOMAttribute( 'checked', checked, {
-        asProperty: true
+        type: 'property'
       } );
     }
   }
@@ -2545,12 +2545,13 @@ export default class ParallelDOM extends PhetioObject {
       namespace: null,
 
       // set the "attribute" as a javascript property on the DOMElement instead of a DOM element attribute
-      asProperty: false,
+      type: 'attribute',
 
       elementName: PDOMPeer.PRIMARY_SIBLING // see PDOMPeer.getElementName() for valid values, default to the primary sibling
     }, providedOptions );
 
     assert && assert( !ASSOCIATION_ATTRIBUTES.includes( attribute ), 'setPDOMAttribute does not support association attributes' );
+    assert && options.namespace && assert( options.type === 'attribute', 'property-setting is not supported for custom namespaces' );
 
     // if the pdom attribute already exists in the list, remove it - no need
     // to remove from the peers, existing attributes will simply be replaced in the DOM
@@ -2560,12 +2561,13 @@ export default class ParallelDOM extends PhetioObject {
            currentAttribute.options.namespace === options.namespace &&
            currentAttribute.options.elementName === options.elementName ) {
 
-        if ( !isTReadOnlyProperty( currentAttribute.value ) && currentAttribute.options.asProperty === options.asProperty ) {
+        // We can simplify the new value set as long as there isn't cleanup (from a Property listener) or logic change (from a different type)
+        if ( !isTReadOnlyProperty( currentAttribute.value ) && currentAttribute.options.type === options.type ) {
           this._pdomAttributes.splice( i, 1 );
         }
         else {
 
-          // Swapping asProperty setting strategies should remove the attribute so it can be set as a property.
+          // Swapping type strategies should remove the attribute, so it can be set as a property/attribute correctly.
           this.removePDOMAttribute( currentAttribute.attribute, currentAttribute.options );
         }
       }
@@ -2602,7 +2604,7 @@ export default class ParallelDOM extends PhetioObject {
   /**
    * Remove a particular attribute, removing the associated semantic information from the DOM element.
    *
-   * It is HIGHLY recommended that you never call this function from an attribute set with `asProperty:true`, see
+   * It is HIGHLY recommended that you never call this function from an attribute set with `type:'property'`, see
    * setPDOMAttribute for the option details.
    *
    * @param attribute - name of the attribute to remove

js/accessibility/pdom/ParallelDOMTests.ts

@@ -1035,7 +1035,7 @@ QUnit.test( 'ParallelDOM setters/getters', assert => {
   assert.ok( getPrimarySiblingElementByNode( b ).tabIndex >= 0, 'set tagName after focusable' );
 
   // test setting attribute as DOM property, should NOT have attribute value pair (DOM uses empty string for empty)
-  a1.setPDOMAttribute( 'hidden', true, { asProperty: true } );
+  a1.setPDOMAttribute( 'hidden', true, { type: 'property' } );
   a1Element = getPrimarySiblingElementByNode( a1 );
   assert.equal( a1Element.hidden, true, 'hidden set as Property' );
   assert.ok( a1Element.getAttribute( 'hidden' ) === '', 'hidden should not be set as attribute' );
@@ -1514,21 +1514,21 @@ QUnit.test( 'setPDOMAttribute', assert => {
   a.removePDOMAttributes();
   const attributeName = 'multiTest';
   a.setPDOMAttribute( attributeName, 'true', {
-    asProperty: false
+    type: 'attribute'
   } );
   aElement = getPrimarySiblingElementByNode( a );
-  assert.ok( aElement.getAttribute( attributeName ) === 'true', 'asProperty:false should set attribute' );
+  assert.ok( aElement.getAttribute( attributeName ) === 'true', 'type:attribute should set attribute' );
 
   a.setPDOMAttribute( attributeName, false, {
-    asProperty: true
+    type: 'property'
   } );
-  assert.ok( !aElement.getAttribute( attributeName ), 'asProperty:true should remove attribute' );
+  assert.ok( !aElement.getAttribute( attributeName ), 'type:property should remove attribute' );
 
   // @ts-expect-error for testing
-  assert.equal( aElement[ attributeName ], false, 'asProperty:true should set property' );
+  assert.equal( aElement[ attributeName ], false, 'type:property should set property' );
 
   const testAttributes = a.getPDOMAttributes().filter( a => a.attribute === attributeName );
-  assert.ok( testAttributes.length === 1, 'asProperty change should alter the attribute, not add a new one.' );
+  assert.ok( testAttributes.length === 1, 'type change should alter the attribute, not add a new one.' );
 
   display.dispose();
   display.domElement.parentElement!.removeChild( display.domElement );
@@ -2153,10 +2153,10 @@ QUnit.test( 'Display.interactive toggling in the PDOM', assert => {
   testDisabled( pdomParagraphChild, DISABLED_TRUE, 'pdomParagraphChild second toggled not interactive' );
   testDisabled( pdomButtonChild, DISABLED_TRUE, 'pdomButtonChild second toggled not interactive' );
 
-  pdomParent.setPDOMAttribute( 'disabled', true, { asProperty: true } );
-  pdomRangeChild.setPDOMAttribute( 'disabled', true, { asProperty: true } );
-  pdomParagraphChild.setPDOMAttribute( 'disabled', true, { asProperty: true } );
-  pdomButtonChild.setPDOMAttribute( 'disabled', true, { asProperty: true } );
+  pdomParent.setPDOMAttribute( 'disabled', true, { type: 'property' } );
+  pdomRangeChild.setPDOMAttribute( 'disabled', true, { type: 'property' } );
+  pdomParagraphChild.setPDOMAttribute( 'disabled', true, { type: 'property' } );
+  pdomButtonChild.setPDOMAttribute( 'disabled', true, { type: 'property' } );
 
   testDisabled( pdomParent, DISABLED_TRUE, 'pdomParent not interactive after setting disabled manually as property, display not interactive' );
   testDisabled( pdomRangeChild, DISABLED_TRUE, 'pdomRangeChild not interactive after setting disabled manually as property, display not interactive' );
@@ -2250,7 +2250,7 @@ QUnit.test( 'Display.interactive toggling in the PDOM', assert => {
   testDisabled( pdomButtonChild, DISABLED_TRUE, 'pdomButtonChild turned disabled.' );
   testDisabled( pdomButtonChild, DISABLED_TRUE, 'pdomButtonChild turned disabled with dag.', 1 );
 
-  pdomButtonChild.setPDOMAttribute( 'disabled', true, { asProperty: true } );
+  pdomButtonChild.setPDOMAttribute( 'disabled', true, { type: 'property' } );
 
   testDisabled( pdomButtonChild, DISABLED_TRUE, 'pdomButtonChild turned disabled set property too.' );
   testDisabled( pdomButtonChild, DISABLED_TRUE, 'pdomButtonChild turned disabled set property too, with dag.', 1 );