search

concord-consortium / lab-interactives-site

Set of interactives built using the Lab Framework from the Concord Consortium
lab.concord.org
Other
14 stars 28 forks source link

Update graph and KE shading #25

Closed rishiloyola closed 9 years ago

ddamelin commented 9 years ago

Hi Rishi,

It would have been good to do this work in the same branch you set up for doing work on that interactive. It looks like we can't set the tick frequency to 0. It is causing lots of errors in the JS console. The KE checkbox doesn't work now. Originally that checkbox was linked to the keShading model property. However, you created a new parameter called keShading which is now overriding the model property. You can set the initial value of this property in the "viewOptions" section of the interactive JSON (or even edit the model JSON itself to change the default to false).

ddamelin commented 9 years ago

I'm waiting to see if Bhavana addresses my comments before merging this pull request. The new dissociation interactive look good in your code.

ddamelin commented 9 years ago

So, this pull request contains old work on the initial breaking molecules interactive, with commits that contain code with errors. This is mixed with commits on a completely separate interactive (the dissociation one) which are good. This is one reason we ask that you keep work on each interactive in it's own branch. It helps with being able to cleanly merge the pull request. If Bhavana doesn't update her pull request, I'll want you to make a new branch for the dissociation commits and issue a pull request just for those commits. Then I can close this one without pulling in the bad code.

ddamelin commented 9 years ago

This pull request seems to have a mixture of things that were tried in the past but have been overwritten by a new pull request. I'm closing this one.