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

Update: Add elem to parent before adding its children #1598

Merged
merged 1 commit into from Feb 9, 2017
Merged

Update: Add elem to parent before adding its children #1598

merged 1 commit into from Feb 9, 2017

Conversation

gyandeeps
Copy link
Contributor

@gyandeeps gyandeeps commented Feb 8, 2017

This was done for 0.2.5 but forgotten for 1.0.0 : #1090 (comment)

Here is the previous PR: #1016
Issue comment: #199 (comment)

This should give enough information.

Theme: Add the element to its parent before you add its children to it. this has perf improvements on IE engines (including Edge).

@gyandeeps gyandeeps mentioned this pull request Feb 8, 2017
22 tasks
Copy link
Member

@dead-claudia dead-claudia left a comment

Choose a reason for hiding this comment

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

Single nit, but the rest looks pretty much fine.

@@ -187,7 +187,7 @@ o.spec("oninit", function() {
called = true

o(vnode.dom).equals(undefined)
o(root.childNodes.length).equals(0)
//o(root.childNodes.length).equals(0)
Copy link
Member

Choose a reason for hiding this comment

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

Please correct this test instead of just commenting it out.

@gyandeeps
Copy link
Contributor Author

@isiahmeadows Sorry about that as i actually forgot. Fixed now.

@tivac tivac self-assigned this Feb 8, 2017
@tivac tivac added the Type: Enhancement For any feature request or suggestion that isn't a bug fix label Feb 8, 2017
@tivac
Copy link
Contributor

tivac commented Feb 8, 2017

I'm going to try and carve out some time to investigate the perf (both generally and for this PR). Thank you so much for re-making this change @gyandeeps and I do apologize again that it got lost in the 0.2 to 1.0 shuffle!

@lhorie lhorie merged commit 2688db8 into MithrilJS:next Feb 9, 2017
@gyandeeps gyandeeps deleted the ie-fix branch February 9, 2017 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement For any feature request or suggestion that isn't a bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants