-
Notifications
You must be signed in to change notification settings - Fork 160
Conversation
There was one actual API change. We had to convert the |
@@ -48,7 +48,7 @@ | |||
var i = this.ids.indexOf(inId); | |||
return this.pointers[i]; | |||
}, | |||
get size() { | |||
size: function() { |
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.
This breaks touch.js
use of pointermap.size
, and violates the Map API.
I think we need to have a more generic function that wraps this.
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.
The issue is in IE 6-8, just defining the getter throws a syntax error so it can't be used. We could fix the issue in touch.js
by using pointermap.size()
but that doesn't fix the Map API violation. Any other thoughts or suggestions?
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 think the best course of action would be to slightly alter PointerMap to extend a Map, and add a non-getter pointers
function.
In this case, I never write the size
getter where Map is undefined, and use Map.size where it is.
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.
@azakus Do you want to land that update now and then we can test with our old IE layer and update the PR?
I'll modify the PointerMap api and somehow you guys can rebaseline with it. Should I send a PR to your PR? |
You can just update in master and we'll rebase on top of that and force push to update the PR. |
With c1d7830, drop changes to pointermap.js and the tests and replace |
Replace PointerMap.size getter with a size method. Quote properties named delete.
This has been rebased. |
LGTM, thanks for all the hard work! |
These are the changes necessary to be able to get PointerEvents working in old IE. The first commit removes anything that will break in old IE. The second commit provides hooks to implement the IE-specific logic.