(feature) Introduce the concept of motion.

[brcolow]

Allow the user to explicitly request a certain type of motion
using the API. Automatically detect when certain types of motion
should be used and if the user has not explicitly requested a type
of motion use the most appropriate one.

For example when moving to a MenuItem the mouse should move vertically
(along the y-axis) first and the horizontally so that adjacent menus
are not activated.

Fixes #305.

[hastebrot]

Note: This changes parts of TestFX's public API.

[hastebrot]

I like the idea of having different types of motions (like euclidean/direct and manhattan/horizontal/vertical) to solve this problem. I think it's great to customize the mouse motion/movement.

However I tend to be against changing the method signatures. We might instead maybe introduce a FxRobot#motionTo() method. Example

moveTo("File")
  .motionTo("Export", VERTICAL_FIRST)
  .motionTo("XML format", HORIZONTAL_FIRST")
  .click()

or

moveTo("File")
  .motion(VERTICAL_FIRST).moveTo("Export")
  .motion(HORIZONTAL_FIRST).moveTo("XML format")
  .motion(DIRECT).click()

[hastebrot]

private void moveMouseStepwiseBetween(
    Point2D sourcePoint, 
    Point2D targetPoint
    Motion motion
) 

We could possibly make this even more customizable by changing the method signature to

private void moveMouseBetween(
    Point2D sourcePoint, 
    Point2D targetPoint
    Interpolator2D interpolator
) 

[hastebrot]

An alternative solution for the menu movement problem is to directly jump to the target position without interpolation.

[brcolow]

Thank you very much for taking the time to review this thoughtfully, it is much appreciated.

Given that TestFX 4 is "alpha", IMO we don't need to be overly concerned about changing the public API. However even in this case the API is not changed in the sense that it will break anyone's existing code. In fact, with these changes things will behave exactly in the same way as before. The only thing that has changed from a public API perspective is more possible arguments (via overloading) to the moveTo and clickOn methods.

I thought about taking an approach similar the one you listed above wherein we would have a separate, chainable, motionTo (or similarly named method) methods for this behavior. But then I thought about it more and realized two things:

1.) Inherent in the concept of clickOn("#something") is that if we are not yet at #something, we will need to move to it. Indeed, this is why the clickOn implementations call moveTo if necessary.
2.) Inherent in the concept of moveTo("#something") is the question of "how"? We have two distinct points in the plane A and B and we need to get from one to the other. There are many possible paths and currently we take the Euclidean path. But the fact is that the motion is inherently tied to the moveTo concept. They are not logically separate and therefore I don't think they should be separated in the public API.

These two points are to show that clickTo is really two distinct parts, one of which is moveTo and that moveTo is inherently related to motion. When not explicitly specified by the user (meaning they end of using one of the default methods that calls the overloaded moveTo with Motion.DEFAULT) it makes sense to do the least surprising thing. This is open to argument but I think we both agree that what you call the Euclidean motion (and this is a good name for it) is the least surprising thing. However it makes sense to make the moveTo method expressive enough to control the motion and that is why I have added the motion argument to the moveTo function - because they are so closely tied together.
This becomes more apparent when you consider your proposal (which is similar to what I thought of before I settled on the argument approach): having motionTo (or motion and moveTo adds ambiguity to the API...it isn't immediately clear what the difference is, when one should be used over the other, etc. The methods currently in FXRobotInterface are all non-ambiguous actions with clear separation.

I am not inflexible in this regard, but did at least want to put my thoughts on the matter out there for more consideration.

In regards to the second point about the moveMouseBetween signature - this is part of the private API and so is much less important to get right. I think that making the method more flexible is indeed a good idea though, and will at least make a note of it with a comment in the code.

[jibbe]

@hastebrot please merge. +1

[brcolow]

I think this is okay to merge. I have thought a lot about this API change and I think using default methods is a good solution to this problem.