Rate Reminder implementation by pritam-bs · Pull Request #64 · nstack-io/flutter-sdk

pritam-bs · 2024-05-13T16:03:55Z

Both default and custom configuration strategies
Documentation for Rate Reminder usage
Example for Rate Reminder
Some clean-up in Message and Version-Control
Fix the template for enabling the test mode

- Both default and custom configuration strategies - Documentation for Rate Reminder usage - Example for Rate Reminder - Some clean-up in Message and Version-Control - Fix the template for enabling the test mode

lib/src/models/rate_reminder.dart

hassan-saleh-ml · 2024-05-14T10:18:54Z

lib/src/repository/nstack_repository.dart

+      final rateReminderResponse = await http.get(
+        Uri.parse(
+          '$_baseUrl/notify/rate_reminder_v2?guid=${appInfoData.guid}',
+        ),
+        headers: mutableHeaders,
+      );


I remember @BucekJiri said that we don't have to fetch anything from the API except for app open, @pritam-bs can't we get the info here from app open response since we already fetched this information?

Open app API does not provide the rate reminder information.
https://nstack-io.github.io/docs/docs/features/rate-reminder.html?sidebarScroll=0

hassan-saleh-ml · 2024-05-14T10:20:07Z

lib/src/repository/nstack_repository.dart

+
+      final result = json.decode(rateReminderResponse.body);
+
+      if (rateReminderResponse.statusCode == 445) {


This is a very specific error code, are there any error codes we should handle?

Shouldn't all 4xx status codes be handled?

This log message mainly informs the developer how many points must be triggered before the app can show the rate reminder alert.
the other errors will be logged in the catch block.

@pritam-bs can we then add a comment right in the code for future generations?

hassan-saleh-ml · 2024-05-14T10:22:35Z

lib/src/sdk/nstack_features.dart

 )?;

+typedef OnRateReminder = void Function(
+  NStackRateReminder? rateReminderInfo,


I don't think this should be nullable, when onRateReminder we should expect the info to never be null.

Same for NStackRateReminderAnswer? rateReminderAnswer,

Also the method in the type def itself should not be nullable, for example:

typedef OnRateReminderAnswered = void Function( NStackRateReminderAnswer rateReminderAnswer, );

Then in the class you can make it nullable:

final OnRateReminder? onRateReminder; final OnRateReminderAnswered? onRateReminderAnswered;

The NStackRateReminder is nullable for the onRateReminder callback when the points are less than the certain threshold set in the console, and the devs can make a conditional check if needed.
But NStackRateReminderAnswer should not be nullable; I will fix that.

@pritam-bs if we recieve null from BE should we display the reminder? If we should not display it then we should not call the callback in the first place and it can be nun nullable

The intention was to enable developers to check the NStackRateReminder value and decide whether to do anything when it is null. Anyway, change it to not nullable.

- Remane Localization for Rate Reminder - method in the type def made not nullable

nivisi · 2024-05-16T12:39:11Z

lib/src/sdk/nstack_features.dart

+typedef OnRateReminder = void Function(
+  NStackRateReminder? rateReminderInfo,
+);
+
+typedef OnRateReminderAnswered = void Function(
+  NStackRateReminderAnswer rateReminderAnswer,
+);


On another topic for typedefs. There is no written naming convention available (based on a quick research I just did), but usually these callbacks must be named differently. Here are a few examples from the Flutter repo (see below).

I recommend to follow these rules:

Suggested change

typedef OnRateReminder = void Function(

NStackRateReminder? rateReminderInfo,

);

typedef OnRateReminderAnswered = void Function(

NStackRateReminderAnswer rateReminderAnswer,

);

typedef RateReminderCallback = void Function(

NStackRateReminder data,

);

typedef RateReminderAnswerCallback = void Function(

NStackRateReminderAnswer answer,

);

Examples

(see more here)

typedef ValueGetter<T> = T Function(); /// Signature for callbacks that filter an iterable. typedef IterableFilter<T> = Iterable<T> Function(Iterable<T> input); /// Signature of callbacks that have no arguments and return no data, but that /// return a [Future] to indicate when their work is complete. /// /// See also: /// /// * [VoidCallback], a synchronous version of this signature. /// * [AsyncValueGetter], a signature for asynchronous getters. /// * [AsyncValueSetter], a signature for asynchronous setters. typedef AsyncCallback = Future<void> Function(); /// Signature for callbacks that report that a value has been set and return a /// [Future] that completes when the value has been saved. /// /// See also: /// /// * [ValueSetter], a synchronous version of this signature. /// * [AsyncValueGetter], the getter equivalent of this signature. typedef AsyncValueSetter<T> = Future<void> Function(T value);

typedef GestureDragEndCallback = void Function(DragEndDetails details); /// Signature for when the pointer that previously triggered a /// [GestureDragDownCallback] did not complete. /// /// Used by [DragGestureRecognizer.onCancel]. typedef GestureDragCancelCallback = void Function(); /// Signature for a function that builds a [VelocityTracker]. /// /// Used by [DragGestureRecognizer.velocityTrackerBuilder]. typedef GestureVelocityTrackerBuilder = VelocityTracker Function(PointerEvent event);

nivisi · 2024-05-16T12:41:23Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+          'If onRateReminder is null, then onAnswered must not be null.',
+        );
+
+  final Widget? child;


Put child widgets last in the property list. Again, this is a convention that majority of Flutter widgets follow (and we do follow it as well). Same for the constructor parameter list

There's a related rule for this: https://dart.dev/tools/linter-rules/sort_child_properties_last

nivisi · 2024-05-16T12:41:49Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+    this.onAnswered,
+  }) : assert(
+          onRateReminder != null || onAnswered != null,
+          'If onRateReminder is null, then onAnswered must not be null.',


Suggested change

'If onRateReminder is null, then onAnswered must not be null.',

'Either onRateReminder or onAnswered must be provided.',

nivisi · 2024-05-16T12:42:17Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+  final void Function(NStackRateReminder?)? onRateReminder;
+  final void Function(NStackRateReminderAnswer)? onAnswered;


Use our typedefs here

nivisi · 2024-05-16T12:43:25Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+    WidgetsBinding.instance.addPostFrameCallback(
+      (timeStamp) async {
+        final rateReminderInfo =
+            await context.nstack.rateReminders.getRateReminderInfo(
+          defaultLocale: Localizations.localeOf(context),
+        );
+        _onRateReminder(rateReminderInfo);
+      },
+    );


We don't need a post frame callback in didChangeDependencies, it's only required in initState.

Although are we sure this needs to be done in didChangeDependencies? It might get executed quite frequently here

nivisi · 2024-05-16T12:44:27Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+      rateReminder.localization?.laterBtn ?? _laterButtonTitleFallback,
+    );
+
+    Future<void> buttonAction(


Let's move this out of the build method

nivisi · 2024-05-16T12:44:59Z

lib/templates/nstack_template.txt

+    bool? testMode,
  }) : super(
          key: key,
          child: child,
          platformOverride: platformOverride,
          onComplete: onComplete,
          config: config,
+          testMode: testMode ?? false,


Suggested change

bool? testMode,

}) : super(

key: key,

child: child,

platformOverride: platformOverride,

onComplete: onComplete,

config: config,

testMode: testMode ?? false,

bool testMode = false,

}) : super(

key: key,

child: child,

platformOverride: platformOverride,

onComplete: onComplete,

config: config,

testMode: testMode,

nivisi · 2024-05-16T12:45:24Z

lib/src/sdk/widgets/nstack_rate_reminder_widget.dart

+  State<StatefulWidget> createState() => _NStackRateReminderWidgetSate();
+}
+
+class _NStackRateReminderWidgetSate extends State<NStackRateReminderWidget> {


Suggested change

State<StatefulWidget> createState() => _NStackRateReminderWidgetSate();

}

class _NStackRateReminderWidgetSate extends State<NStackRateReminderWidget> {

State<StatefulWidget> createState() => _NStackRateReminderWidgetState();

}

class _NStackRateReminderWidgetState extends State<NStackRateReminderWidget> {

- Rate Reminder implementation

99f91c0

- Both default and custom configuration strategies - Documentation for Rate Reminder usage - Example for Rate Reminder - Some clean-up in Message and Version-Control - Fix the template for enabling the test mode

pritam-bs requested review from BucekJiri, hassan-saleh-ml and nivisi May 13, 2024 16:10

hassan-saleh-ml requested changes May 14, 2024

View reviewed changes

- snake case to camel case

65f75b1

- Remane Localization for Rate Reminder - method in the type def made not nullable

pritam-bs requested a review from hassan-saleh-ml May 14, 2024 12:41

nivisi suggested changes May 16, 2024

View reviewed changes

- Clean up to follow Flutter convention

1f53818

pritam-bs requested a review from nivisi May 17, 2024 10:41

- sort child properties last

a62d6b6


		final result = json.decode(rateReminderResponse.body);

		if (rateReminderResponse.statusCode == 445) {

	'If onRateReminder is null, then onAnswered must not be null.',
	'Either onRateReminder or onAnswered must be provided.',

		final void Function(NStackRateReminder?)? onRateReminder;
		final void Function(NStackRateReminderAnswer)? onAnswered;

Conversation

pritam-bs commented May 13, 2024

Uh oh!

Uh oh!

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!

nivisi May 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Examples

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nivisi May 16, 2024 •

edited

Loading