Moving zoom slider or launching reenables zoom buttons

[KatieWoe]

Device
Dell
OS
Win 10
Browser
Chrome
Problem Description
For phetsims/qa#657..
When zoom buttons are disabled, but the zoom slider is not, moving the slider reenables the buttons. Using the buttons while the slider is disabled does not seem to reenable the slider.
Visuals

EDIT
More significantly, the buttons reenable when the sim is launched, meaning this doesn't seem to be saved in state.

[samreid]

The code that causes the behavior is this:

    // add disabling effect for buttons
    if ( isIncrease ) {

      // plus button
      scaleProperty.link( scaleValue => this.setEnabled( scaleValue !== range.max ) );
    }
    else {

      // minus button
      scaleProperty.link( scaleValue => this.setEnabled( scaleValue !== range.min ) );
    }

link runs on startup and when the property changes. @arouinfar would you like to solve this problem by making the button enabledProperty phetioReadOnly, and encourage users to make the buttons invisible if they shouldn't be used?

[arouinfar]

When zoom buttons are disabled, but the zoom slider is not

I don't think this is a reasonable customization, and I'm not bothered by the odd outcome.

One of the client requests is to set the zoom level and disable or hide it, and we instruct them to disable each part of the zoom control. A better user experience would be to have something like zoomControl.enabledProperty which would disable the buttons and the slider, but I don't see it as a critical update.

More significantly, the buttons reenable when the sim is launched, meaning this doesn't seem to be saved in state.

That is a serious issue, but I am unable to reproduce it in master. @KatieWoe can you reproduce in master?

[KatieWoe]

Took me awhile to figure this out. It is happening on master, but it takes some extra steps (same in the RC). Basically, this happens when the slider is in a non-default position.
Steps:

1. Move the zoom slider away from it's starting position.
2. Disable buttons and/slider with studio
3. Launch

[samreid]

More significantly, the buttons reenable when the sim is launched, meaning this doesn't seem to be saved in state.

The values are saved in the state, but overridden when the slider value changes. When the slider state is loaded in the sim, the values is changed, and the buttons enable/disable accordingly. The rule for the sim is that if the slider is at min or max, that button should be disabled. Otherwise the buttons should be enabled. This rule is enforced whenever the slider value changes. Therefore, this is basically the came cause and effect as in #392 (comment), and we have similar options:

• Make it so that link does nothing during set state. That would allow the creator to make a bad state in studio, and launch the bad state.
• Make it so after set state, the link automatically fires. That means the creator could create a bad state in studio, then launch would correct it.
• Leave it as it is now
• Find a way to make it more difficult for the creator to create a bad state in the first place.

@arouinfar can you please advise?

[arouinfar]

The odd behavior arises from an incomplete/unrealistic customization, so I am fine with leaving things as-is @samreid.

[KatieWoe]

I'm not so sure it's unrealistic. A user might set a custom zoom and then disable the controls for the user. This wouldn't work and it did seem to be requested. Or am I misunderstanding?

[arouinfar]

I thought weird things happen because the zoom controls weren't fully disabled (just the buttons, not the slider). If you want to prevent users from changing the zoom level, both the buttons and the slider need to be disabled.

[samreid]

Yes, but adjusting the slider, then disabling the slider + buttons, then pressing "preview sim", re-enables the buttons (and the slider stays disabled).

[arouinfar]

Ah, I missed the part about disabling the slider too. This is definitely publication-blocking.

@samreid would this issue be circumvented by adding something like zoomControl.enabledProperty?

[samreid]

In the commit, I instrumented zoomControl.enabledProperty and marked the slider and buttons as phetioReadOnly: true. Can you please review/test?

[arouinfar]

@samreid that seems to have done the trick. I cannot reproduce using the steps outlined in #394 (comment) (using the new zoomControl.enabledProperty)

Since lots of the parts of the ZoomControl are now read-only, I think it makes sense to unfeature them in the overrides.

• *.zoomControl.minusButton.enabledProperty
• *.zoomControl.minusButton.visibleProperty
• *.zoomControl.plusButton.enabledProperty
• *.zoomControl.plusButton.visibleProperty
• *.zoomControl.slider.enabledProperty
• *.zoomControl.slider.visibleProperty

I'll also need to update the client guide entry about the zoom level.

[arouinfar]

@samreid the above commits are ready for cherry-picking.

[samreid]

I also had to cherry pick the scenery commit phetsims/scenery@9bf35fe to make this work.

[samreid]

I cherry picked the sim and common repo changes, will be ready for testing in next RC.

[samreid]

This can be confirmed by checking that e.g., gravityAndOrbits.modelScreen.view.playAreaNode.planetMoonSceneView.zoomControl.enabledProperty is instrumented and works properly to disable the slider and that e.g., zoomControl.minusButton.enabledProperty are phetioFeatured: false and phetioReadOnly: true.