Should selected HomeScreenButton fire on up instead of down?

[jessegreenberg]

Is there a reason that the selected HomeScreenButton fires on down event instead of up? This is especially noticeable now that the zoom feature is enabled by default. If you go to a multi-screen sim and try to zoom in on the selected screen button, it will go right to that screen rather than allow user to zoom in on button.

Could the button fire on pointer up instead?

[ariel-phet]

@jessegreenberg sorry for the delay reply. I see no problem with firing on pointer up for homescreen buttons, especially if it fixes this issue

[jessegreenberg]

Change made above. @chrisklus I saw you listed as an author of HomeScreenButton.js would you mind reviewing this change?

[chrisklus]

Thanks @jessegreenberg - looks great. It was unclear to me why fire on down was the desired behavior when I ported the button from its previous file anyway. Back to you to close if all set.

[jessegreenberg]

OK great, thanks!

[jbphet]

The behavior on touchscreen devices that results from this change is not correct. With this in place, a single touch of a home screen icon goes to the screen, rather than selecting the icon.

[jbphet]

@jessegreenberg - we should coordinate the effort on this one and #689, since they are closely related.

[jessegreenberg]

I see, thanks. It is caused by the touchover listener

    // If you touch an unselected button, it become selected. If then without lifting your finger you swipe over to the
    // next button, that one becomes selected instead.
    this.addInputListener( {
      touchover: event => {
        buttonWasAlreadySelected = homeScreenModel.selectedScreenProperty.value === screen;
        homeScreenModel.selectedScreenProperty.value = screen;
      }
    } );

I am not sure how important this is, would it be OK to remove this? That way both small and large buttons will just fire on up. It looks like you were exploring that in #689 (comment) as well.

[jessegreenberg]

In #689, we decided to revert this change and wait until a decision is made for how to proceed with HomeScreenButtons. This issue may be resolved naturally if the 'swipe selection' feature is removed, or it may be resolved naturally when other changes to HomeScreenButton occur. Unfortunately this regression is in the CCK Virtual Lab RC, so the revert will need to be cherry-picked into that release.

Marking as on-hold until #689 is resolved.

EDIT: Revert made for now in d0be22a

[samreid]

I believe @jbphet indicated he intends to work on this.

[samreid]

While working on phetsims/circuit-construction-kit-common#667, @jonathanolson recommended using the maintenance release process to patch between f08abbb and d0be22a. This commit was only in for 3 days, but we know it affected at least two release branches (for CCK-DC and CCK-DC Virtual Lab). The MR process would be able to discover if it affected any other release branches. @jessegreenberg can you take the lead on this?

Marking as high priority since this part is blocking CCK RC.2.

[jessegreenberg]

@samreid and I used the MR process to verify that CCKDC and CCKDC Virtual Lab were the only release branches that had picked up the breaking commit f08abbb.

[jbphet]

At this point, I think this issue is redundant with #689. I'm going to close this issue and continue the work in the other.