-
Notifications
You must be signed in to change notification settings - Fork 547
8373038: Interpolator factories should follow method naming convention #1996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
/reviewers 2 |
|
👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@mstr2 has indicated that a compatibility and specification (CSR) request is needed for this pull request. @mstr2 please create a CSR request for issue JDK-8373038 with the correct fix version. This pull request cannot be integrated until the CSR request is approved. |
|
This change makes sense from purely esthetic reasons, though it probably won't be noticed by the app developers as they mostly deal with the interpolators via CSS (and there is no change there, as far as I can tell). |
andy-goryachev-oracle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
|
||
| /** | ||
| * Creates an {@code Interpolator}, which {@link #curve(double) curve()} is | ||
| * Use {@link #ofSpline(double, double, double, double)}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe something like "This poorly named method is deprecated in favor of ..." or words to that extent?
or keep the original description?
| private static final double SWIPE_THRESHOLD = 0.30; | ||
| private static final double TOUCH_THRESHOLD = 15; | ||
| private static final Interpolator interpolator = Interpolator.SPLINE(0.4829, 0.5709, 0.6803, 0.9928); | ||
| private static final Interpolator interpolator = Interpolator.ofSpline(0.4829, 0.5709, 0.6803, 0.9928); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off-topic: shouldn't this interpolator be specified by the stylesheet (i.e. to be able to modify the transition if needed)?
More of a question for @kevinrushforth
The following interpolator factories don't follow the standard method naming convention:
Interpolator.SPLINE(double, double, double, double)Interpolator.TANGENT(Duration, double, Duration, double)Interpolator.TANGENT(Duration, double)Interpolator.STEPS(int, StepPosition)New methods named
ofSpline,ofTangent, andofStepsare added. The existing methods are deprecated (not for removal) in favor of the correctly-named methods. This change is in line with the newofLinearmethod added with #1977.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1996/head:pull/1996$ git checkout pull/1996Update a local copy of the PR:
$ git checkout pull/1996$ git pull https://git.openjdk.org/jfx.git pull/1996/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1996View PR using the GUI difftool:
$ git pr show -t 1996Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1996.diff
Using Webrev
Link to Webrev Comment