phetsims/collision-lab

Branch: ball-vector2-properties

Closed this issue · 2 comments

For #159

I'd also like a response on whether this is necessary to split into x/y Properties at this level. Can't this be on a view component so that it only modifies a specific component? Is there something from phet-io where we gain from having separate x/y? Having a single positionProperty and velocityProperty seems cleaner in general.

I looked over this again and it seems like we can remove having separate x/y properties. The only time these Properties are referenced in other classes is in BallValuesPanelNumberDisplay, which I suppose should display a DerivedProperty of the component of the position/velocity vector2Properties. When the user goes to edit the value via KeypadDialog, it seems like instead of passing in a Property to modify we could directly set one of the components, as you mentioned.

I'll investigate this locally.

It also seems that BallValuesPanelNumberDisplay could see some improvements, particularly the BallValuesPanelColumnTypes enumeration. I'll try converting this to a Rich Enumeration to remove the static methods in BallValuesPanelNumberDisplay.

In the commits above, I was able to remove the x/y component-specific Properties. I added setters for the specific components, instead.

I would like @jonathanolson to look at this branch. The files that were changed are Ball, KeypadDialog, BallValuesPanelNumberDisplay, and BallValuesPanelColumnTypes.

Looks great! Merged to master.