-
Notifications
You must be signed in to change notification settings - Fork 116
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
Next Dom document implementation #118
Conversation
24494ce
to
67b354a
Compare
src/dom/document.zig
Outdated
@@ -226,6 +261,44 @@ pub fn testExecFn( | |||
}; | |||
try checkCases(js_env, &createDocumentFragment); | |||
|
|||
var createTextNode = [_]Case{ | |||
.{ .src = "document.createTextNode('foo')", .ex = "[object Text]" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it will be better to use the nodeName
getter for the equality (here #text
) rater than the string representation.
I see that the console of Chrome and Firefox returns different values here: just the value of the text itself (ie. foo
) for Chrome and the concatenation of the nodeName and the text for Firefox (#text "foo"
).
But when you call the toString
function you have [object Text]
so I guess it's only the console of the dev tools who change the result printed to help web developers with a more explicit return.
So I guess the object string representation [object Text]
is OK for equality at the JS VM level, but the nodeName could be better (but more verbose). Sincerly I'm not sure. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't really happy with this string representation, I'm ok to use nodeName instead 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also do it in 1 line document.createTextNode('foo').nodeName
but I'm OK with this version
for document unit test
Relates to #18
fix #26