RectangularRadioButtonGroup does not respect touchAreaDilation options

[samreid]

Probably related to #851. In phetsims/center-and-variability#462, I was asked to dilate the pointer areas for the radio button groups, but the option is not being respected.

Probably being overwritten in

sun/js/buttons/RectangularButton.ts

Lines 180 to 205 in 56a5f02

	Multilink.multilink( [
	this.isWidthResizableProperty,
	this.isHeightResizableProperty,
	this.layoutSizeProperty
	], ( isWidthSizable, isHeightSizable, layoutSize ) => {
	if ( isWidthSizable || isHeightSizable ) {
	buttonBackground.shape = createButtonShape(
	isWidthSizable ? layoutSize.width - this.maxLineWidth : buttonWidth,
	isHeightSizable ? layoutSize.height - this.maxLineWidth : buttonHeight,
	options );
	assert && assert( !isWidthSizable || buttonBackground.width <= layoutSize.width + 1e-7, 'button width cannot exceed layout width' );
	assert && assert( !isHeightSizable || buttonBackground.height <= layoutSize.height + 1e-7, 'button width cannot exceed layout height' );
	}
	if ( isFirstlayout || isWidthSizable || isHeightSizable ) {
	// Set pointer areas.
	this.touchArea = buttonBackground.localBounds
	.dilatedXY( options.touchAreaXDilation, options.touchAreaYDilation )
	.shiftedXY( options.touchAreaXShift, options.touchAreaYShift );
	this.mouseArea = buttonBackground.localBounds
	.dilatedXY( options.mouseAreaXDilation, options.mouseAreaYDilation )
	.shiftedXY( options.mouseAreaXShift, options.mouseAreaYShift );
	}
	isFirstlayout = false;
	} );

.

So this may have been broken in 21fb64f as part of #48.

[samreid]

I have a potential workaround to pass through the pointer dilations to the children, but it would likely clash with fff91db and #708

Subject: [PATCH] Expand touch areas, see https://github.com/phetsims/center-and-variability/issues/462
---
Index: js/buttons/RectangularRadioButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/buttons/RectangularRadioButtonGroup.ts b/js/buttons/RectangularRadioButtonGroup.ts
--- a/js/buttons/RectangularRadioButtonGroup.ts	(revision 036005add1db1fe57725e4e98dd469d1a8d97e35)
+++ b/js/buttons/RectangularRadioButtonGroup.ts	(date 1692742579427)
@@ -221,6 +221,12 @@
       if ( item.voicingContextResponse ) {
         radioButtonOptions.voicingContextResponse = item.voicingContextResponse;
       }
+      if ( options.touchAreaXDilation ) {
+        radioButtonOptions.touchAreaXDilation = options.touchAreaXDilation;
+      }
+      if ( options.touchAreaYDilation ) {
+        radioButtonOptions.touchAreaYDilation = options.touchAreaYDilation;
+      }
 
       const radioButton = new RectangularRadioButton( property, item.value, radioButtonOptions );
 

[jonathanolson]

This might be good to pair on sometime.

sun/js/buttons/RectangularRadioButtonGroup.ts

Lines 283 to 295 in 08180ca

	buttonWithLayoutParent.radioButton.touchArea = Shape.rectangle(
	-options.touchAreaXDilation - maxLineWidth / 2,
	-options.touchAreaYDilation - maxLineWidth / 2,
	maxButtonWidth + 2 * options.touchAreaXDilation,
	maxButtonHeight + 2 * options.touchAreaYDilation
	);
	buttonWithLayoutParent.radioButton.mouseArea = Shape.rectangle(
	-options.mouseAreaXDilation - maxLineWidth / 2,
	-options.mouseAreaYDilation - maxLineWidth / 2,
	maxButtonWidth + 2 * options.mouseAreaXDilation,
	maxButtonHeight + 2 * options.mouseAreaYDilation
	);

looks like it should be changed, presumably it should be creating the buttons with the proper touch areas.

[samreid]

What if we comment out the code in #856 (comment) and apply the patch in #856 (comment) ? it works well when the buttons are the same size. When the buttons are different sizes, are you saying we would compute a different dilation for each so they will have the same touch or mouse areas on startup? But they could change size at runtime based on dynamic strings.

Would it be OK if 2 radio buttons with different sizes have different touch areas? That would simplify things if it is OK.

[pixelzoom]

Rather than every sim having to dilate pointer areas for the buttons in RectangularRadioButtonGroup, it might be nice if the pointer areas were auto-dilated to fill the space between the buttons. This would be consistent with AquaRadioButtonGroup and CheckboxGroup, and would make sims consistent.

[samreid]

In phetsims/center-and-variability#462 it was decided that this is not blocking for CaV. Self-unassigning, but I would recommend review at the next iteration planning in case we want to schedule it.

[samreid]

Please see notes from @pixelzoom in phetsims/projectile-data-lab#187 (comment) and below

[marlitas]

Dev Meeting 3/7/24

• CM: It would be ideal to refactor RectangularRadioButtonGroup using dynamic layout as well as the radioButtonGroup options.
• @marlitas and @jonathanolson will work on these refactors and bugs.

[jonathanolson]

@samreid I believe this is resolved with the above commit, can you confirm?

[samreid]

To test the change, I temporarily added a large y-dilation to the sample size radio buttons in the 4th screen of Projectile Data Lab like so:

Subject: [PATCH] Fix a spelling error, see https://github.com/phetsims/faradays-electromagnetic-lab/issues/2
---
Index: js/sampling/view/SampleSizeSection.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/sampling/view/SampleSizeSection.ts b/js/sampling/view/SampleSizeSection.ts
--- a/js/sampling/view/SampleSizeSection.ts	(revision 5d387795957446a92d9d439511035cfd0f6d6a51)
+++ b/js/sampling/view/SampleSizeSection.ts	(date 1710452709788)
@@ -69,7 +69,8 @@
       },
       layoutOptions: {
         align: 'center'
-      }
+      },
+      touchAreaYDilation: 20
     } );
     super( ProjectileDataLabStrings.sampleSizeNStringProperty, sampleSizeRadioButtonGroup, providedOptions );
   }

The result worked very nicely:

[samreid]

I confirmed the behavior is good, and the first commit f80ffeb is great. However, I don't feel like I can evaluate the second commit ce5d930. It overlaps with work done by @jessegreenberg in: fff91db

@jessegreenberg are you available to complete this review?