RusticiSoftware / TinCanJS

JavaScript library for the Experience API (Tin Can API)
http://rusticisoftware.github.io/TinCanJS/
Apache License 2.0
207 stars 115 forks source link

fix error when interaction components not used in chrome #37

Closed garemoko closed 11 years ago

garemoko commented 11 years ago

Just a minor fix.

I'm not sure what's going on with the line lengths - they look the same on my computer but seem to have changed in the diff. Any ideas?

bscSCORM commented 11 years ago

@garemoko not sure why that is either, but at least it's only the one changed line.

@brianjmiller Please merge if this change looks good to you.

brianjmiller commented 11 years ago

@garemoko the line length is because that line's indent is now tabs rather than spaces.

Overall I'm not sure what this change fixes, the interaction component props in that loop are being checked on the local object which always has those properties defined and set to null, assuming you are using the constructor for the object. Are you getting an undefined error on a prop?

garemoko commented 11 years ago

Hi Brian, I can't remember the details of the error now. I know my TinCaptivate project upgraded fine but when I upgraded TinStatement to the 1.0.0 version of the library it stopped working. This fixed it.

I may not be using the constructor. Sometimes it's easier to create the object and add the properties on after. I guess that's the wrong way to do it?

"Brian J. Miller" notifications@github.com wrote:

@garemoko the line length is because that line's indent is now tabs rather than spaces.

Overall I'm not sure what this change fixes, the interaction component props in that loop are being checked on the local object which always has those properties defined and set to null, assuming you are using the constructor for the object. Are you getting an undefined error on a prop?

— Reply to this email directly or view it on GitHub.

brianjmiller commented 11 years ago

@garemoko How are you creating the object to add the properties?

garemoko commented 11 years ago

Actually it looks like in TinStatement I'm doing this:

https://github.com/garemoko/tinStatement/blob/master/TinStatement.js#L170

But elsewhere I do things like this:

https://github.com/garemoko/tinStatement/blob/master/TinFormFunctions.js#L353

Andrew

"Brian J. Miller" notifications@github.com wrote:

@garemoko How are you creating the object to add the properties?

— Reply to this email directly or view it on GitHub.

brianjmiller commented 11 years ago

@garemoko Those are effectively the same (assuming 'objectType' is something like "ActivityDefinition") and both are using the constructor appropriately so the properties should be available on the object already and assigned to be null if you aren't providing values otherwise. Can you give me a test reproduction? I'm inclined to close this but I want to make 100% sure that something internal in the lib isn't causing the issue.

garemoko commented 11 years ago

Hi Brian, Probably the best way to see the error is to download TinStatement and replace my tincanjs folder with the unedited version and try it in chrome.

I'll see if I can make some time to replicate the error myself too, just to make sure I'm telling you the right repository. It should run locally in chrome.

Andrew

"Brian J. Miller" notifications@github.com wrote:

@garemoko Those are effectively the same (assuming 'objectType' is something like "ActivityDefinition") and both are using the constructor appropriately so the properties should be available on the object already and assigned to be null if you aren't providing values otherwise. Can you give me a test reproduction? I'm inclined to close this but I want to make 100% sure that something internal in the lib isn't causing the issue.

— Reply to this email directly or view it on GitHub.

garemoko commented 11 years ago

Found the error in my code