From d874df54826dce21a9c5117e2ab8e2f6b120bc73 Mon Sep 17 00:00:00 2001 From: Chris Malley Date: Thu, 24 Jan 2019 11:15:20 -0700 Subject: [PATCH] add highlightCornerRadius, move responsibility for highlight into ComboBoxListItemNode, #453 Signed-off-by: Chris Malley --- js/ComboBox.js | 5 ++++- js/ComboBoxListBox.js | 15 +-------------- js/ComboBoxListItemNode.js | 35 ++++++++++++++++------------------- 3 files changed, 21 insertions(+), 34 deletions(-) diff --git a/js/ComboBox.js b/js/ComboBox.js index a15a6166..8d56dc31 100644 --- 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 index dea4c001..3710f82c 100644 --- 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 index 0cdd59ee..bfbc27ab 100644 --- 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; } + } ); } }