javafx.scene.input.*Event classes construction

[Martin Sladecek]

Richard, what do you think?

The approved changes were pushed to the repository already, so it would 
be good to fix the issues Pavel found ASAP.

And there's also one more thing that should be changed. In the original 
proposal, there are integer codes in the KeyEvent constructors, but the 
feature request to add them is still open ( 
http://javafx-jira.kenai.com/browse/RT-20448). Since KeyEvents currently 
operates with KeyCode enum only, these constructors should be removed 
and only the constructor with KeyCode should be left in the API.

Thanks,
-Martin


On 11/08/2012 12:42 PM, Pavel Safrata wrote:
> Hello,
> shame on me, I'm really sorry for getting here so late. I've just read 
> through the proposal and I'd like to propose several changes before it 
> starts to be used by apps.
>
>> DragEvent:
>> public DragEvent copyFor(Object source, EventTarget target, Object 
>> gestureSource, Object gestureTarget, Dragboard dragboard)
>> public DragEvent copyFor(Object source, EventTarget target, Object 
>> gestureSource, Object gestureTarget, TransferMode transferMode, 
>> EventType<DragEvent> eventType)
>>
>
> I think we should not complicate the API with those two methods. They 
> have no obvious usage for users and I think all existing internal 
> usages can be removed by a simple refactoring.
>
>> MouseEvent:
>> public MouseEvent(EventType<? extends MouseEvent> eventType, double 
>> x, double y, double screenX, double screenY, MouseButton button,int 
>> clickCount, boolean shiftDown, boolean controlDown, boolean altDown, 
>> boolean metaDown,  boolean primaryButtonDown, boolean 
>> middleButtonDown, boolean secondaryButtonDown, boolean synthesized, 
>> boolean popupTrigger)
>
> I would drop this one, there is a similar one with one more argument: 
> stillSincePress. This argument is not that special, we can use the 
> other method instead. There are places in robot where we're not sure 
> what to put there, but it doesn't really justify for having this 
> duplicated method (it should rather be fixed there).
>
>> TouchEvent:
>> public boolean isDirect() // was impl_ because (according to a 
>> comment in code) there are no indirect events yet, but GestureEvent 
>> already has this public.
>
> I think we should not publish isDirect() and that we should remove the 
> "direct" argument from constructors. Currently touch events are always 
> direct. We are able to produce indirect touch events from TrackPad, 
> but our API specifically states that we don't and won't deliver them. 
> They are just prepared for the possibility that we add custom gesture 
> recognizers in the future that may work with those indirect events, 
> but it is not really certain we'll ever do that, and if we will, it is 
> not certain if it will be allowed to feed custom events to them. So 
> publishing this useless flag would be pretty confusing right now.
>
> Finally, to make TouchEvent constructor usable, we need also 
> constructor for TouchPoint.
>
> Let me once more apologize for the late response.
> Thaks,
> Pavel
>
>>
>>
>> -Martin
>>
>> On 06/15/2012 11:44 PM, Richard Bair wrote:
>>>>>> As for the approach, I think you do the constructors with all 
>>>>>> params (since events are immutable you have no choice really -- 
>>>>>> static factory or constructor and I prefer in this case a 
>>>>>> constructor) + builder (auto generated).
>>>>> And what do you think about impl_copy methods? Personally I think 
>>>>> we should remove them completely and turn them to some internal 
>>>>> utility methods.
>>>> My initial thought was a copy constructor.
>>>>
>>>>> Also, some events are not 100% immutable, as they are modified 
>>>>> during the Scene processing through some impl_methods, like 
>>>>> MouseEvent.impl_setClickParam. We'd either need to make some/all 
>>>>> of the Event fields protected and do this modifications through 
>>>>> subclasses or create some "accessor" in javafx.scene.input package 
>>>>> for calling these methods from javafx.scene package.
>>>> Good question, I guess we can revisit these in context of the other 
>>>> impl_ method removal things we're working on.
>>>>
>>>> Richard
>