-
Notifications
You must be signed in to change notification settings - Fork 5
feat(#51): Implement Messages
feature
#55
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
base: master
Are you sure you want to change the base?
feat(#51): Implement Messages
feature
#55
Conversation
/// ); | ||
/// } | ||
/// ``` | ||
class NStackMessageListener extends StatefulWidget { |
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 we do factory methods on the main NStack class?
Kind of like NStack.Message()
We could potentially extend that to have NStack.MessageCustom()
etc
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.
@johsoe why?
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.
No other reason than i like that pattern more than exposing multiple NStack* classes upfront which can be a bit messy to figure out
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.
@johsoe I guess you mean a named constructor, cause a factory on a class T
can only return an instance of T
, so in this case if we use a factory on NStack
we can't return a widget. Named constructors also can't return widgets:
class NStack {
Widget NStack.Message() {
return Container();
}
}
You will get the following errors:
Constructors can't have a return type. Try removing the return type.dart(constructor_with_return_type)
Constructors can't return values. Try removing the return statement or using a factory
To return a widget you can use a static method:
class NStack {
static Widget message({Widget? child}) {
return child ?? Container();
}
}
// And then call `NStack.message(child: HomeScreen())`
However, one interesting implementation I came up with is using a mixin:
mixin NStackMessageListener<T extends StatefulWidget> on State<T> {
StreamSubscription? _subscription;
void onMessage(Message message) {}
@override
void didChangeDependencies() {
if (_subscription == null) {
final scope = NStackScope.of(context);
_subscription = scope.messages.onMessage.listen(onMessage);
}
super.didChangeDependencies();
}
@override
void dispose() {
_subscription?.cancel();
super.dispose();
}
}
And in your widget:
class MainScreen extends StatefulWidget {
const MainScreen({super.key});
@override
State<MainScreen> createState() => _MainScreenState();
}
class _MainScreenState extends State<MainScreen> with NStackMessageListener {
@override
void onMessage(Message message) {
NStackMessageDialog.show(context, message: message);
super.onMessage(message);
}
@override
Widget build(BuildContext context) {
// ...
}
}
// There's no dispose / close method | ||
// ignore: close_sinks | ||
final _onMessage = StreamController<Message>.broadcast(); |
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.
So, this stream controller will remain open? I also noticed another StreamController
not being closed in NStackLocalization
called _onLocaleChanged
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.
Maybe the stream controller is a bit overkill here, I guess we can leave it as a normal future request that we call from the main widget and just call onMessage
.
In the future if a web-socket connection is required we can refactor and make it a stream in a way that it can be disposed.
// There's no dispose / close method | ||
// ignore: close_sinks | ||
final _onMessage = StreamController<Message>.broadcast(); |
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.
Maybe the stream controller is a bit overkill here, I guess we can leave it as a normal future request that we call from the main widget and just call onMessage
.
In the future if a web-socket connection is required we can refactor and make it a stream in a way that it can be disposed.
@@ -142,6 +158,8 @@ class _Test extends SectionKeyDelegate { | |||
* NStack Flutter Widgets | |||
* | |||
*/ | |||
|
|||
/// Allows to access NStack features via a `BuildContext`. | |||
class NStackScope extends InheritedWidget { | |||
final NStackState state; |
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.
This is not being used I think it's safe to remove, plus storing a widget's state is not a best practice and might produce unpredictable results.
onMessage: (message) { | ||
NStackMessageDialog.show(context, message: message); | ||
}, |
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 think the onMessage can be added directly to NStackWidget
, hence no need for an NStackMessageListener
.
class ExampleApp extends StatelessWidget {
const ExampleApp({super.key});
@override
Widget build(BuildContext context) {
return MaterialApp(
builder: (context, child) {
return NStackWidget(
child: child!,
onMessage: (message) {
// ...
}
);
},
home: const MainScreen(),
);
}
}
@@ -42,20 +51,27 @@ export 'package:nstack/models/app_open_platform.dart'; | |||
*/ | |||
|
|||
final NStack = NStackSdk<Localization>( |
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.
Does it have to be a global variable?
Resolves #51
NStack.messages.onMessage()
NStackMessageListener
widgetNStackMessageDialog(message: message)
widget. On Android it shows a material dialog, on iOS — a cupertino dialog