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

Options extension without setting options #59

Closed
jessegreenberg opened this issue Mar 3, 2018 · 5 comments
Closed

Options extension without setting options #59

jessegreenberg opened this issue Mar 3, 2018 · 5 comments
Assignees

Comments

@jessegreenberg
Copy link
Contributor

For #49. I saw this code in DistanceArrowNode.

    _.extend( {
      tandem: Tandem.required
    }, options );

I think this was added for PhET-iO instrumentation. @mbarlow12 @samreid should this be

    options = _.extend( {
      tandem: Tandem.required
    }, options );
@samreid
Copy link
Member

samreid commented Mar 3, 2018

The options was incorrect (because it failed to set options = , but I don't think we need to require a tandem for the DistanceArrowNode. I'm inclined to eliminate that statement altogether.

@samreid samreid removed their assignment Mar 3, 2018
@jessegreenberg
Copy link
Contributor Author

Sounds good to me, thanks @samreid.

@mbarlow12
Copy link
Contributor

It looks like we're setting a tandem for the label text at the moment. @samreid are you thinking that we don't need any tandems for the DistanceArrowNode, or that we simply don't want to require it.

mbarlow12 added a commit that referenced this issue Mar 5, 2018
@samreid
Copy link
Member

samreid commented Mar 5, 2018

I'm thinking that we don't need to require the tandem (though we may supply it anyways).

mbarlow12 added a commit that referenced this issue Mar 5, 2018
mbarlow12 added a commit that referenced this issue Mar 5, 2018
@mbarlow12
Copy link
Contributor

Requirement removed, but we're still passing the tandem through the options.

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

3 participants