[flutter_local_notifications] Refactor the README into platform-specific guides #2477
Conversation
|
Just finished the Android page, I will try to use a similar style for the other platforms. |
|
Sorry for the slow reply - busy month. I will take a look @Levi-Lesches |
|
@MaikuB The branch is at the halfway point: It has the setup guides for all three platforms that require it (android, iOS, and Windows) that explains all the compile-time steps and platform code changes needed to get their apps compiling. I also finished the cross-platform usage guide, which describes at a high level how the plugin is used and all the common features between the platforms (basically, all the methods on Now all that's left is to make a platform-specific usage guide for all the platforms (android, darwin, linux, windows, and web), with details on all the initialization settings, permissions nuances, and notification details. These are mentioned throughout the docs I wrote, so it's important I get them ready soon, but this is also a good time to start a review if you want to since the docs that are pushed are complete thorough. Once I've finished all the other guides, I'll rewrite the main README to be shorter and more to-the-point, with a table of links to the other setup guides. Also, can you confirm that MacOS and Linux don't have any special build / platform folder setup, like |
|
@MaikuB Ready for a review! Check out https://github.com/Levi-Lesches/flutter_local_notifications/tree/readme-refactor/flutter_local_notifications and all the links in the table there (1,300 lines of docs...) |
|
The tests can probably be fixed by just running them again. I made a PR to fix the windows tests at #2696 |
There was a problem hiding this comment.
Thanks a lot for this. Hopefully I've reviewed everything and left comments where appropriate. In addition to those comments, these are some general callouts
General comments
- Whilst I didn't put a style guide for this, be mindful of use of capitalisation. What I had been doing is to use sentence casing as much as possible even for titles. I can see titles had remained in sentence casing. However, the links are in title case though the application hasn't been consistent. I haven't commented on the each instance on where sentence casing should be used. Let me know if you'd like me to do so
- Review usages of comma vs colon when listing out properties/methods alongside their purposes. I've commented on most of these instances that can be reviewed
- A lot of paragraphs are being repeated in each of the platform-specific usage guides. There a way to reduce the repetition? One of the goals was to help the docs easier to digest but the repetition can add more "noise" for readers to sift through as they go through each platform
- Look at breaking up long running sentences. I've commented on some instances of this
onprefix is used for callbacks that indicate a particular event took place and shouldn't be used for methods. By using theonprefix for method names this breaks the rule of methods being named after verbs. Method names should also summarise what happens within the method as well. Furthermore, will not semantically make sense either to keep theonprefix. An example of addressing this issue is to use
onDidReceiveNotificationResponse: processNotificationResponse
This will translate to an English sentence along the lines of "on receiving a notification response, process the notification response". As opposed to
onDidReceiveNotificationResponse: onNotificationTap
This will translate to an English sentence along the lines of "on receiving a notification response, on tapping the notification".
sidenote: I had also noticed inconsistent naming (tap vs press) and past/present tense (tap vs tapped). Addressing what I mentioned will solve this and keep it simple. I recommend keeping this on the back of your mind for your own development projects. It's better to stick to one (e.g. tap vs press) and avoid using synonyms that can lead to confusion in a team environment. This may also lead to questions around if there are rules when one is used over the other
| > Given how quickly the Flutter ecosystem evolves, the minimum Flutter SDK version will be bumped occasionally to make it easier to maintain the plugin. Note that the official plugins already follow a similar approach. If this affects your applications (e.g., you need to support an older OS version), you may need to consider maintaining your own fork. | ||
| > [!IMPORTANT] | ||
| > | ||
| > **Local Notifications** are very different than **Push Notifications**! Push notifications, as offered by Firebase Cloud Messaging and Apple's Push Notifications Service, are sent from the server to your users directly, while local notifications are generated by the app on the user's device and not from your server. |
There was a problem hiding this comment.
| > **Local Notifications** are very different than **Push Notifications**! Push notifications, as offered by Firebase Cloud Messaging and Apple's Push Notifications Service, are sent from the server to your users directly, while local notifications are generated by the app on the user's device and not from your server. | |
| > **Local notifications** are very different than **push notifications**! Push notifications, as offered by Firebase Cloud Messaging (FCM) and Apple Push Notification Service (APNs), are sent from the server to your users directly. Local notifications are generated by the app on the user's device and not from your server. |
Changes proposed there to
- address casing
- fix naming of what APNs represents
- include abbreviations for FCM and APNs and
- avoid long sentences
| > | ||
| > Make sure to read the [Android Setup Guide](./android-setup.md) and [General Usage Guide](./usage.md) first. | ||
| This guide will explore all the features this plugin has available for apps running on Android. Some features can only be called on the Android plugin, not the general plugin. The rest of this guide will assume the following: |
There was a problem hiding this comment.
| This guide will explore all the features this plugin has available for apps running on Android. Some features can only be called on the Android plugin, not the general plugin. The rest of this guide will assume the following: | |
| This guide will explore all the features this plugin has available for apps running on Android. Some features can only be called on the Android plugin, not the general plugin. The rest of this guide will assume the Android plugin has been resolved through the following code: |
Elaborated on the last sentence as it may not clear to readers what "the following" was referring and can look like they were given some code without much context
| ### Notification channel options | ||
|
|
||
| Notification channels support the following options. These options may only be set when the channel is being created, and may not be changed later. If you need to, you can delete the channel and recreate it, or guide the user to change it themselves. You can call `getNotificationChannels()` to get a list of channels, but note that users may change these options at any time in the settings, so the options may not be the same as when you created them. |
There was a problem hiding this comment.
| Notification channels support the following options. These options may only be set when the channel is being created, and may not be changed later. If you need to, you can delete the channel and recreate it, or guide the user to change it themselves. You can call `getNotificationChannels()` to get a list of channels, but note that users may change these options at any time in the settings, so the options may not be the same as when you created them. | |
| Notification channels support the following options. These options may only be set when the channel is being created, and may not be changed later. If you need to, you can delete the channel and recreate it, or guide the user to change it themselves. You can call `getNotificationChannels()` to get a list of channels, but note that users may change these options at any time in the settings. Therefore, the options may not be the same as when you created them. |
Broke up long sentence at the end. On a general note, I'd advise to look for opportunities to avoid long running sentences. This is something I used to do when writing documentation, theses etc so passing on the advice I got from back then too
|
|
||
| - `description`: will be shown to the user in the settings app, but not in the notifications themselves | ||
| - `groupId`: which notification channel group this channel belongs to (see below) | ||
| - `importance`: how important notifications in this channel should be |
There was a problem hiding this comment.
I've not spotted it but it looks as though a note around the importance is missing. The documentation in the plugin use to bring attention to how a heads-up notification depended on correct importance. It has a link to this
An appropriate spot should be found to add mention of this and reference appropriate Android docs. May also be worth adding a link to this as well
| ## Permissions | ||
|
|
||
| This next section will deal with requesting permissions. As noted in the general usage guide, requesting permissions is a very sensitive process in terms of user experience, and must be done only when necessary and only after informing the user of why you need the permissions. If the user rejects your request – which they might if they feel annoyed or don't understand why you need it – Android will stop showing your requests to the user and start blocking them automatically. If this happens, you will need to prompt the user to go to the settings themselves and grant your app permissions manually. |
There was a problem hiding this comment.
| This next section will deal with requesting permissions. As noted in the general usage guide, requesting permissions is a very sensitive process in terms of user experience, and must be done only when necessary and only after informing the user of why you need the permissions. If the user rejects your request – which they might if they feel annoyed or don't understand why you need it – Android will stop showing your requests to the user and start blocking them automatically. If this happens, you will need to prompt the user to go to the settings themselves and grant your app permissions manually. | |
| This section will deal with requesting permissions. As noted in the general usage guide, requesting permissions is a very sensitive process in terms of user experience. It must be done only when necessary and only after informing the user of why you need the permissions. If for whatever reason the user rejects your request, Android will stop showing your requests to the user and start blocking them automatically. If this happens, you will need to prompt the user to go to the settings themselves and grant your app permissions manually. |
- "This next section..." -> "This section...": next was redundant
- Broke up sentences
- Shortened sentence of user rejecting. This involved stripping out the reasons to call out this can happen for any reason to avoid having an exhaustive list of potential reasons
There was a problem hiding this comment.
Good shout on mentioning that app can guide users to settings. Since it's doing so already, perhaps it should mention an example of how to do so via this function from the permission_handler package?
|
|
||
| For example, imagine a messaging app that just got a message from another user. Your notification may include a text action, allowing the user to send a response directly within the notification. Again, the exact details here differ by platform. | ||
|
|
||
| Actions affect how your app handles responses. The foreground handler, background handler, and `getNotificationAppLaunchDetails()` method all provide you with a `NotificationResponse` object, which will contain a non-null `actionId` if an action was used, with a non-null `input` (or `data`) if a text field was used. For example, a notification can have a payload `new-message_CHAT_ID` and an `actionId` of `mark-read`. |
There was a problem hiding this comment.
See comment on foreground/background handler that has implications on this section. Once addressed and renamed, can add a link to the sections on said handlers too
|
|
||
| ## MSIX packaging | ||
|
|
||
| The [MSIX packaging format](https://learn.microsoft.com/en-us/windows/msix/overview) is a relatively new technique to package and distribute modern Windows apps. An MSIX installs an app onto the system as if it were downloaded from the Windows store (indeed, Windows store apps are packaged with MSIX). MSIX installers can also be used to update an existing application while preserving user data, making them really convenient. |
There was a problem hiding this comment.
The note around store apps being packaged as a MSIX can go out into its own sentence. There may also be opportunity to link to this or one of the subsections on packaging in the official Flutter docs
|
|
||
| ### Custom images and sounds | ||
|
|
||
| images can come from different sources: the web, MSIX bundles, files on the user's device, or Flutter assets. See the documentation for [`WindowsImage`](https://pub.dev/documentation/flutter_local_notifications/latest/flutter_local_notifications/WindowsImage-class.html) for complete details on when each time of source is usable. |
There was a problem hiding this comment.
| images can come from different sources: the web, MSIX bundles, files on the user's device, or Flutter assets. See the documentation for [`WindowsImage`](https://pub.dev/documentation/flutter_local_notifications/latest/flutter_local_notifications/WindowsImage-class.html) for complete details on when each time of source is usable. | |
| Images can come from different sources: the web, MSIX bundles, files on the user's device, or Flutter assets. See the documentation for [`WindowsImage`](https://pub.dev/documentation/flutter_local_notifications/latest/flutter_local_notifications/WindowsImage-class.html) for complete details on when each time of source is usable. |
Casing issue at the beginning
|
|
||
| ### Custom XML | ||
|
|
||
| Windows notifications are sent in the form of an XML schema, which the Windows SDK has to parse and verify. Using the `WindowsNotificationDetails` class in this library lets you skip that and build your notification with pure Dart code, but if you wanted to use raw XML for some unsupported or custom behavior, you can: |
There was a problem hiding this comment.
- Missing a bit of context towards the end by saying something along the lines of "you can call the following methods"
- The list if missing a colon
:to separate the methods and description like you have elsewhere
|
|
||
| To use a custom icon, specify `LinuxInitializationSettings.defaultIcon` for every notification, or override on a per-notification basis with `LinuxNotificationDetails.icon`. Icons can be one of: | ||
|
|
||
| - `AssetsLinuxIcon`, which take a Flutter asset |
There was a problem hiding this comment.
Use colon as separator for lists like this
3 participants
Based on the work in #2308, this PR will split the README into platform-specific guides and topics (that can then be hosted on another website besides github, if desired), and leave the main README for Dart-specific details. Specifically, my goal is to:
Progress:
Verifying that all the new docs are correct (by going through them with a brand new app) is on my list but will take a bit longer. In the meantime, I've reviewed, revised, and re-reviewed all of these several times, and caught mistakes in the README and API docs, so I'm reasonably confident.
cc @davetrollope if you're interested
Closes #2196
Closes #2308