Add getModifierState definition to TouchEvent by dtapuska · Pull Request #91 · w3c/touch-events (original) (raw)
This repository was archived by the owner on Jul 9, 2024. It is now read-only.
Conversation13 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This matches the text that is used in the UI Events spec.
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 keyArg actually look like in use? i.e. if i had a touch, what would I write to query if shift is also pressed? something like e.targetTouches[0].getModifierState("Shift") ? and would upper/lowercase etc matter?)
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.
This is on the TouchEvent and not TouchPoint so you might say something like:
Oops, right you are.
e.getModifierState("CapsLock") or e.getModifierState("Shift")
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? ...
Add getModifierState definition to TouchEvent. This matches the text Add getModifierState definition to TouchEvent
patrickhlauke pushed a commit that referenced this pull request
This matches the text that is used in the UI Events spec.
patrickhlauke added a commit that referenced this pull request
This matches the text that is used in the UI Events spec.
Co-authored-by: Dave Tapuska dtapuska@chromium.org