AquaRadioButton/AquaRadioButtonGroup dynamic layout

[jonathanolson]

AquaRadioButton and AquaRadioButtonGroup should be brought up-to-speed for dynamic layout, looks like this is causing #832.

AquaRadioButton should behave a lot like Checkbox: it's a layout container with the label as its primary content. CheckboxConstraint is almost exactly what we need for AquaRadioButton's layout (perhaps we could factor this out...). We'll need to do a similar "is the mouse/touch area customized" logic presumably for backwards compatibility (unfortunate).

AquaRadioButton should probably also have its own touchAreaXDilation et al., similar to other buttons.

AquaRadioButtonGroup should just FlowBox them, and if it's a vertical one, { stretch: true } things. It should forward touch/mouse area dilations (instead of setting them, even if setting once).

AquaRadioButton is also doing some... almost-DAG-like things by having two parents for the labelNode. AquaRadioButton is ALSO not removing the parents of the label, so it's memory-leaking parents on the label node.

@marlitas, this looks like something either of us could do, thoughts? Looks blocking for #832, but I don't see RPAL on the current iteration schedule.

[jonathanolson]

Implemented above. @marlitas can you review?

[marlitas]

I'm finishing up Calculus Grapher review today, but can hit this tomorrow.

[marlitas]

I was a bit confused by this comment:

 // For now just position content. Future updates could include widthResizable content?

is the content not widthResizable as long as a widthResizable node is passed in? Or is this referring to requiring a widthResizable node? Perhaps adding in margins? Mostly curious.

I also have a patch here that renames content in AquaRadioButtonConstraint to labelNode. I wasn't sure why there was a naming shift moving from AquaRadioButton to the constraint, and was confused at first that content included the radio button as well as the label, not just the label.

Subject: [PATCH] switch to labelNode
---
Index: js/AquaRadioButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/AquaRadioButton.ts b/js/AquaRadioButton.ts
--- a/js/AquaRadioButton.ts	(revision f5b13a12680ef429a404851ab95a43beae7a68d8)
+++ b/js/AquaRadioButton.ts	(date 1678898923037)
@@ -241,49 +241,49 @@
 class AquaRadioButtonConstraint<T> extends LayoutConstraint {
   private readonly radioButton: AquaRadioButton<T>;
   private readonly radioNode: Node;
-  private readonly content: Node;
+  private readonly labelNode: Node;
   private readonly rectangle: Rectangle;
   private readonly options: Required<SelfOptions>;
 
-  public constructor( radioButton: AquaRadioButton<T>, radioNode: Node, content: Node, rectangle: Rectangle, options: Required<SelfOptions> ) {
+  public constructor( radioButton: AquaRadioButton<T>, radioNode: Node, labelNode: Node, rectangle: Rectangle, options: Required<SelfOptions> ) {
     super( radioButton );
 
     this.radioButton = radioButton;
     this.radioNode = radioNode;
-    this.content = content;
+    this.labelNode = labelNode;
     this.rectangle = rectangle;
     this.options = options;
 
     this.radioButton.localPreferredWidthProperty.lazyLink( this._updateLayoutListener );
 
-    this.addNode( content );
+    this.addNode( labelNode );
   }
 
   protected override layout(): void {
     super.layout();
 
-    // LayoutProxy helps with some layout operations, and will support a non-child content.
-    const contentProxy = this.createLayoutProxy( this.content )!;
+    // LayoutProxy helps with some layout operations, and will support a non-child labelNode.
+    const labelNodeProxy = this.createLayoutProxy( this.labelNode )!;
 
-    const contentWidth = contentProxy.minimumWidth;
+    const labelNodeWidth = labelNodeProxy.minimumWidth;
 
-    const minimumWidth = this.radioNode.width + this.options.xSpacing + contentWidth;
+    const minimumWidth = this.radioNode.width + this.options.xSpacing + labelNodeWidth;
 
     const preferredWidth = this.radioButton.localPreferredWidth === null ? minimumWidth : this.radioButton.localPreferredWidth;
 
     // Attempt to set a preferredWidth
-    if ( isWidthSizable( this.content ) ) {
-      contentProxy.preferredWidth = preferredWidth - this.radioNode.width - this.options.xSpacing;
+    if ( isWidthSizable( this.labelNode ) ) {
+      labelNodeProxy.preferredWidth = preferredWidth - this.radioNode.width - this.options.xSpacing;
     }
 
-    // For now just position content. Future updates could include widthResizable content?
-    contentProxy.left = this.radioNode.right + this.options.xSpacing;
-    contentProxy.centerY = this.radioNode.centerY;
+    // For now just position labelNode. Future updates could include widthResizable labelNode?
+    labelNodeProxy.left = this.radioNode.right + this.options.xSpacing;
+    labelNodeProxy.centerY = this.radioNode.centerY;
 
-    // Our rectangle bounds will cover the radioNode and content, and if necessary expand to include the full
+    // Our rectangle bounds will cover the radioNode and labelNode, and if necessary expand to include the full
     // preferredWidth
-    this.rectangle.rectBounds = this.radioNode.bounds.union( contentProxy.bounds ).withMaxX(
-      Math.max( this.radioNode.left + preferredWidth, contentProxy.right )
+    this.rectangle.rectBounds = this.radioNode.bounds.union( labelNodeProxy.bounds ).withMaxX(
+      Math.max( this.radioNode.left + preferredWidth, labelNodeProxy.right )
     );
 
     // Update pointer areas (if the client hasn't customized them)
@@ -296,7 +296,7 @@
     }
     this.radioButton._isSettingAreas = false;
 
-    contentProxy.dispose();
+    labelNodeProxy.dispose();
 
     // Set the minimumWidth last, since this may trigger a relayout
     this.radioButton.localMinimumWidth = minimumWidth;

Back to @jonathanolson!

[jonathanolson]

I was a bit confused by this comment

It was a bad comment!

Also applied the naming changes. Thanks for the review!