add highlightCornerRadius, move responsibility for highlight into Com…

…boBoxListItemNode, #453 Signed-off-by: Chris Malley <[email protected]>
phetsims · Jan 24, 2019 · d874df5 · d874df5
1 parent 1ee367d
commit d874df5
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 34 deletions.
diff --git a/js/ComboBox.js b/js/ComboBox.js
@@ -55,11 +55,12 @@ define( require => {
         labelXSpacing: 10, // horizontal space between label and combo box
         enabledProperty: new BooleanProperty( true ),
         disabledOpacity: 0.5, // {number} opacity used to make the control look disabled, 0-1
-        cornerRadius: 4, // applied to list and button
+        cornerRadius: 4, // applied to button, listBox, and item highlights
         highlightFill: 'rgb( 245, 245, 245 )', // {Color|string} highlight behind items in the list
 
         // Margins around the edges of the button and listbox when highlight is invisible.
         // Highlight margins around the items in the list are set to 1/2 of these values.
+        // These values must be > 0.
         xMargin: 12,
         yMargin: 8,
 
@@ -84,6 +85,8 @@ define( require => {
       }, options );
 
       // validate option values
+      assert && assert( options.xMargin > 0 && options.yMargin > 0,
+        'margins must be > 0, xMargin=' + options.xMargin + ', yMargin=' + options.yMargin );
       assert && assert( options.disabledOpacity > 0 && options.disabledOpacity < 1,
         'invalid disabledOpacity: ' + options.disabledOpacity );
       assert && assert( LIST_POSITION_VALUES.includes( options.listPosition ),

diff --git a/js/ComboBoxListBox.js b/js/ComboBoxListBox.js
@@ -64,7 +64,6 @@ define( require => {
 
           // hide the list
           hideListBoxCallback();
-          listItemNode.setHighlightVisible( false );
 
           // prevent nodes (eg, controls) behind the list from receiving the event
           event.abort();
@@ -81,18 +80,6 @@ define( require => {
         phetioEventType: 'user'
       } );
 
-      // Highlights the item that the pointer is over.
-      const highlightListener = {
-
-        enter( event ) {
-          event.currentTarget.setHighlightVisible( true );
-        },
-
-        exit( event ) {
-          event.currentTarget.setHighlightVisible( false );
-        }
-      };
-
       //TODO sun#462 replace fireEmitter and selectionListener with a standard scenery listener
       // Handles selection from the list box.
       const selectionListener = {
@@ -131,6 +118,7 @@ define( require => {
         const listItemNode = new ComboBoxListItemNode( item, highlightWidth, highlightHeight, {
           align: options.align,
           highlightFill: options.highlightFill,
+          highlightCornerRadius: options.cornerRadius,
 
           // highlight overlaps half of margins
           xMargin: 0.5 * options.xMargin,
@@ -140,7 +128,6 @@ define( require => {
         } );
         listItemNodes.push( listItemNode );
 
-        listItemNode.addInputListener( highlightListener );
         listItemNode.addInputListener( selectionListener );
       } );
 

diff --git a/js/ComboBoxListItemNode.js b/js/ComboBoxListItemNode.js
@@ -1,7 +1,8 @@
 // Copyright 2019, University of Colorado Boulder
 
 /**
- * Node for an item in a combo box list.  Can be highlighted by calling setHighlightVisible.
+ * Node for an item in a combo box list.
+ * Responsible for highlighting itself when the pointer is over it.
  * Typically instantiated by ComboBox, not by client code.
  *
  * @author Chris Malley (PixelZoom, Inc.)
@@ -34,7 +35,8 @@ define( require => {
         cursor: 'pointer',
         align: 'left',
         xMargin: 6,
-        highlightFill: 'rgb( 245, 245, 245 )', // {Color|string}
+        highlightFill: 'rgb( 245, 245, 245 )', // {Color|string} highlight behind the item
+        highlightCornerRadius: 4, // {number} corner radius for the highlight
 
         // phet-io
         tandem: Tandem.required,
@@ -50,10 +52,12 @@ define( require => {
       assert && assert( options.innerContent === undefined, 'ComboBoxListItemNode sets innerContent' );
       options.innerContent = item.a11yLabel;
 
-      // Highlight rectangle
-      const highlightRectangle = new Rectangle( 0, 0, width, height );
+      // Highlight that is shown when the pointer is over this item. This is not the a11y focus rectangle.
+      const highlightRectangle = new Rectangle( 0, 0, width, height, {
+        cornerRadius: options.highlightCornerRadius
+      } );
 
-      // Wrapper for the item's Node. Do not transform ComboBoxItem.node because it is shared with ComboBoxButton!
+      // Wrapper for the item's Node. Do not transform item.node because it is shared with ComboBoxButton!
       const itemNodeWrapper = new Node( {
         children: [ item.node ],
         centerY: height / 2
@@ -76,23 +80,16 @@ define( require => {
       // @public (read-only)
       this.item = item;
 
-      // @private
-      this.highlightRectangle = highlightRectangle;
-      this.highlightFill = options.highlightFill;
-
-      // focus highlight is fitted to this Node's bounds, so that it doesn't overlap items above/below in the list box
+      // a11y focus highlight is fitted to this Node's bounds, so that it doesn't overlap other items in the list box
       this.focusHighlight = Shape.bounds( this.localBounds );
-    }
-
-    /**
-     * Sets visibility of the highlight that appear's behind the item's node. (This is not the a11y focus highlight.)
-     * @param {boolean} visible
-     * @public
-     */
-    setHighlightVisible( visible ) {
 
+      // Show highlight when pointer is over this item.
       // Change fill instead of visibility so that we don't end up with vertical pointer gaps in the list
-      this.highlightRectangle.fill = visible ? this.highlightFill : null;
+      this.addInputListener( {
+        enter() { highlightRectangle.fill = options.highlightFill; },
+
+        exit() { highlightRectangle.fill = null; }
+      } );
     }
   }