-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implementing Element in web-sys #508
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
Implementing Element in web-sys #508
Conversation
@@ -0,0 +1,186 @@ | |||
/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ |
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.
It looks like git says this was all added, was this copied from the available
folder? If so, feel free to delete the version in available
!
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 only added the top section so I could add the rest and comment it out. I had a few problems with large comment sections so took this approach.
crates/webidl/src/util.rs
Outdated
// `DOMString` is not supported yet in other positions. | ||
webidl::ast::TypeKind::DOMString => return None, | ||
webidl::ast::TypeKind::DOMString => { | ||
simple_path_ty(vec![rust_ident("wasm_bindgen"), rust_ident("JsValue")]) |
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'd personally advocate for String
here rather than JsValue
myself, but others may disagree!
Thanks for the PR @jonathanKingston! I'm personally fine adding standardized APIs even if they're brand new (like |
44efd42
to
84de1b2
Compare
@alexcrichton I have done your suggestions I think. |
Looks great! If you want to rebase it should get CI green again, and then I think this is probably good to go! |
84de1b2
to
dac3f38
Compare
Open questions:
toggle_attribute
it's only in Nightly's of Firefox, Canary Chrome, Webkit.toggle_attribute
be controlled by a environment var or build flag?moz_matches_selector
it's non standard and Firefox only.Part of #446 as noted there, many key APIs aren't available still.