-
Notifications
You must be signed in to change notification settings - Fork 24
Add getModifierState definition to TouchEvent #91
Conversation
that is used in the UI Events spec.
Looks conceptually good to me (though, as an aside - and not having properly looked at the UI Events spec - what would the further, per #90, do we need to make sure to add web platform tests for it? |
This is on the TouchEvent and not TouchPoint so you might say something like: e.getModifierState("CapsLock") or e.getModifierState("Shift") Specifically this adds the ability to query the non (alt/ctrl/meta/shift) modifiers that getModifierState allows... which is the useful part. |
Oops, right you are.
Ah, perfect. So yeh, happy to merge when/if we include a web platform test (if that's feasible/easy to test) |
@dtapuska is this mergeable? are there any web platform tests for it now? |
It is mergeable but we don't have an implementation or web platform tests since there wasn't any traction on it. @NavidZ can you get someone to complete this? |
@dtapuska you mean the Chrome implementation or the test or both? Has there been any recent tractions and requests to get this in or is it just to align touch events with ui events? |
The Chrome implementation has neither. But it does have accessors for alt,ctrl,shift,meta. So really it should have the API on the IDL for modifierState to query them all. This is just a consistency thing. The data should be there already for the Chrome implementation, the IDL just needs to change and web platform tests need to be written. |
I think it should for consistency. However I don't think any browser implements it yet. @flackr @mustaqahmed ? |
@flackr @mustaqahmed any idea? (sorry for harking on here, but keen to clear the deck of any long-standing PRs etc) |
I agree with the consistency comment, but also share the cross-browser support concern there! So LGTM provided we have either a supporting comment from another browser or a WPT. |
I think merging this and adding a wpt is reasonable. As @dtapuska pointed out we already have all of the information so adding support should be simple and non-controversial. |
I'll go ahead and merge, and make a new issue about getting a wpt. Now the million dollar question...who's got the knowledge/time to make a wpt? ... |
This matches the text that is used in the UI Events spec.
This matches the text that is used in the UI Events spec.