ComboBox plays both the "no change" and selection sound when closed

[jbphet]

While reviewing https://github.com/phetsims/sun/issues/768 I noticed that the sound generation behavior for ComboBox has changed. As originally designed, an individual selection sound would be played if the user selected something different, and a generic "close" sound was played if the user opened the ComboBox, then closed it without changing anything. On the current version of master, both the generic and selection-specific sounds are played. This can be easily demonstrated in the current master version of Molarity.

[jbphet]

From what I can tell, this problem was introduced in 8b4861fd80122b2026b92815763b65222a74c96b. With this change, focusButtonCallback is now being called before setting the property value. Unfortunately, focusButtonCallback also hides the list box. I'm not sure that this is intentional though, because there is a comment several lines later that says "hide the list". Here's what that looks like in context:

      // So that something related to the ComboBox has focus before changing Property value.
      focusButtonCallback();

      // set value based on which item was chosen in the list box
      property.value = listItemNode.item.value;

      // hide the list
      hideListBoxCallback();

The closedNoChange sound player is being triggered by the hiding of the list, and it happens every time now, since the list is becoming invisible before the property change occurs.

@pixelzoom - Since you made this change, I wanted to ask: Is it intentional that the list box be hidden at this point? If so, I can investigate alternative places to identify the list-box-closed-with-no-change situation, but I thought I'd check with you first, since the behavior doesn't seem to quite match the code flow and documentation.

[pixelzoom]

Yes, https://github.com/phetsims/sun/commit/8b4861fd80122b2026b92815763b65222a74c96b was very intentiional. And it's linked to the problem it fixes, https://github.com/phetsims/sun/issues/721, which I worked on with @jessegreenberg. Also note the comment that was added above the call to focusButtonCallback.

// So that something related to the ComboBox has focus before changing Property value.
focusButtonCallback();

The sound code seems to be making bad assumptions about when the button receives focus. The button must receive focus whenever the listbox is popped down, regardless of whether a new selection was made. So you probably should not be playing sounds in focusButtonCallback.

[jbphet]

This problem has gone away. I dug into the code a bit in an effort to understand why, since it seems like it could potentially come back as easily as it disappeared, but most of the code related to the sound production and the hiding and showing of the list box hasn't changed in a way that would obviously affect this. My best guess is that something changed in the focus code that made it so that focusButtonCallback no longer hides the list box, and it doesn't seem like a good use of my time to track it all the way down. While testing this, I added debug code and could definitively see that the property value is now being set before the list box's visibility is set to false, which is what is needed for the sounds to be correctly produced.

I'll close this, but will my eye out for any odd behavior associated with sound production in combo boxes in this use case.