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

surface.setContent does not do anything when the surface is removed from the DOM in the next render-cycle #673

Closed
IjzerenHein opened this issue Apr 2, 2015 · 3 comments
Labels

Comments

@IjzerenHein
Copy link
Contributor

When you call surface.setContent it sets both this.content and this._contentDirty on the Surface. In the next commit it is then deployed to the DOM. However, when in the next render-cycle, the surface is not committed, but instead recalled, the value set by setContent is overridden by surface.recall. Later, when the surface is commited again and added to the DOM, not the last value set by setContent is shown, but instead the last DOM content is shown. This is a problem in the recall function which should deal with the _contentDirty case in some way.

Original code:

Surface.prototype.recall = function recall(target) {
  var df = document.createDocumentFragment();
  while (target.hasChildNodes()) df.appendChild(target.firstChild);
  this.setContent(df);
};

Suggested fix:

Surface.prototype.recall = function recall(target) {
  if (!this._contentDirty) {
    var df = document.createDocumentFragment();
    while (target.hasChildNodes()) df.appendChild(target.firstChild);
    this.setContent(df);
  }
};

I've tested it in my own environment and works perfectly. I'll create a pull request for it as well.

Cheers,
Hein

@IjzerenHein IjzerenHein changed the title surface.setContent does not work when the surface is removed from the DOM in the next render-cycle surface.setContent does not do anything when the surface is removed from the DOM in the next render-cycle Apr 2, 2015
@jd-carroll
Copy link
Contributor

So I'm a little confused by this... Why would you ever want to prevent a Surface from being recalled? It shouldn't matter if the surface is dirty or not, it is being recalled.

Can you please provide a little more context behind this. I think placing any conditions on the recall will introduce more problems than it could potentially solve. (But maybe I'm wrong)

@IjzerenHein
Copy link
Contributor Author

Hi, I'll provide a test case, so you can see the problem in action. It will probably make sense then.
But I agree completely with your sentiment, introducing unnecessary conditions should be avoided at all cost.

@MylesBorins
Copy link
Contributor

We are no longer actively maintaining this repo and as such we will not be fixing this.

Thanks you for taking the time to discuss the issue, we hope you will bring the same drive and spirit to the famous engine, which can be found at --> http://github.com/famous/engine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants