Skip to content
This repository has been archived by the owner on May 11, 2021. It is now read-only.

timeline improvements #28661

Merged
merged 14 commits into from Sep 1, 2012
Merged

timeline improvements #28661

merged 14 commits into from Sep 1, 2012

Conversation

joelburget
Copy link
Contributor

I ported over the remaining exercises that didn't work with the timeline to the custom answer type. I also included a few little timeline improvements that have been bitrotting for a while.

@joelburget
Copy link
Contributor Author

The ports are still fairly rough, I need to go back and take a second look at them, but I wanted to get this review started.

One problem I know for sure exists is that the orange derivative that shows up when you either get all the points right or ask for that hint in derivative intuition doesn't show up in the timeline. I also noticed on at least one exercise that the guess reverts back to the default when viewing a hint that occured after a guess.

<div class="guess">
(function(){
var guess = [];
$(answers).find('.answer-label').each(function(label) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried this might not work correctly in all browsers. I've always been more comfortable with $("#answers") rather than $(answers)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually gonna ask about this black magic. I used it in a couple other places too but wasn't sure whether it was kosher.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I've never heard of that before. I discovered that answers was a global by accident and just assumed it was Resig magic. Will revert immediately.

@beneater
Copy link
Contributor

Seems possible there are some floating point precision bugs introduced in some of the validators. I was able to convince myself there weren't any, but there's a chance I'm wrong. You may want to monitor the bug reports a little more closely after shipping this.

@beneater
Copy link
Contributor

Clowncopterize!

@joelburget
Copy link
Contributor Author

The derivative intuition derivative marker still doesn't show up in timeline mode but I don't think that's a bugle-stopper. Would love if you could take a look.

@beneater
Copy link
Contributor

Probably because raphael's .animate() is doing a setTimeout(0) or equivalent, which defers until after the timeline takes its snapshot.

This works around it if you can stand the hackyness of it:

diff --git i/utils/derivative-intuition.js w/utils/derivative-intuition.js
index 0af13d9..db97d2f 100644
--- i/utils/derivative-intuition.js
+++ w/utils/derivative-intuition.js
@@ -274,7 +274,7 @@ $.extend(KhanUtil, {
             graph.style({
                 stroke: KhanUtil.DDX_COLOR,
                 strokeWidth: 1,
-                opacity: 0
+                opacity: duration === 0 ? 1 : 0
             }, function() {
                 ddxplot = graph.plot(function(x) {
                     return KhanUtil.ddx(x);

@joelburget
Copy link
Contributor Author

I'm fine with it. There are still problems though. For some reason in timeline some (but not all) of the derivative intuition dots fail to move to the right place. Also, tutorials seem to have caused some rendering bugs but that's not a timeline problem.

@beneater
Copy link
Contributor

beneater commented Sep 1, 2012

this lgtm, then

beneater pushed a commit that referenced this pull request Sep 1, 2012
@beneater beneater merged commit b946c97 into Khan:master Sep 1, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants