More event props improvements #9

cornerman · 2017-11-16T11:40:14Z

No description provided.

raquo · 2017-11-17T08:12:32Z

shared/src/main/scala/com/raquo/domtypes/generic/defs/eventProps/ErrorEventProps.scala

@@ -10,5 +10,5 @@ trait ErrorEventProps[EP[_ <: DomEvent], DomEvent, DomErrorEvent <: DomEvent] {
  /**
    * Script to be run when an error occurs when the file is being loaded
    */
-  lazy val onError: EP[DomErrorEvent] = eventProp("onerror")


Oopsie. Good catch

raquo · 2017-11-17T08:17:51Z

shared/src/main/scala/com/raquo/domtypes/generic/defs/eventProps/MiscellaneousEventProps.scala

+  /**
+    * Fires when the browser window is resized
+    */
+  lazy val onResize: EP[DomUIEvent] = eventProp("resize")


Why do you want to move onResize to this trait? Is it somehow conceptually different from other events in WindowEventProps? Looking at MDN this event seems to be window-specific by nature.

Looking at onLoad and onScroll, I see that they can be applied to elements as well. It would be nice to update the comments on those to not be window-specific and to link to https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onload and https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onscroll

The reason is that I want to separate event handlers that are exclusive to Window, Document, Element and HTMLElement. All three of them implement GlobalEventHandlers. And, for example the event handler onResize is a GlobalEventHandler and therefore not exclusive to Window (eventhough the resize event only exists on Window).

But here they differ:

Window additionally implements WindowEventHandlers and its own event handlers, which more or less corresponds to our WindowEventProps (these properties are not available on Document or Element).

Document also adds some own event handlers. We currently do not have event properties for those.

Element has two event handler of its own. We currently do not have event properties for those.

HTMLElement adds the clipboard and touch events. The clipboard events are defined in our ClipboardEventProps, we do not have touch events.

Maybe, we can also make it more clear somehow, which EventProps traits belong to which EventTarget?

I will update the comments

raquo · 2017-11-17T08:35:20Z

shared/src/main/scala/com/raquo/domtypes/generic/defs/eventProps/MouseEventProps.scala


  /**
    * The click event is raised when the user clicks on an element. The click
    * event will occur after the mousedown and mouseup events.
    *
    * MDN
    */
-  lazy val onClick: EP[DomMouseEvent] = eventProp("click")
+  lazy val onClick: EP[DomMouseEvent with DomHtmlElementTargetEvent] = eventProp("click")


I suspect we can indeed do this, but is there any formal indication that such events will only happen on elements and not e.g. text nodes or SVG elements?

Good catch, i think you are right! MDN actually only says the target type is Element.

Then, I was probably also wrong to make onBlur and onFocus in FormEventProps be an HTMLElementTargetEvent, as they are also only defined for target Element. Right?

Yeah I think we should start out being somewhat conservative in this kind of thing, I'd be happy with dom.Element where MDN says so. If we start relying on "conveniently incorrect" types the eventual withdrawal will be painful... I'm still hoping that the other approach I mentioned in a comment on this issue in outwatch will work better for this.

Sounds right, I will update those event props to having target of type dom.Element. We can always revisit if there is new information.

raquo · 2017-11-19T04:44:50Z

js/src/main/scala/com/raquo/domtypes/jsdom/defs/events/TypedTargetEvent.scala

@@ -6,6 +6,6 @@ import scala.scalajs.js

 /** Represents an event for which we know what exact type the `target` is. */
 @js.native
-trait ElementTargetEvent[+E <: dom.html.Element] extends dom.Event {
+trait TypedTargetEvent[+E <: dom.EventTarget] extends dom.Event {


raquo · 2017-11-19T07:18:03Z

shared/src/main/scala/com/raquo/domtypes/generic/defs/eventProps/MouseEventProps.scala


  /**
    * The click event is raised when the user clicks on an element. The click
    * event will occur after the mousedown and mouseup events.
    *
    * MDN
    */
-  lazy val onClick: EP[DomMouseEvent] = eventProp("click")
+  lazy val onClick: EP[DomMouseEvent with DomHtmlElementTargetEvent] = eventProp("click")


Yeah I think we should start out being somewhat conservative in this kind of thing, I'd be happy with dom.Element where MDN says so. If we start relying on "conveniently incorrect" types the eventual withdrawal will be painful... I'm still hoping that the other approach I mentioned in a comment on this issue in outwatch will work better for this.

raquo · 2017-11-19T07:36:52Z

js/src/main/scala/com/raquo/domtypes/jsdom/defs/package.scala

+    trait EventPropsInWindow[EP[_ <: dom.Event]] extends EventPropsInGlobal[EP] with WindowEventProps[EP] { this: generic.builders.EventPropBuilder[EP, dom.Event] => }
+    trait EventPropsInDocument[EP[_ <: dom.Event]] extends EventPropsInGlobal[EP] with ClipboardEventProps[EP] { this: generic.builders.EventPropBuilder[EP, dom.Event] => }
+    trait EventPropsInElement[EP[_ <: dom.Event]] extends EventPropsInGlobal[EP] { this: generic.builders.EventPropBuilder[EP, dom.Event] => }
+    trait EventPropsInHTMLElement[EP[_ <: dom.Event]] extends EventPropsInGlobal[EP] with ClipboardEventProps[EP] { this: generic.builders.EventPropBuilder[EP, dom.Event] => }


Hmm... I'm not sure if these new traits should reside in SDT. I don't really see a good way to use more than one of those traits at a time in a typical consuming library. I mean, the names would conflict, and you'd need to instantiate way more eventprop objects than needed... right? Maybe I'm missing something?

I'm guessing #13 is related here

Actually, I would like sdt to bring those type as they tell me which event properties are applicable to which underlying EventTarget type.

My plan is to have the following three usages:

trait DomEventProps extends EventPropsInHTMLElement[EmitterBuilder] with EventPropBuilder[EmitterBuilder, dom.Event] for normal usage within tags as it is now, like: input(onChange --> sink).

object WindowEvents extends EventPropsInWindow[Observable] with EventPropBuilder[Observable, dom.Event] for registering window events via an observable: WindowEvents.onStorage.map(ev => ...).

object DocumentEvents extends EventPropsInDocument[Observable] with EventPropBuilder[Observable, dom.Event] for registering document events via an observable: DocumentEvents.onLoad.map(ev => ...).

The latter two can be build by creating an observable from the callback we have when registering an event with (Window|Document).attachEventListener(eventKey, callback)

Oh that's a nice pattern! I myself was using node.events(onClick) pattern because I don't want each node to extend a trait with a hundred vals, even if they're lazy. But for document / window this looks reasonable.

Let's keep all those traits then.

I'm assuming you named the new traits ...InX as opposed to X... because EventPropsInWindow would otherwise conflict with WindowEventProps? Should we maybe rename the original WindowEventProps to WindowOnlyEventProps or something like that? Would look nicer and would also sort of better match Javascript's stuff:

https://developer.mozilla.org/en-US/docs/Web/API/WindowEventHandlers
https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers

Exactly! Sure, we can rename them; I wasn't too happy with this ...In... :)

node.events(onClick)

With regards to outwatch/outwatch#93, that approach would work much better with the currentTarget-event.

cornerman · 2017-11-19T10:40:11Z

shared/src/main/scala/com/raquo/domtypes/generic/defs/eventProps/FormEventProps.scala

@@ -19,7 +22,7 @@ import com.raquo.domtypes.generic.builders.EventPropBuilder
  *            DOM InputEvent https://developer.mozilla.org/en-US/docs/Web/API/InputEvent
  *            Note: This type is not implemented in scala-js-dom
  */
-trait FormEventProps[EP[_ <: DomEvent], DomEvent, DomFocusEvent <: DomEvent, DomInputEvent <: DomEvent, DomHtmlElementTargetEvent <: DomEvent, DomFormElementTargetEvent <: DomEvent, DomInputElementTargetEvent <: DomEvent] { this: EventPropBuilder[EP, DomEvent] =>
+trait FormEventProps[EP[_ <: DomEvent], DomEvent, DomFocusEvent <: DomEvent, DomInputEvent <: DomEvent, DomElementTargetEvent <: DomEvent, DomHtmlElementTargetEvent <: DomEvent, DomFormElementTargetEvent <: DomEvent, DomInputElementTargetEvent <: DomEvent] { this: EventPropBuilder[EP, DomEvent] =>


I am not completely sure about the target types anymore. Why does MDN often say target: Element, but then in the description gives a more concrete type on which elements the event can occur?

Well, conventional JS ideology is largely "let's try to run this and see what we get", so I don't imagine JS devs care all that much about what types are specified. There are also potentially some differences in what is specified in the DOM standard and what is actually implemented in browsers, but I'd think those are minimal by now. I think it's mostly just that JS users largely don't care about minor typing details like this.

For the submit event MDN says "Note that submit is fired only on the form element, not the button or submit input. (Forms are submitted, not buttons.)", and I've verified this behavior manually in a couple browsers, so I'm confident enough to put Form as the type of target for such an event.

- mouse events with element target - move onscroll to MiscellaneousEventProps (has no mouse event) - move onload and onresize to MiscellaneousEventProps (are global event handlers)

…ntTarget

raquo · 2017-11-20T01:25:32Z

@cornerman Thanks for working on this! I've just released v0.4

cornerman · 2017-11-20T17:29:33Z

Great! I think, it should be ready now for outwatch. Thank you so much for all your effort!

cornerman force-pushed the cleanup-events branch from 23467b5 to 74750b4 Compare November 16, 2017 12:02

raquo reviewed Nov 17, 2017

View reviewed changes

cornerman force-pushed the cleanup-events branch 2 times, most recently from 9d0149a to 6d9f28d Compare November 17, 2017 14:18

raquo reviewed Nov 19, 2017

View reviewed changes

cornerman commented Nov 19, 2017

View reviewed changes

cornerman force-pushed the cleanup-events branch from b748f95 to acbdd54 Compare November 19, 2017 11:14

cornerman added 8 commits November 19, 2017 12:14

element target for mouse events and cleanup EventProp separation

a136d17

- mouse events with element target - move onscroll to MiscellaneousEventProps (has no mouse event) - move onload and onresize to MiscellaneousEventProps (are global event handlers)

specify MessageEvent type

55e3f5f

consistently remove on-prefix for event names

7ef814d

update comments for some global event handlers

5dc7f57

add traits for global/window/document/element/htmlelement event props

b3d6954

rename ElementTargetEvent to TypedTargetEvent and make generic on Eve…

02a3594

…ntTarget

rename WindowEventProps -> WindowOnlyEventProps

57300cf

restrict mouse event and onBlur/onFocus to have target type Element

acbdd54

cornerman mentioned this pull request Nov 19, 2017

Use scala-dom-types outwatch/outwatch#62

Merged

raquo approved these changes Nov 20, 2017

View reviewed changes

raquo merged commit 7a7ecb2 into master Nov 20, 2017

raquo deleted the cleanup-events branch November 20, 2017 00:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More event props improvements #9

More event props improvements #9

cornerman commented Nov 16, 2017

raquo Nov 17, 2017

raquo Nov 17, 2017

cornerman Nov 17, 2017 •

edited

Loading

cornerman Nov 17, 2017

raquo Nov 17, 2017

cornerman Nov 17, 2017 •

edited

Loading

raquo Nov 19, 2017

cornerman Nov 19, 2017

raquo Nov 19, 2017

raquo Nov 19, 2017

raquo Nov 19, 2017

cornerman Nov 19, 2017

raquo Nov 19, 2017 •

edited

Loading

cornerman Nov 19, 2017

cornerman Nov 19, 2017

cornerman Nov 19, 2017

raquo Nov 20, 2017

raquo commented Nov 20, 2017

cornerman commented Nov 20, 2017

More event props improvements #9

More event props improvements #9

Conversation

cornerman commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cornerman Nov 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cornerman Nov 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raquo Nov 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raquo commented Nov 20, 2017

cornerman commented Nov 20, 2017

cornerman Nov 17, 2017 •

edited

Loading

cornerman Nov 17, 2017 •

edited

Loading

raquo Nov 19, 2017 •

edited

Loading