A fourth composite pull request #80
Conversation
It's like Browser#after, but works with promises. Allows you to combine your timers with other promise code.
Now, this commit may be quite controversial and if some of you find it too controversial, I would gladly hear from you. I worked on Iframe#content_window, which sometimes returns a weird object. It's a DOM node, but it's "Restricted", you can't edit its properties, but it has a few features that you can use, like a postMessage method. None of the Native::Wrapper or Browser::NativeCachedWrapper could work with such a node, because they checked a DOM node with a Kernel#native? method - which checks for `#@Native.$$class` - this triggers a SecurityError. Therefore if we could catch such an exception... maybe we could treat this kind of object in a special way? Below is a sample code which can create such an object: ``` z = DOM { iframe sandbox: "" } z.append_to($document.body) z.content_window ``` This workaround uses Browser::NativeCachedWrapper. If it detects, that a native object that is about to be wrapped is restricted, it skips all other checks and creates a limited object. It works, because we migrated all DOM to use NativeCachedWrapper. Of course, such objects aren't cached anymore (well, it's not anymore - they never were, because they never worked), but it may be possible to cache them with a different approach, maybe further justifying the location of this workaround...
This patch implements what's needed to support File input from
multiple sources (copy&paste, drag&drop, input[type=file]).
This patch also implements `DataTransferItem` as
`Browser::Event::DataTransfer::Item` which can be useful to handle
drag&drop inside a web application.
I used the following code to test:
```ruby
$document.ready do
DOM { input type: "file", multiple: true; div.fd! { div } }.append_to($document.body)
def show_files(e, method, files=nil)
e.prevent
items = []
if e.respond_to?(:transfer) && files.nil?
files = e.transfer.files
items = e.transfer.items
end
p [e.class, method, files.count, items.count]
z = $document.at_css('#fd>div')
z.replace_with DOM { |b|
b.div {
b.div "method: #{method}"
# This is a demo for DataTransfer#files
files.each do |f|
b.div(style: "border: 3px solid black") {
b.div "filename: #{f.name}"
b.div "mime type: #{f.type}"
b.div "file size: #{f.size}"
b.div "last modified: #{f.last_modified.to_s}"
b.div { b.img src: f.to_url, style: "width: 300px" }
}
end
# This is a demo for DataTransfer#items
items.each do |i|
if i.file?
f = i.to_file
b.div(style: "border: 3px solid blue") {
b.div "filename: #{f.name}"
b.div "mime type: #{f.type}"
b.div "file size: #{f.size}"
b.div "last modified: #{f.last_modified.to_s}"
b.div { b.img src: f.to_url, style: "width: 300px" }
}
elsif i.string?
b.div(style: "border: 3px solid lime") {
b.div "type: #{i.type}"
}
# It's a weird API that can't return ASAP
i.to_string.then { |s| `console.log(#{s})` }
else
b.div(style: "border: 3px solid yellow") {
b.div "unknown kind: #{i.kind}"
b.div "type: #{i.type}"
}
end
end
}
}
end
input_file = $document.at_css('input[type="file"]')
$window .on("paste") { |e| show_files(e, :paste) }
$document .on("dragover") { |e| show_files(e, :dragover) } # Must be e.prevented for this to work
#$document .on("dragenter") { |e| show_files(e, :dragenter) } <-._ ^-._
#$document .on("dragleave") { |e| show_files(e, :dragleave) } <-------------- Those 3 won't give you files
$document .on("drop") { |e| show_files(e, :drop) }
input_file.on("change") { |e| show_files(e, :input, input_file.files) }
end
```
* HTTP::Request now supports file upload via a newly implemented FormData interface. * Unify query string generation. Move everything under a FormData class, deprecate old methods defined in opal/browser/utils. * Support Rack and PHP compatible nested query strings. Fixes opal#33. * Implement DOM::Element::Form. Support FormData generation from forms.
opal/browser/css.rb
Outdated
| # | ||
| # @return [Browser::DOM::Element] the created `<style>` element | ||
| def CSS(*args, &block) | ||
| document = args.pop || $document |
There was a problem hiding this comment.
this, I think, needs to be solved in another way, what if I just pass the css text as first argument… wouldn't it pop the last element from args which happens to also be the first one?
I'd suggest def CSS(text = nil, document = $document, &block), or a check to see if the argument is a String, or —maybe better— use the if block check as it's done in DOM(…).
opal/browser/blob.rb
Outdated
| end | ||
| end | ||
|
|
||
| end |
opal/browser/blob.rb
Outdated
| # receive it. | ||
| def text(&block) | ||
| promise = nil | ||
| if ! block |
There was a problem hiding this comment.
any reason for not using unless block_given? here?
opal/browser/form_data.rb
Outdated
| def <<(tuple) | ||
| key, value, filename = tuple | ||
|
|
||
| if ! filename |
There was a problem hiding this comment.
here and elsewhere, any reason for not using unless?
There was a problem hiding this comment.
A personal preference, I tend to evade unless and until. In those cases unless may be a good choice.
| obj = allocate | ||
| obj.instance_variable_set :@native, native | ||
| return obj | ||
| end |
There was a problem hiding this comment.
About this I would suggest to move all the restricted logic here, and do the check within x-strings, catching just the security error instead of a generic JS::Error. This will make things faster, allow unrelated errors to correctly pop up, and keep this specific code inside NativeCachedWrapperClassMethods until a more general way of dealing with this kind of problem is found.
There was a problem hiding this comment.
I think about expanding the scope of that someday in the future.
It would be nice to have something similar to Native(`[]`). Like Browser.native(`iframe.contentWindow`) which would return an element of type Browser::Window.
This would probably involve using Object.getPrototypeOf and it could allow us to drop a warning if types don't match.
The previous patch missed a call to Kernel#CSS of a form: `CSS(text)` in which case it treated `text` as a Document instance.
This commit also adds a test and moves things around as per @elia's suggestion.
|
Thank you for your review. I do appreciate your input a lot :) Please feel free to merge this PR now or later. |
There was a problem hiding this comment.
Thank you for your review. I do appreciate your input a lot :)
I'm happy to hear that! I do my best to give it enough time to make it valuable in some way 👍👍👍
About using unless consider also this:
When dealing with library code it's always good to have a special regard for performance, in this case using if ! results in one additional method call to (Object#!@) that can become significant the moment this code is used while iterating through many elements.
foo if ! bar; nilbecomes:
if ($truthy(self.$bar()['$!']())) {
self.$foo()};
return nil;
I would particularly like to bring some attention to this commit: 7781a2d. If someone has a better idea about where such a code can be located (or how it can be formed better), I would be glad to know.