phetsims / plinko-probability

"Plinko Probability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 7 forks source link

Create a common ScreenView element #41

Closed jessegreenberg closed 8 years ago

jessegreenberg commented 8 years ago

PlinkoProbabilityCommonModel collects common elements between Intro and Lab models. Can the same thing be done for PlinkoProbabilityLabView and PlinkoProbabilityIntroView? There seems to be a fair amount of common code between the two screen views, and it might reduce code duplication.

veillette commented 8 years ago

@memo330179 : I think you had proposed a similar idea. The hopper and the board, with the GaltonBoardCanvasNode should belong in the CommonModel. The HistogramNode can be added there as well.

Currently, the eraser and reset all have different listeners but We should try to find a way to merge them so that we can avoid code duplication.

Most of the layout should be done the common code.

Assigning to @memo330179

memo330179 commented 8 years ago

I created a new screenView element and added the common element in the lab and intro views. I also update the JSDoc for the common view.

jessegreenberg commented 8 years ago

Thanks @memo330179, that looks great! In the IntroView and the LabView, creation of GaltonBoardCanvasNode, BallsLayerNode, and VerticalRadioButtonCommonare almost identical. Would it be possible to move any of these to the new common screen view?

memo330179 commented 8 years ago

I've moved BallsLayerNode to the common view. @jessegreenberg Moving GaltonBoardCanvasNode and VerticalRadioButtonCommon is a bit trickier. You are right that they creation of them is almost identical, but they need different tab specifics options.

In particular, GaltonBoardCanvasNode created both the triangular GaltonBoard background and the pegs that reside in it. In the intro tab these pegs are round and so an openingAngle or 0.1 is being passed into it. However, in the lab tab this opening angle has to be the default which is much bigger. These images are created once and then painted as needed.

In order to put the creation of GaltonBoardCanvasNose in the common view, there must be some way to remake the image so that it has the appropriate shape. This way we can create in the common view and then reshape as needed. This can be done by creating a function and then calling it in each tab. To me this seems like ovekill, but it can be done.

The VerticalRadioButtonCommon are in a similar state. The creation is the same, but the images and values that have to be passed are different.

Let me know your thought and sorry for the wall of text.

jessegreenberg commented 8 years ago

That makes sense @memo330179. I agree with your thoughts that it would be more work and less maintainable to do something like

here must be some way to remake the image so that it has the appropriate shape.

Thanks for clarifying and describing to me! Closing.