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

Expands the <content> element to remember logical DOM #1017

Merged
merged 1 commit into from
Dec 11, 2014

Conversation

jmesserly
Copy link
Contributor

It's now capable of rerendering if logical DOM changes.

Notes:

  • lightChildren, if present, indicates the logical child views. lightParent will be set for those elements.
  • lightChildren is an array, because that seemed like the "simplest thing that could possibly work". We could expose linked list APIs instead (firstChild, nextSibling, etc) if we wanted.
  • addLightChild/removeLightChild are a convenience, but not recommended for adding large number of items because they automatically call distributeContent(). Another option would be to schedule it and do it lazily.

@jmesserly
Copy link
Contributor Author

oops, apparently using Custom Element constructors in a test doesn't work on Safari (and maybe IE 10, but works 11?). I'll just go back to document.createElement.

It's now capable of rerendering if logical DOM changes.

Notes:

* lightChildren, if present, indicates the logical child views. lightParent will be set for those elements.
* lightChildren is an array, because that seemed like the "simplest thing that could possibly work". We could expose linked list APIs instead (firstChild, nextSibling, etc) if we wanted.
* addLightChild/removeLightChild are a convenience, but not recommended for adding large number of items because they automatically call distributeContent(). Another option would be to schedule it and do it lazily.
@jmesserly
Copy link
Contributor Author

hmmm, I'm also seeing a failure in ready's unit test, maybe IE10 is expected to fail in the 0.8-preview branch? fwiw, IE11 passes and so does Safari when I run locally (on the bot it seems to have issues)

@sorvell
Copy link
Contributor

sorvell commented Dec 11, 2014

LGTM. Great work John. I have some minor comments but I'm going to merge this and then we can iterate. I may tweak some small style stuff and change a couple method names.

sorvell pushed a commit that referenced this pull request Dec 11, 2014
Expands the `<content>` element to remember logical DOM
@sorvell sorvell merged commit 2283b36 into 0.8-preview Dec 11, 2014
@jmesserly
Copy link
Contributor Author

Thank you Steve!

Yes definitely feel free with any style tweaks/refactoring. The code has come a long way since extracting in from SD, but there's probably still more to do. After a long time of restructuring the code, I'm probably blind to some of the oddities at this point :). Will be looking forward to seeing it evolve even more cleaner/nicer.

@kevmoo kevmoo deleted the content_lightchildren branch March 18, 2015 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants