-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[rfw] Material slider widget #6610
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
Changes from 12 commits
e7fe375
70c0c4d
a0d3e44
e36ff8f
4174533
77f6303
d502d46
771f595
6f89100
5b48d90
905f8a9
77028ca
bb567e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| ## 1.0.27 | ||
| * Adds `Slider` material widget | ||
|
|
||
| ## 1.0.26 | ||
| * Supports overriding the error widget builder. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ import 'runtime.dart'; | |||||||||||||||||||
| /// * [Material] | ||||||||||||||||||||
| /// * [OutlinedButton] | ||||||||||||||||||||
| /// * [Scaffold] | ||||||||||||||||||||
| /// * [Slider] | ||||||||||||||||||||
| /// * [TextButton] | ||||||||||||||||||||
| /// * [VerticalDivider] | ||||||||||||||||||||
| /// * [OverflowBar] | ||||||||||||||||||||
|
|
@@ -499,6 +500,39 @@ Map<String, LocalWidgetBuilder> get _materialWidgetsDefinitions => <String, Loca | |||||||||||||||||||
| ); | ||||||||||||||||||||
| }, | ||||||||||||||||||||
|
|
||||||||||||||||||||
| 'Slider': (BuildContext context, DataSource source) { | ||||||||||||||||||||
| // not implemented: overlayColor, mouseCursor, semanticFormatterCallback, focusNode, autofocus | ||||||||||||||||||||
| final min = source.v<double>(['min']) ?? 0.0; | ||||||||||||||||||||
| final value = source.v<double>(['value']) ?? min; | ||||||||||||||||||||
| final labelText = source.v<String>(['label']); | ||||||||||||||||||||
| final label = labelText != null ? '$labelText: ${value.toStringAsFixed(2)}' : value.toStringAsFixed(2); | ||||||||||||||||||||
| return Slider( | ||||||||||||||||||||
| value: value, | ||||||||||||||||||||
| secondaryTrackValue: source.v<double>(['secondaryTrackValue']), | ||||||||||||||||||||
| onChanged: source.handler(['onChanged'], | ||||||||||||||||||||
| (HandlerTrigger trigger) => (double value) { | ||||||||||||||||||||
| trigger({'value': value}); | ||||||||||||||||||||
| }), | ||||||||||||||||||||
|
Comment on lines
+512
to
+515
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per the style guide, indentation should never have a line that is less indented than a line that is of higher lexical scope (well the style guide doesn't say it quite that way but that's the intent).
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (same below) |
||||||||||||||||||||
| onChangeStart: source.handler(['onChangeStart'], | ||||||||||||||||||||
| (HandlerTrigger trigger) => (double value) { | ||||||||||||||||||||
| trigger({'value': value}); | ||||||||||||||||||||
| }), | ||||||||||||||||||||
| onChangeEnd: source.handler(['onChangeEnd'], | ||||||||||||||||||||
| (HandlerTrigger trigger) => (double value) { | ||||||||||||||||||||
| trigger({'value': value}); | ||||||||||||||||||||
| }), | ||||||||||||||||||||
| min: min, | ||||||||||||||||||||
| max: source.v<double>(['max']) ?? 1.0, | ||||||||||||||||||||
| divisions: source.v<int>(['divisions']), | ||||||||||||||||||||
| label: label, | ||||||||||||||||||||
| activeColor: ArgumentDecoders.color(source, ['activeColor']), | ||||||||||||||||||||
| inactiveColor: ArgumentDecoders.color(source, ['inactiveColor']), | ||||||||||||||||||||
| secondaryActiveColor: ArgumentDecoders.color(source, ['secondaryActiveColor']), | ||||||||||||||||||||
| thumbColor: ArgumentDecoders.color(source, ['thumbColor']), | ||||||||||||||||||||
| allowedInteraction: ArgumentDecoders.enumValue<SliderInteraction>(SliderInteraction.values, source, ['allowedInteraction']), | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| }, | ||||||||||||||||||||
|
|
||||||||||||||||||||
| 'TextButton': (BuildContext context, DataSource source) { | ||||||||||||||||||||
| // not implemented: buttonStyle, focusNode | ||||||||||||||||||||
| return TextButton( | ||||||||||||||||||||
|
|
||||||||||||||||||||
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.
Should the number of decimals be configurable from the definition as well? I think having a "0 decimals" which calls
roundmight be a very common use case.Uh oh!
There was an error while loading. Please reload this page.
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.
Btw technically speaking, since we cannot really "script" from outside easily having label as string param is not useful since one can only provide hardcoded text that is why I've also added common use case of showing current value with 2 decimal places but I can also add number of decimal places as a param.
Uh oh!
There was an error while loading. Please reload this page.
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.
@uberchilly I agree. I also think the current solution is not great for i18n. This maybe could be re-implemented with something similar to
sprintfor a way for users to specify where in the string they want the number (to enable them to pass'## Birds'so the label is "18 Birds" rather than "Birds: 18") (but I don't think this is a blocker for the feature).I don't think 2 decimal places is more common than 1 decimal place or 0 (or 5), it all depends on the number of "divisions" or the size of the increment. For example, if I were to use this widget with integer values (between 0 and 100 with a step of 10), I think I wouldn't want to see any decimal positions anywhere.
If I was doing between 0 to 1 with 1000 divisions, I'd certainly would want to see increments in the 3rd decimal.
Instead of attempting to figure out what's the ideal number of decimals, I'd just add a value so users can configure it (and that's why I suggested it).
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.
I agree in general, but on the other hand other already supported widgets have bigger limitations imo. In my personal project because of these limitations I had to also add DoubleText and IntText as local widgets because I couldn't do what was suggested here somewhere and that was to add string variant of value that I need to show in Text in data beside regular double for example, and showing number value in Text widget is much more common usecase than using label in slider. So, I am not sure how deep should each widget go in terms of supporting things vs an effort to add scripting easily to prepare values from outside