Initial Implementation for Rating Widget and Change EmptyPage to InitialPage on Samples #503

a-man-called-q · 2025-04-28T16:54:00Z

Describe the changes

Initial Implementation for Rating Page

Checklist

I have read the CONTRIBUTING.md.
I have included the relevant unit/golden tests.
I have included the relevant samples.
I have updated the documentation accordingly.
I have added the required flutters_hook for widget controllers.
I have updated the CHANGELOG.md if necessary.

… sample and change EmptyPage to InitialPage showing all the routes available.

Copilot

Copilot reviewed 4 out of 11 changed files in this pull request and generated no comments.

Files not reviewed (7)

forui/lib/forui.dart: Language not supported
forui/lib/src/widgets/rating/rating.dart: Language not supported
forui/lib/src/widgets/rating/rating_content.dart: Language not supported
forui/lib/widgets/rating.dart: Language not supported
forui/test/src/widgets/rating/rating_golden_test.dart: Language not supported
samples/lib/main.dart: Language not supported
samples/lib/widgets/rating.dart: Language not supported

a-man-called-q · 2025-04-28T16:56:59Z

This should complete #328 with additional caveats :

the implementation is quite complex since there's a hover handler for displaying the rating and supports half value ( need a decent refactor )
I don't know if the implementation fits the way of Forui should be.

auto-maid · 2025-04-28T16:58:33Z

Documentation Preview

Latest Commit SHA	`3aac7c1`
Preview URL	https://b9aec55e.forui-docs-dev.pages.dev/docs

You're seeing this because the docs/samples were updated.

Pante

Thanks for the PR!

Just a heads up, we probably won't be able to fully review this until next week as we're got quite a packed week ahead of us.

Pante · 2025-04-29T01:47:41Z

forui/lib/src/widgets/rating/rating.dart

