w3c / touch-events

https://w3c.github.io/touch-events/
26 stars 24 forks source link

TouchEvent constructor is very unusual since it converts one type of object (sequence) to another type (TouchList) #54

Open smaug---- opened 8 years ago

smaug---- commented 8 years ago

I'm not aware of other events having such ctors. Maybe this was already discussed in some other bug, but don't recall now. Anyhow the inconsistency is a bit annoying. And more importantly it isn't defined how one creates a TouchList from sequence.

This is perhaps mostly editorial (tough I'm not happy with the inconsistency).

RByers commented 8 years ago

Yeah I agree this is a little weird. TouchList itself seems like legacy, but presumably we can't change the type of the existing TouchEvent properties. I called this out as something to discuss in the PR and on the list, but there were no concerns raised at the time.

Would you prefer if callers had to explicitly create TouchList instances? Eg:

  new TouchEvent(touches: new TouchList([touch]), changedTouches: new TouchList([touch]), targetTouches: new TouchList([touch]))?

It's probably too late for us to change blink to require that, but we could perhaps permit both styles.

Or maybe we can compatibly change the type of the TouchEvent members to be something other than TouchList? sequence<Touch> is probably out because we probably don't want to require a copy on every read (and so that's illegal anyway - I just raised this issue with FontFaceLoadEvent). Should we ideally be using FrozenArray<Touch> perhaps?

RByers commented 8 years ago

Or do you think we should just add prose defining how TouchListss are created from sequence<Touch>?

smaug---- commented 8 years ago

I think I could live with adding just prose defining how TouchLists are created. Tough the API would look more consistent if TouchList had the ctor and then one would pass those to event ctor. But if you think removing support for sequence in EventDict is too late, I guess we just have to go with the next simplest approach, which is to support only sequence and describe in prose how to create the lists.

RByers commented 8 years ago

I wouldn't want to say it's necessarily too late to remove sequence<Touch> from TouchEventInitDict. We could certainly try (eg. support both, flip our guidance, measure usage of the current approach and remove once usage is low enough). My guess is that this is new enough that we could probably pull it off, but we'd need good justification for doing so.

To me, from a web dev perspective that design seems strictly worse. They don't think about TouchList when using touch events. To a JS web dev, they feed in arrays and get something that they treat like an array out. So forcing them to create a weird TouchList thing as an extra layer around their array seems like a net loss for the developer.

I looked for precedent elsewhere and found one example of something similar in the notifications spec. There they use a sequence<NotificationAction> for input and a FrozenArray<NotificationAction> for output.

It would probably be compatible to replace TouchList with FrozenArray<Touch> in TouchEvent, right? Of course we'd have to keep TouchList around for anyone using createTouchList, but we could deprecate it.

RByers commented 8 years ago

Hmm, ServiceWorkerMessageEvent is another interesting example. It takes a sequence on input and uses an array type on output. Presumably there's some story about WebIDL array types being replaced by FrozenArray<T>?

smaug---- commented 8 years ago

As I said on IRC, I consider about everything in ServiceWorker to be pretty much anti-pattern for the web.

patrickhlauke commented 7 years ago

in light of us wanting to potentially deprecate https://github.com/w3c/touch-events/issues/80 is this still relevant? perhaps for clarity/historical reasons?