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

Potential issue with seeing notification actions on iOS #1481

Closed
MaikuB opened this issue Feb 11, 2022 · 1 comment
Closed

Potential issue with seeing notification actions on iOS #1481

MaikuB opened this issue Feb 11, 2022 · 1 comment
Labels
notification actions Relates to functionality associated notification actions

Comments

@MaikuB
Copy link
Owner

MaikuB commented Feb 11, 2022

At some point, I've not been able to see notification actions on iOS yet. Notification actions do appear on macOS though. Upon digging further the culprit seems to be around this code

[center getNotificationCategoriesWithCompletionHandler:^(

Here the code is querying the list of currently registered categories and then appending to that list the categories that was specified via the APIs exposed by the plugin. As far as I can tell from various docs/articles and my own testing, querying and then appending isn't necessary. It's sufficient to replace the logic with a single call to [center setNotificationCategories:newCategories].

If I'm not mistaken, the logic in its current state (i.e. where it merges the categories) could potentially lead to bugs where if apps call initialize() multiple times with different categories, the older categories aren't remain even though the semantics of that imply that the categories in the last call should be the ones that remain.

@ened have I missed a reason why the plugin should query the list of notification categories and should spend to it instead of overriding it?

Note: interestingly similar logic exists on macOS but I haven't run into this issue

@MaikuB
Copy link
Owner Author

MaikuB commented Apr 23, 2022

Closing this as the change has been done and it fixed the issue I was experiencing. Furthermore, saw that at least one other notifications library for other cross-platform frameworks approach this the same way where they don't append to the existing categories

@MaikuB MaikuB closed this as completed Apr 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notification actions Relates to functionality associated notification actions
Projects
None yet
Development

No branches or pull requests

1 participant