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.

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

Implement Messages feature #61

Merged
merged 6 commits into from
Jan 31, 2024
Merged

Implement Messages feature #61

merged 6 commits into from
Jan 31, 2024

Conversation

pritam-bs
Copy link

  • Provided NStackMessageWidget for the user to get and show messages from NStack
  • The user can use either the default dialog provided by the SDK or override the dialog and implement the UI as per the requirements.

- Provided NStackMessageWidget for the user to get and show messages from NStack
- The user can use either the default dialog provided by the SDK or override the dialog and implement the UI as per the requirements.
example/lib/main.dart Outdated Show resolved Hide resolved
example/lib/nstack.dart Show resolved Hide resolved
lib/sdk/messages/nstack_messages.dart Outdated Show resolved Hide resolved
example/lib/main.dart Outdated Show resolved Hide resolved
@hassan-saleh-ml
Copy link
Collaborator

@pritam-bs let's wait for @nivisi to review before merging

Copy link
Contributor

@nivisi nivisi left a comment

Choose a reason for hiding this comment

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

Edit: two changes requested on the nstack.dart must actually be on the .txt file, jfyi 😄

Comment on lines 259 to 278
abstract class NStackMessageOptions {}

class DefaultNstackMessageOptions implements NStackMessageOptions {
/// Title of the OK button.
final String? okButtonTitle;

/// Title of the Open URL button.
final String? openUrlButtonTitle;

/// Title of the dialog.
final String? dialogTitle;

DefaultNstackMessageOptions({
this.okButtonTitle,
this.openUrlButtonTitle,
this.dialogTitle,
});
}

class CustomNstackMessageOptions implements NStackMessageOptions {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since dart 3 we have new class modifiers that allow us to put restrictions on who and how can inherit from our classes (Official docs). My suggestion would be to follow the new rules:

  1. Make the parent NStackMessageOptions a sealed class, meaning it can only be inherited within our library. It will allow us to use a switch statement in the _onMessage callback. Refer to my message below.
  2. Make two subclasses final to prevent subtyping outside of this NStack library.
Suggested change
abstract class NStackMessageOptions {}
class DefaultNstackMessageOptions implements NStackMessageOptions {
/// Title of the OK button.
final String? okButtonTitle;
/// Title of the Open URL button.
final String? openUrlButtonTitle;
/// Title of the dialog.
final String? dialogTitle;
DefaultNstackMessageOptions({
this.okButtonTitle,
this.openUrlButtonTitle,
this.dialogTitle,
});
}
class CustomNstackMessageOptions implements NStackMessageOptions {
sealed class NStackMessageOptions {}
final class DefaultNstackMessageOptions implements NStackMessageOptions {
/// Title of the OK button.
final String? okButtonTitle;
/// Title of the Open URL button.
final String? openUrlButtonTitle;
/// Title of the dialog.
final String? dialogTitle;
DefaultNstackMessageOptions({
this.okButtonTitle,
this.openUrlButtonTitle,
this.dialogTitle,
});
}
final class CustomNstackMessageOptions implements NStackMessageOptions {

Comment on lines 320 to 336
void _onMessage(Message message) {
if (widget.messageOptions is CustomNstackMessageOptions) {
(widget.messageOptions as CustomNstackMessageOptions)
.onMessage
.call(message);
} else {
final messageOptions =
widget.messageOptions as DefaultNstackMessageOptions;
NStackMessageDialog.show(
context,
message: message,
okButtonTitle: messageOptions.okButtonTitle,
openUrlButtonTitle: messageOptions.openUrlButtonTitle,
dialogTitle: messageOptions.dialogTitle,
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If we'll use a sealed base class, we'll be able to do:

Suggested change
void _onMessage(Message message) {
if (widget.messageOptions is CustomNstackMessageOptions) {
(widget.messageOptions as CustomNstackMessageOptions)
.onMessage
.call(message);
} else {
final messageOptions =
widget.messageOptions as DefaultNstackMessageOptions;
NStackMessageDialog.show(
context,
message: message,
okButtonTitle: messageOptions.okButtonTitle,
openUrlButtonTitle: messageOptions.openUrlButtonTitle,
dialogTitle: messageOptions.dialogTitle,
);
}
}
void _onMessage(Message message) {
final messageOptions = widget.messageOptions;
switch (messageOptions) {
case CustomNstackMessageOptions():
messageOptions.onMessage(message);
break;
case DefaultNstackMessageOptions():
NStackMessageDialog.show(
context,
message: message,
okButtonTitle: messageOptions.okButtonTitle,
openUrlButtonTitle: messageOptions.openUrlButtonTitle,
dialogTitle: messageOptions.dialogTitle,
);
break;
}
}

Comment on lines 21 to 34
Future<void> setMessageViewed(int messageId) async {
try {
if (_appOpenData != null) {
await _repository.postMessageSeen(
appOpenData: _appOpenData!,
messageId: messageId,
);
} else {
LogUtil.log('NStack --> Could not post message seen.');
}
} catch (e) {
LogUtil.log('NStack --> Could not post message seen.');
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Future<void> setMessageViewed(int messageId) async {
try {
if (_appOpenData != null) {
await _repository.postMessageSeen(
appOpenData: _appOpenData!,
messageId: messageId,
);
} else {
LogUtil.log('NStack --> Could not post message seen.');
}
} catch (e) {
LogUtil.log('NStack --> Could not post message seen.');
}
}
Future<void> setMessageViewed(int messageId) async {
final appOpenData = _appOpenData;
if (appOpenData == null) {
LogUtil.log('NStack --> Could not post message seen.');
return;
}
try {
await _repository.postMessageSeen(
appOpenData: appOpenData,
messageId: messageId,
);
} catch (e) {
LogUtil.log('NStack --> Could not post message seen.');
}
}

example/lib/nstack.dart Outdated Show resolved Hide resolved
lib/templates/nstack_template.txt Outdated Show resolved Hide resolved
lib/templates/nstack_template.txt Outdated Show resolved Hide resolved
example/lib/nstack.dart Outdated Show resolved Hide resolved
@pritam-bs pritam-bs requested a review from nivisi January 25, 2024 20:23
Copy link
Contributor

@nivisi nivisi left a comment

Choose a reason for hiding this comment

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

LGTM

});

/// Configuration of how the message will be handled.
/// It can be either `DefaultNstackHandlerConfiguration` or `CustomNstackHandlerConfiguration`.
Copy link
Contributor

Choose a reason for hiding this comment

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

If we use square brackets, you'll be able to navigate to the references classes (basically anything referenced: methods, fields etc) easier by holding the CMND (or whatever) button.

Suggested change
/// It can be either `DefaultNstackHandlerConfiguration` or `CustomNstackHandlerConfiguration`.
/// It can be either [DefaultNstackHandlerConfiguration] or [CustomNstackHandlerConfiguration].

@pritam-bs pritam-bs merged commit 5f407a3 into develop Jan 31, 2024
1 check passed
@pritam-bs pritam-bs deleted the feature/message branch January 31, 2024 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants