search

MaikuB / flutter_local_notifications

A Flutter plugin for displaying local notifications on Android, iOS, macOS and Linux
2.47k stars 1.4k forks source link

[flutter_local_notification_linux] fixed definition and registration of Linux implementation #2371

Closed MaikuB closed 4 months ago

MaikuB commented 4 months ago

Closes #2368

Gustl22 commented 3 months ago

@MaikuB just chiming in and was curious, as it seems removing default_package in pubspec.yaml of flutter_local_notifications for linux seems to be the more proper fix for this, as it doesn't use any native platform code which needs registering. Or did I miss something?

EDIT: Sorry, I missed, that it's handled as a dart only plugin now. But then, the registration shouldn't be handled by the main package anymore: https://github.com/MaikuB/flutter_local_notifications/blob/49c487f351185c8067d0378ea896cc0fc24b4fa9/flutter_local_notifications/lib/src/flutter_local_notifications_plugin.dart#L41

MaikuB commented 3 months ago

@Gustl22 agreed though I would more than it doesn't have to be handled by main package than should. This is more because how the plugin is created where from a consumer's point of view, they're constructing an instance of the plugin so it would've been more natural to expect that the Linux implementation is registered then IMO. I hadn't made the changes on the main plugin as it was less urgent that it could be done later (note: I also need to test it out) and AFAIK doesn't cause an issue. The changes I did here were to conform with the expectations that were enforced by the recent changes in Flutter. I am aware though that federated plugin spec documents certain things to be done.

Something this issue does bring to mind though is the message behind the warning should re-evaluated. I saw that you submitted the PR to the Flutter SDK around this. Perhaps something you could look into? I'm not sure how much of an impact this may have on the community on the future

Gustl22 commented 3 months ago

My introduced error message just should make aware, that the package uses a keyword default_package, which has no effect and should be used otherwise and bring up the options. But yes, the error message is kind of misleading, as there can exist packages, but they just don't serve as a native or Dart plugin. So I added this as proposed by stuartmorgan: https://github.com/flutter/flutter/pull/152134