-
Notifications
You must be signed in to change notification settings - Fork 407
Zone-binds FileReader#onEventName listeners #174
Conversation
@vicb can you take a look at this? |
|
||
it('should have correct readyState', function (done) { | ||
fileReader.onloadend = function () { | ||
expect(fileReader.readyState).toBe(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.
pls change 2 for FileReader.DONE
(everywhere this is applicable)
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.
Done
var utils = require('../utils'); | ||
|
||
function apply() { | ||
utils.patchClass('FileReader'); |
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.
Quick review, not much time this week:
- What if the patch has been applied in
lib/patch/event-target.js
already ? patchClass('FileReader')
willnew FileReader()
, is that allowed ?
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.
- What if the patch has been applied in lib/patch/event-target.js already ?
The event target patch patches addEventListener
and removeEventListener
, which patchClass
will wrap in addition to taking care of onXXXX
listeners / other function setters that need to be bound. The two patches achieve separate things and the order in which they're applied shouldn't matter.
patchClass('FileReader')
willnew FileReader()
, is that allowed ?
Yes. There's a guard in patchClass
for environments where the class is falsey. I'm not aware of any other scenarios in which you can't new a FileReader
.
LGTM, thanks ! |
closed via #178 |
I took a stab at making the changes to close #137.
No issues as far as I can tell, as
utils.patchClass
does all the leg work. For some reason though, Android 4.3's native browser'sFileReader
doesn't haveaddEventListener
orremoveEventListener,
so there's a work around for that.