FXMisc / Flowless

Efficient VirtualFlow for JavaFX
BSD 2-Clause "Simplified" License
185 stars 38 forks source link

Expose scrolling values & add scrollToPixel capabilities #7

Closed JordanMartinez closed 8 years ago

TomasMikula commented 8 years ago

Looks good to me now. Do you want to propose some method name changes?

JordanMartinez commented 8 years ago

Cool! Thank you for guiding me through all this. The only proposition I have is changing the scrolling method names to be clearer. There are the two methods now scrollX(deltaX) and scrollXToPixel(pixel) that can be misunderstood by a person who just starts using the library. If the first method's name was changed to scrollXBy(deltaX), I think that would be clearer overall. The 2nd method could be renamed to scrollXTo(pixel), but I don't like that for some reason. I'm not sure why.

TomasMikula commented 8 years ago

Makes sense. Though I would keep and deprecate the old scrollX and scrollY methods to give users some transition period. I cannot currently think of better names.

JordanMartinez commented 8 years ago

Ok. I'll also update OrientationHelper's method to correspond to the method naming. scrollHorizontally & scrollVertically will become scrollHorizontallyBy & scrollVerticallyBy, respectively.

TomasMikula commented 8 years ago

Sounds good.

JordanMartinez commented 8 years ago

scrollX & scrollY are used in VirtualFlow's ScrollEventHandler, so I'll be updating those methods as well.

TomasMikula commented 8 years ago

Good.

In OrientationHelper, you don't need to keep any deprecated methods around, because OrientationHelper is not part of public API.

Only keep and deprecate the old methods on VirtualFlow. Also, please use the @deprecated Javadoc tag to explain the deprecation:

/**
 * ...
 * @deprecated use {@link #scrollXBy(double)} instead.
 */
JordanMartinez commented 8 years ago

Still learning how to use those tags properly. I guess that's the next thing I should study. I've updated it.

TomasMikula commented 8 years ago

Thanks, looks good. Merging.

JordanMartinez commented 8 years ago

Sweet! Thanks for guiding me through that.

To release this updated version, what else needs to be done? Now that the scrolling API is exposed, I can't expose it in RichTextFX until the new release, right?

TomasMikula commented 8 years ago

That is basically correct, although in your IDE you can set your local Flowless project as a dependency to your local RichTextFX project if you want to start playing with it.

I will release a new minor version soon.

JordanMartinez commented 8 years ago

Ok, cool. True. I get that I need to change gradle's dependency management to use the local directory, but I haven't yet researched that enough to know how to do that. Even so, I've already written a few lines of code (commented until I can figure out the dependency thing).

TomasMikula commented 8 years ago

At least Eclipse allows me to add projects as dependencies to other projects. No need to update gradle build script. (Of course that will only have effect in Eclipse, not on gradle builds.)

TomasMikula commented 8 years ago

Version 0.4.6 published.

JordanMartinez commented 8 years ago

Now that I think about it, I've already done something like that in my IDE in the past. Hmm... not sure why that didn't come to mind...

Sweet!