+  }
+
+  @override
+  Widget build(BuildContext context) {


I think you can use FTappable to manage the tapping/hovering logic.

Pante · 2025-04-29T01:48:16Z

forui/lib/src/widgets/rating/rating.dart

+  final int count;
+
+  /// Called when the rating value changes.
+  final ValueChanged<double>? onStateChanged;


This should be onChange, onStateChange is meant for WidgetState changes.

Pante · 2025-04-29T01:49:01Z

forui/lib/src/widgets/rating/rating.dart

+  final FocusNode? focusNode;
+
+  /// {@macro forui.foundation.doc_templates.onFocusChange}
+  final ValueChanged<bool>? onFocusChange;


We should probably add a onHoverChange, I think you can use FButton as a reference.

Pante · 2025-04-29T01:49:59Z

forui/lib/src/widgets/rating/rating.dart

+  /// The spacing between rating icons.
+  final double spacing;


This should be in the style.

Pante · 2025-04-29T01:51:20Z

forui/lib/src/widgets/rating/rating.dart

+    this.filledIcon = const Icon(FIcons.star, color: Color(0xFFFFD700)), // Gold
+    this.emptyIcon = const Icon(FIcons.starOff, color: Color(0xFFBDBDBD)), // Gray


We should move the color to inside the style. Generally for icons, we wrap them in a IconTheme inside the build method, e.g. IconTheme(style.iconThemeData, child: filledIcon).

Pante · 2025-04-29T01:51:46Z

forui/lib/src/widgets/rating/rating.dart

+      ..add(IntProperty('count', count))
+      ..add(FlagProperty('enabled', value: enabled, ifTrue: 'enabled'))
+      ..add(DiagnosticsProperty('style', style))
+      ..add(ObjectFlagProperty<ValueChanged<double>?>.has('onStateChanged', onStateChanged))


Suggested change

..add(ObjectFlagProperty<ValueChanged<double>?>.has('onStateChanged', onStateChanged))

..add(ObjectFlagProperty.has('onStateChanged', onStateChanged))

Generally we don't specify the generic type for diagnostic properties.

Pante · 2025-04-29T01:52:30Z

forui/lib/src/widgets/rating/rating.dart

+      ..add(DiagnosticsProperty('style', style))
+      ..add(ObjectFlagProperty<ValueChanged<double>?>.has('onStateChanged', onStateChanged))
+      ..add(StringProperty('semanticsLabel', semanticsLabel))
+      ..add(DiagnosticsProperty<bool>('autofocus', autofocus))


Suggested change

..add(DiagnosticsProperty<bool>('autofocus', autofocus))

..add(FlagProperty('autofocus', value: autofocus, ifTrue: 'autofocus'))

Pante · 2025-04-29T02:00:17Z

forui/lib/src/widgets/rating/rating.dart

+  double _calculateRating(double dx, double totalWidth) {
+    if (totalWidth <= 0) {
+      return 0;
+    }
+
+    final itemWidth = totalWidth / widget.count;
+    final x = dx.clamp(0.0, totalWidth);
+    final index = x ~/ itemWidth;
+    final localPosition = x - (index * itemWidth);
+
+    double rating = index + 1.0;
+    if (widget.halfFilledIcon != null && localPosition < itemWidth / 2) {
+      rating -= 0.5;
+    }
+
+    return rating.clamp(0.0, widget.count.toDouble());
+  }
+
+  void _handleHover(PointerHoverEvent event) {
+    final box = context.findRenderObject() as RenderBox?;
+    if (box == null) {
+      return;
+    }
+
+    final localPosition = box.globalToLocal(event.position);
+    setState(() => _hoverValue = _calculateRating(localPosition.dx, box.size.width));
+  }
+
+  void _handleTap(TapUpDetails details) {
+    final box = context.findRenderObject()! as RenderBox;
+    final localPosition = box.globalToLocal(details.globalPosition);
+    final rating = _calculateRating(localPosition.dx, box.size.width);
+
+    widget.onStateChanged?.call(rating);
+    setState(() => _isHovering = false);
+  }


Generally, I think we should try to avoid using retrieving the renderbox like this. Perhaps, you can use the PointerEnterEvent passed into MouseRegion.onEnter to calculate the size?

I think there's something similar for tap gestures too.

Pante · 2025-04-29T02:00:52Z

forui/lib/widgets/rating.dart

+/// A line calendar displays dates in a single horizontal, scrollable line.
+///
+/// See https://forui.dev/docs/data/line-calendar for working examples.


docs need to be updated.

Pante · 2025-04-29T02:03:04Z

forui/lib/src/widgets/rating/rating.dart

+final class FRatingStyle with Diagnosticable, _$FRatingStyleFunctions {
+  /// The color of the rating icons.
+  @override
+  final Color? color;
+
+  /// The size of the rating icons.
+  @override
+  final double? size;
+
+  /// Creates a [FRatingStyle].
+  const FRatingStyle({this.color, this.size});
+}


I think these properties can be merged into a IconThemeData. IconThemeData can then be wrapped inside a FWidgetStateMap to account for the various states, i.e. selected & unselected.

I think you can reference FTileContentStyle.

a-man-called-q · 2025-04-29T05:02:38Z

It's ok. I'll be busy till weekend anyways.

a-man-called-q · 2025-04-29T07:21:08Z

Btw.. I suggest that this should be delayed until 0.12. Seeing how @Pante refactored the nested header. It seems like I need to be guided more.

Not a good programmer here lol, plus I need the header update soon for my future app.

Pante · 2025-04-29T11:36:53Z

No worries! On another note, we're planning to release 0.11.0 on 1st May.

Add FRating widget and related tests for customizable ratings, update…

24cc3c1

… sample and change EmptyPage to InitialPage showing all the routes available.

Copilot AI review requested due to automatic review settings April 28, 2025 16:54

Copilot AI reviewed Apr 28, 2025

View reviewed changes

a-man-called-q temporarily deployed to docs-development April 28, 2025 16:54 — with GitHub Actions Inactive

Commit from GitHub Actions (Forui Samples Presubmit)

3aac7c1

auto-maid bot temporarily deployed to docs-development April 28, 2025 16:56 Inactive

Pante requested a review from kawaijoe April 29, 2025 01:44

Pante requested changes Apr 29, 2025

View reviewed changes

		this.filledIcon = const Icon(FIcons.star, color: Color(0xFFFFD700)), // Gold
		this.emptyIcon = const Icon(FIcons.starOff, color: Color(0xFFBDBDBD)), // Gray

	..add(ObjectFlagProperty<ValueChanged<double>?>.has('onStateChanged', onStateChanged))
	..add(ObjectFlagProperty.has('onStateChanged', onStateChanged))

	..add(DiagnosticsProperty<bool>('autofocus', autofocus))
	..add(FlagProperty('autofocus', value: autofocus, ifTrue: 'autofocus'))

Initial Implementation for Rating Widget and Change EmptyPage to InitialPage on Samples #503

Are you sure you want to change the base?

Initial Implementation for Rating Widget and Change EmptyPage to InitialPage on Samples #503

Uh oh!

Conversation

a-man-called-q commented Apr 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

a-man-called-q commented Apr 28, 2025

Uh oh!

auto-maid bot commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation Preview

Uh oh!

Pante left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-man-called-q commented Apr 29, 2025

Uh oh!

a-man-called-q commented Apr 29, 2025

Uh oh!

Pante commented Apr 29, 2025

Uh oh!

Uh oh!

auto-maid bot commented Apr 28, 2025 •

edited

Loading