SWT has horizontal mouse wheel support since 3.6: https://bugs.eclipse.org/bugs/show_bug.cgi?id=223637. There are SWT.MouseVerticalWheel, SWT.MouseHorizontalWheel and SWT.MouseWheel = MouseVerticalWheel. In JavaScript we can also use event.axis or event.wheelDeltaX to listen horizontal scrolling. Does it make sense to replace EventId.mouseWheel with EventId.mouseVerticalWheel and EventId.mouseHorizontalWheel?
brianWed 27 Apr 2011
This is a good question. We should probably fix the API one way or another. By convention we've been using letter h or v instead of the long spelled out words (hbar/vbar or HAlign/VAlign). Although I sort of think X and Y might make more sense.
I see couple of options:
Deprecate EventId.mouseWheel and add EventId.mouseWheelY and mouseWheelX
Keep EventId.mouseWheel for vertical and add mouseWheelX
Keep one EventId.mouseWheel and add new meta-data to Event such as Orientation
Yuri StrotThu 28 Apr 2011
3) Don't really like the idea to have unnecessary field for most of events
2) Looks too confusing IMHO
1) Probably the best option
Note that in browser when you scroll diagonally you'll get one event with x and y deltas. So in case of additional field it's better to have Size? delta. However this doesn't look useful for other events.
andyThu 28 Apr 2011
I would treat this the same as mouseMoved - so a single mouseWheel callback. Is it plausible to reuse Event.pos to contain the "delta"? Tho you give up the the actual mouse pos then, which I think would come back to bite us.
brianThu 28 Apr 2011
Yeah if the lower level toolkit reports mouse wheel x and y changes in one event, then FWT should too. And if FWT adopts that approach we easily cover cases where X and Y are reported independently without breaking anybody's code.
What I suggest is a breaking change to the onMouseWheel event:
count: stop reporting this field which previously was Y scroll delta
data: start to report this as a Point with both X and Y deltas
Sound like a game plan?
Yuri StrotFri 29 Apr 2011
I think data should works. However it's currently marked as "user data" and in case of wheel event we restrict user to collect another data. Also data is Obj? and we need to point out the right field/type in fandoc and cast it everytime...
Why we have single event type for all kind of events? If I add mouse move handler, why I need "number of characters", "index for list based events" and popup menu? Another issue with such approach is necessity to mark all fields nullable. Even if I listen mouse move I should use not safe Point?
May be it's better to use hierarchical events and just provide following:
class MouseWheelEvent : MouseEvent
{
Point delta
...
}
Why not?
DanielFathFri 29 Apr 2011
Umm... is there a MouseEvent? Maybe Mouse(Wheel)Event should be it's seperate event with special data etc.
PS. TBH I don't get why all events got subsumed under one big Event?
brianFri 29 Apr 2011
I think adding Point delta field to Event is fine, so let's do that.
Why we have single event type for all kind of events?
Because I personally find 20 different event classes just a bunch of bloat in the API and it is often hard to remember what class corresponds to what event. So I really like the way SWT had a simple, single Event class (at least originally I guess).
brianFri 29 Apr 2011
Promoted to ticket #1509 and assigned to brian
DanielFathFri 29 Apr 2011
Dunno, that seemed contrary to good OOP practices. I mean what should I do if I want to create my own event (like a multi-touch event)? Manually change the fwt to add that one event or wait for you guys to implement it? Plus all the data in one class is a bit redundant if I'm not mistaken (there isn't an instance of multiple events occuring at one time, right?).
Yuri StrotFri 29 Apr 2011
I think there is no need to remember all event types while it's associated with handler:
widget.onMouseMove.add |e| { echo("moved to ${e.pos}") }
widget.onMouseWheel.add |e| { echo("scrolled to ${e.delta}") }
widget.onSelect.add |e| { echo("selection changed to ${e.offset}") }
I needn't care about event type here. But in case of structural events IDE can do a good job for me to provide actual code completion.
andyFri 29 Apr 2011
To weigh in here - I have probably written more FWT code than anyone else by a pretty good margin - and this has never been an issue. I did one time extend Event since data wasn't flexible enough - but that was a very sophisticated use case. Overall I think the simplification of a single Event type and a single EventListener type is much more useful in practice than a proper Type hierarchy.
DanielFathFri 29 Apr 2011
I see, its hard to evade years of Java conditioning. But ystrot has a point. How much is current system IDE friendly? Is there a more elegant way to not make too many Event classes but still keep it simple?
Also why did SWT switch from a single Event to a multitude of smaller classes?
Yuri StrotFri 29 Apr 2011
Oh, EventListener... OK, without generics event type hierarchy doesn't look real.
Anyway I think this is not a breaking change which really make sense. The right way for event model would be reactive programming. But for now the new delta field should work.
Just one more question: Why Point, not Size? As I understand Point is like position while Size is something like dimension which is closer for wheel delta. We haven't complete arithmetic for gfx geometry yet, but I think it makes sense at some point. And in this case the difference between Point/Size will be appreciable: Point +/- Size = Point makes sense. So can use: textWidget.topRightPos += wheelEvent.delta.
brianFri 29 Apr 2011
Just one more question: Why Point, not Size?
I think Point makes more sense since its a translation on the x-y coordinate space. Or at least it makes more sense than using width and height to indicate changes.
qualidafialFri 29 Apr 2011
Just want to reiterate a point made earlier:
In SWT, event type is just an int so I can invent my own if needed.
In FWT it's an enum that I can't change. Would be nice if we substituted this with some const mixin so that anybody could define their own event types.
brianSat 30 Apr 2011
In SWT, event type is just an int so I can invent my own if needed.
Good point, maybe we can just add a new value to the enum like custom?
andySat 30 Apr 2011
Good point, maybe we can just add a new value to the enum like custom
I've run across this one or twice - you still want to define what the event is without having to extend Event - so I think you'd have to make id an Int (and switch EventId over to const static fields) to make it worthwhile.
brianMon 2 May 2011
Giving up the enum seems a bit drastic since its so nice to have a printable enum.
I would suggest these changes:
add custom to EventId
add customId to Event
That isn't perfect b/c it treats custom ids as third class citizens. But seems a good compromise to keep clean enum for built-in, commonly used event ids.
Yuri StrotMon 2 May 2011
Sorry guys, but I didn't catch why you need custom events. If user created custom component outside of fwt it can use own events and own listeners. And I think this is a good idea because Event already overflowed. Even if you need to handle fwt event and provide custom one (for example key press -> text change) and want to reuse collected information, it's better to use composition:
class MyEvent
{
Event e
...
}
P.S. Looks like there are a lot of discussions outside of original mouse wheel topic. I think it makes sense to open few separate topics.
qualidafialWed 4 May 2011
Andy, I'm not advocating we use Ints for event IDs like SWT. I would rather introduce some const mixin for event IDs, and retrofit EventId to implement it. That way we don't even need a custom event type in FWT.
andyFri 27 May 2011
Ticket resolved in 1.0.59
Both horizontal and vertical mouse wheel support are now available for both SWT and JavaScript. The wheel delta for each direction is available from the new Event.delta field. Much thanks to ystrot for putting together these patches.
Yuri Strot Wed 27 Apr 2011
SWT has horizontal mouse wheel support since 3.6: https://bugs.eclipse.org/bugs/show_bug.cgi?id=223637. There are
SWT.MouseVerticalWheel
,SWT.MouseHorizontalWheel
andSWT.MouseWheel = MouseVerticalWheel
. In JavaScript we can also useevent.axis
orevent.wheelDeltaX
to listen horizontal scrolling. Does it make sense to replaceEventId.mouseWheel
withEventId.mouseVerticalWheel
andEventId.mouseHorizontalWheel
?brian Wed 27 Apr 2011
This is a good question. We should probably fix the API one way or another. By convention we've been using letter
h
orv
instead of the long spelled out words (hbar/vbar or HAlign/VAlign). Although I sort of think X and Y might make more sense.I see couple of options:
Yuri Strot Thu 28 Apr 2011
Note that in browser when you scroll diagonally you'll get one event with x and y deltas. So in case of additional field it's better to have
Size? delta
. However this doesn't look useful for other events.andy Thu 28 Apr 2011
I would treat this the same as
mouseMoved
- so a singlemouseWheel
callback. Is it plausible to reuseEvent.pos
to contain the "delta"? Tho you give up the the actual mouse pos then, which I think would come back to bite us.brian Thu 28 Apr 2011
Yeah if the lower level toolkit reports mouse wheel x and y changes in one event, then FWT should too. And if FWT adopts that approach we easily cover cases where X and Y are reported independently without breaking anybody's code.
What I suggest is a breaking change to the onMouseWheel event:
count
: stop reporting this field which previously was Y scroll deltadata
: start to report this as a Point with both X and Y deltasSound like a game plan?
Yuri Strot Fri 29 Apr 2011
I think
data
should works. However it's currently marked as "user data" and in case of wheel event we restrict user to collect another data. Alsodata
isObj?
and we need to point out the right field/type in fandoc and cast it everytime...Why we have single event type for all kind of events? If I add mouse move handler, why I need "number of characters", "index for list based events" and popup menu? Another issue with such approach is necessity to mark all fields nullable. Even if I listen mouse move I should use not safe
Point?
May be it's better to use hierarchical events and just provide following:
Why not?
DanielFath Fri 29 Apr 2011
Umm... is there a
MouseEvent
? Maybe Mouse(Wheel)Event should be it's seperate event with special data etc.PS. TBH I don't get why all events got subsumed under one big Event?
brian Fri 29 Apr 2011
I think adding Point delta field to Event is fine, so let's do that.
Because I personally find 20 different event classes just a bunch of bloat in the API and it is often hard to remember what class corresponds to what event. So I really like the way SWT had a simple, single Event class (at least originally I guess).
brian Fri 29 Apr 2011
Promoted to ticket #1509 and assigned to brian
DanielFath Fri 29 Apr 2011
Dunno, that seemed contrary to good OOP practices. I mean what should I do if I want to create my own event (like a multi-touch event)? Manually change the fwt to add that one event or wait for you guys to implement it? Plus all the data in one class is a bit redundant if I'm not mistaken (there isn't an instance of multiple events occuring at one time, right?).
Yuri Strot Fri 29 Apr 2011
I think there is no need to remember all event types while it's associated with handler:
I needn't care about event type here. But in case of structural events IDE can do a good job for me to provide actual code completion.
andy Fri 29 Apr 2011
To weigh in here - I have probably written more FWT code than anyone else by a pretty good margin - and this has never been an issue. I did one time extend Event since
data
wasn't flexible enough - but that was a very sophisticated use case. Overall I think the simplification of a single Event type and a single EventListener type is much more useful in practice than a proper Type hierarchy.DanielFath Fri 29 Apr 2011
I see, its hard to evade years of Java conditioning. But ystrot has a point. How much is current system IDE friendly? Is there a more elegant way to not make too many Event classes but still keep it simple?
Also why did SWT switch from a single Event to a multitude of smaller classes?
Yuri Strot Fri 29 Apr 2011
Oh, EventListener... OK, without generics event type hierarchy doesn't look real.
Anyway I think this is not a breaking change which really make sense. The right way for event model would be reactive programming. But for now the new delta field should work.
Just one more question: Why
Point
, notSize
? As I understandPoint
is like position whileSize
is something like dimension which is closer for wheel delta. We haven't complete arithmetic for gfx geometry yet, but I think it makes sense at some point. And in this case the difference between Point/Size will be appreciable:Point +/- Size = Point
makes sense. So can use:textWidget.topRightPos += wheelEvent.delta
.brian Fri 29 Apr 2011
I think Point makes more sense since its a translation on the x-y coordinate space. Or at least it makes more sense than using width and height to indicate changes.
qualidafial Fri 29 Apr 2011
Just want to reiterate a point made earlier:
In SWT, event type is just an int so I can invent my own if needed.
In FWT it's an enum that I can't change. Would be nice if we substituted this with some const mixin so that anybody could define their own event types.
brian Sat 30 Apr 2011
Good point, maybe we can just add a new value to the enum like
custom
?andy Sat 30 Apr 2011
I've run across this one or twice - you still want to define what the event is without having to extend Event - so I think you'd have to make
id
an Int (and switch EventId over to const static fields) to make it worthwhile.brian Mon 2 May 2011
Giving up the enum seems a bit drastic since its so nice to have a printable enum.
I would suggest these changes:
That isn't perfect b/c it treats custom ids as third class citizens. But seems a good compromise to keep clean enum for built-in, commonly used event ids.
Yuri Strot Mon 2 May 2011
Sorry guys, but I didn't catch why you need custom events. If user created custom component outside of fwt it can use own events and own listeners. And I think this is a good idea because Event already overflowed. Even if you need to handle fwt event and provide custom one (for example key press -> text change) and want to reuse collected information, it's better to use composition:
P.S. Looks like there are a lot of discussions outside of original mouse wheel topic. I think it makes sense to open few separate topics.
qualidafial Wed 4 May 2011
Andy, I'm not advocating we use Ints for event IDs like SWT. I would rather introduce some const mixin for event IDs, and retrofit EventId to implement it. That way we don't even need a custom event type in FWT.
andy Fri 27 May 2011
Ticket resolved in 1.0.59
Both horizontal and vertical mouse wheel support are now available for both SWT and JavaScript. The wheel delta for each direction is available from the new
Event.delta
field. Much thanks to ystrot for putting together these patches.