Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add enable/disable to FineCoarseSpinner #461

Closed
jessegreenberg opened this issue Jan 22, 2019 · 6 comments
Closed

Add enable/disable to FineCoarseSpinner #461

jessegreenberg opened this issue Jan 22, 2019 · 6 comments

Comments

@jessegreenberg
Copy link
Contributor

In phetsims/forces-and-motion-basics#274 I replaced the custom ArrowButtons and value readout with new UI component FineCoarseSpinner. But in forces-and-motion-basics, the component needs to be full disabled in some circumstances. When disabled, all ArrowButtons should look disabled, and the NumberDisplay text should have a different fill (probably a greyish color by default).

@pixelzoom would you like me to do this as part of phetsims/forces-and-motion-basics#274 or would you like to do this from #450?

@pixelzoom
Copy link
Contributor

pixelzoom commented Jan 22, 2019

I would handle this ala NumberControl, using opacity. I.e.:

options:

   options = _.extend( {
      enabledProperty: new BooleanProperty( true ), // is this control enabled?
      disabledOpacity: 0.5, // {number} opacity used to make the control look disabled
      ...
    } );

field in constructor:

    this.enabledProperty = options.enabledProperty; // @public
    const enabledObserver = enabled => {
      this.interruptSubtreeInput(); // interrupt interaction
      this.pickable = enabled;
      this.opacity = enabled ? 1.0 : options.disabledOpacity;
    }; 
    this.enabledProperty.link( enabledObserver );

in disposeFineCoarseSpinner:

    this.enabledProperty.unlink( enabledObserver );

setters and getters:

    // @public
    setEnabled: function( enabled ) { this.enabledProperty.set( enabled ); },
    set enabled( value ) { this.setEnabled( value ); },

    // @public
    getEnabled: function() { return this.enabledProperty.get(); },
    get enabled() { return this.getEnabled(); }

pixelzoom added a commit that referenced this issue Jan 23, 2019
@pixelzoom
Copy link
Contributor

Ready for review.

@pixelzoom
Copy link
Contributor

See also the "enabled" checkbox that I added to the demo of FineCoarseSpinner in scenery-phet.

@jessegreenberg
Copy link
Contributor Author

Thanks @pixelzoom, this looks great and is working well in FAMB. Closing,

@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 4, 2019

Reopening. I just noticed that disposeFineCoarseSpinner is unlinking without checking hasListener. If valueProperty or enabledProperty is disposed by the client before FineCoarseSpinner is disposed, unlink will fail:

      this.disposeFineCoarseSpinner = () => {
        valueProperty.unlink( valuePropertyListener );
        if ( ownsEnabledProperty ) {
          this.enabledProperty.dispose();
        }
        else {
          this.enabledProperty.unlink( enabledObserver );
        }
...

pixelzoom added a commit that referenced this issue Mar 4, 2019
@pixelzoom
Copy link
Contributor

Fixed in the above commit. @jessegreenberg feel free to take a peek. But I'm going to (re)close since this change was so trivial.

@pixelzoom pixelzoom reopened this Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants