Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that init/update are not called when invoking flushToDOM on a non loaded entity #2250

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

dmarcos
Copy link
Member

@dmarcos dmarcos commented Jan 6, 2017

@ngokevin
Copy link
Member

ngokevin commented Jan 6, 2017

Can you explain the remove of the updateCachedAttrValue line and the using attr vs. this.attrValue?

@dmarcos
Copy link
Member Author

dmarcos commented Jan 6, 2017

It's not removed. It's now part of updateProperties. It was redundant before in the constructor since it is done in updateProperties as well

@ngokevin
Copy link
Member

ngokevin commented Jan 6, 2017

I'm confused that we're now passing attr (presumably the attribute name) into updateProperties now when before we were passing an attrValue, and the function seems to expect an attrValue.

@dmarcos
Copy link
Member Author

dmarcos commented Jan 6, 2017

this.attrValue is updated by updateAttrForCache. Now everything happens inside updateProperties

@ngokevin
Copy link
Member

ngokevin commented Jan 6, 2017

OK. It seems like something needs to change then, either the function signature or what we pass into updateProperties:

var Component = module.exports.Component = function (el, attr, id) {
  // ...
  this.updateProperties(attr);  // THIS IS THE ATTRIBUTE NAME.
  // ...
};

Component.prototype = {
  // ...
  /**
   * @param {string} value - HTML attribute value.
   */
  updateProperties: function (value) {
    // ...
    // BUT THIS FUNCTION DOC / VAR NAME EXPECTS AN ATTRIBUTE VALUE.
    if (value !== undefined) { this.updateCachedAttrValue(value); }
  }
}

@dmarcos
Copy link
Member Author

dmarcos commented Jan 6, 2017

The jsdoc for the component constructor was wrong. attr was actually the value of the attribute and not the name. I fixed it.

@ngokevin ngokevin changed the title Make sure that init/update are not called when invoking flushToDOM on… Make sure that init/update are not called when invoking flushToDOM on a non loaded entity Jan 6, 2017
@ngokevin
Copy link
Member

ngokevin commented Jan 6, 2017

r+

@dmarcos dmarcos merged commit 87dbeb3 into aframevr:master Jan 6, 2017
ngokevin pushed a commit to pulilab/aframe that referenced this pull request Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants