Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[rfw] Material slider widget #6610

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

[rfw] Material slider widget #6610

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e7fe375
- Added Material Slider widget
uberchilly Apr 25, 2024
70c0c4d
Updated CHANGELOG.md
uberchilly Apr 25, 2024
a0d3e44
Fixed tests for web
uberchilly Apr 25, 2024
e36ff8f
Fixed formatting and end of line in utils
uberchilly Apr 25, 2024
4174533
Merge branch 'main' into material_slider
uberchilly Apr 25, 2024
77f6303
Merge branch 'main' into material_slider
uberchilly Apr 26, 2024
d502d46
- Fixed material slider handler indentations per guideline.
uberchilly May 7, 2024
771f595
- Fixed unused import
uberchilly May 7, 2024
6f89100
- Fixed formatting for utils
uberchilly May 7, 2024
5b48d90
Merge branch 'main' into material_slider
uberchilly May 7, 2024
905f8a9
Merge branch 'main' into material_slider
uberchilly May 7, 2024
77028ca
Merge branch 'main' into material_slider
uberchilly May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/rfw/CHANGELOG.md
View file
Open in desktop
@@ -1,3 +1,6 @@
## 1.0.27
* Adds `Slider` material widget

## 1.0.26
* Supports overriding the error widget builder.

Expand Down
34 changes: 34 additions & 0 deletions packages/rfw/lib/src/flutter/material_widgets.dart
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@ import 'runtime.dart';
/// * [Material]
/// * [OutlinedButton]
/// * [Scaffold]
/// * [Slider]
/// * [TextButton]
/// * [VerticalDivider]
/// * [OverflowBar]
Expand Down Expand Up @@ -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);
Copy link
Member

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 round might be a very common use case.

Copy link
Author

@uberchilly uberchilly May 8, 2024 •
edited

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.

Copy link
Member

@ditman ditman May 9, 2024 •
edited

Choose a reason for hiding this comment

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

having label as string param is not useful since one can only provide hardcoded text

@uberchilly I agree. I also think the current solution is not great for i18n. This maybe could be re-implemented with something similar to sprintf or 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'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.

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).

Copy link
Author

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

return Slider(
value: value,
secondaryTrackValue: source.v<double>(['secondaryTrackValue']),
onChanged: source.handler(['onChanged'],
(HandlerTrigger trigger) => (double value) {
trigger({'value': value});
}),
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(
Expand Down
2 changes: 1 addition & 1 deletion packages/rfw/pubspec.yaml
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@ name: rfw
description: "Remote Flutter widgets: a library for rendering declarative widget description files at runtime."
repository: https://github.com/flutter/packages/tree/main/packages/rfw
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+rfw%22
version: 1.0.26
version: 1.0.27

environment:
sdk: ^3.2.0
Expand Down
97 changes: 97 additions & 0 deletions packages/rfw/test/material_widgets_test.dart
View file
Open in desktop
Expand Up @@ -586,4 +586,101 @@ void main() {
expect(tester.widget<Material>(find.byType(Material)).clipBehavior,
Clip.antiAlias);
});

testWidgets('Slider properties', (WidgetTester tester) async {
final Runtime runtime = setupRuntime();
final DynamicContent data = DynamicContent();
final List<String> eventLog = <String>[];
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(useMaterial3: false),
home: RemoteWidget(
runtime: runtime,
data: data,
widget: const FullyQualifiedWidgetName(testName, 'root'),
onEvent: (String eventName, DynamicMap eventArguments) {
eventLog.add('$eventName $eventArguments');
},
),
),
);
expect(
tester.takeException().toString(),
contains('Could not find remote widget named'),
);

runtime.update(testName, parseLibraryFile('''
import core;
import material;
widget root = Scaffold(
body: Center(
child: Slider(
onChanged: event 'slider' { },
min: 10.0,
max: 100.0,
divisions: 100,
value: 20.0,
activeColor: 0xFF0000FF,
inactiveColor: 0xFF00FF00,
secondaryActiveColor: 0xFFFF0000,
thumbColor: 0xFF000000,
)));
'''));
await tester.pump();

final Finder sliderFinder = find.byType(Slider);
final Slider slider = tester.widget<Slider>(sliderFinder);
expect(slider.value, 20.0);
expect(slider.min, 10.0);
expect(slider.max, 100.0);
expect(slider.divisions, 100);
expect(slider.activeColor, const Color(0xFF0000FF));
expect(slider.inactiveColor, const Color(0xFF00FF00));
expect(slider.secondaryActiveColor, const Color(0xFFFF0000));
expect(slider.thumbColor, const Color(0xFF000000));

runtime.update(testName, parseLibraryFile('''
import core;
import material;

widget root = Scaffold(
body: Container(
child: Slider(
onChanged: event 'slider' { },
onChangeStart: event 'slider.start' { },
onChangeEnd: event 'slider.end' { },
min: 0.0,
max: 100.0,
divisions: 100,
value: 0.0,
)));
'''));
await tester.pump();

//drag slider
await _slideToValue(tester, sliderFinder, 20.0);
await tester.pumpAndSettle();
expect(eventLog,
contains(kIsWeb ? 'slider {value: 20}' : 'slider {value: 20.0}'));
expect(
eventLog,
contains(
kIsWeb ? 'slider.start {value: 0}' : 'slider.start {value: 0.0}'));
expect(
eventLog,
contains(
kIsWeb ? 'slider.end {value: 20}' : 'slider.end {value: 20.0}'));
});
}

// slide to value for material slider in tests
Future<void> _slideToValue(
WidgetTester widgetTester, Finder slider, double value,
{double paddingOffset = 24.0}) async {
final Offset zeroPoint = widgetTester.getTopLeft(slider) +
Offset(paddingOffset, widgetTester.getSize(slider).height / 2);
final double totalWidth =
widgetTester.getSize(slider).width - (2 * paddingOffset);
final double calculateOffset = value * (totalWidth / 100);
await widgetTester.dragFrom(zeroPoint, Offset(calculateOffset, 0));
}