Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented May 12, 2018

This is a follow up on issue #17169 and the pull request #17298

This pull request adds the onChangeStart and onChangeEnd callbacks for CupertinoSlider. These are called when a user starts and ends a change respectively.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@ghost
Copy link
Author

ghost commented May 12, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@gspencergoog
Copy link
Contributor

Thanks @dcaraujo0872! This looks great, but could we bother you to add a test for it too?

@ghost
Copy link
Author

ghost commented May 16, 2018

Sure thing @gspencergoog, on it. :)

widget.onChangeEnd(_lerp(value));
}

// Returns a number between min and max, proportional to value, which must
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation is off here.

///
/// ```dart
/// new CupertinoSlider(
/// value: _duelCommandment.toDouble(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to add the magic comment incantation:

// Examples can assume:
// int _duelCommandment = 1;

at the top of the file, just below the imports (see the top of material/slider.dart for an example) to prevent the dartdoc syntax checker from failing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I gave you only partially correct advice: you need that incantation, but you'll have to change the name of the variable, since that name is already used in the material slider.
Maybe something like _cupertinoSliderValue. More boring, but has the advantage of being unique. :-)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Well my own fault as well because I didn't check other source files. I saw the CI logs a few minutes ago and was wondering why it was failing. Thanks @gspencergoog :)

double _lerp(double value) {
assert(value >= 0.0);
assert(value <= 1.0);
return value * (widget.max - widget.min) + widget.min;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just replace calls to _lerp with this (and eliminate _lerp):

lerpDouble(widget.min, widget.max, value);

The 0-1 enforcement is already being done in the render object.

@gspencergoog
Copy link
Contributor

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

I'll land this as soon as the build dashboard is green again.

@gspencergoog gspencergoog merged commit 8a4db32 into flutter:master May 18, 2018
@ghost ghost deleted the cupertino-slider-onchangestart branch December 29, 2019 15:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants