Added basic notification trigger support#337
Conversation
|
Hi @marcprux, |
marcprux
left a comment
There was a problem hiding this comment.
Apologies for the lengthy review time; I needed to be sure I understood what was happening here, which I think I do now. I assume you've tested it out locally a fair amount? Any screenshots you could post would be handy…
I do think this is a valuable feature. In hindsight, I wish we had kept notifications out of skip-ui altogether and externalized it somewhere like skip-notify, where I'd be more comfortable making experimental changes; but we can't break compatibility at this point.
My other reservation is adding the new androidx-work dependency, but it doesn't seem to be too large a dependency, and it is stable.
All that is to say, I think we're good with merging this once you make the one minor change (changing the "notifications" key).
This PR enhances the local push notification capabilities by introducing scheduled delivery via
UNTimeIntervalNotificationTriggerandUNCalendarNotificationTrigger. It focuses on single-event scheduling by evaluating thenextTriggerDateof the specific trigger, providing a solid foundation for timed push notifications while keeping the initial implementation simple by omitting recurring intervals.To try it out, you can check out the
NotificationPlaygroundin either Skip Showcase or Skip Showcase Fuse.Related PRs:
Thank you for contributing to the Skip project! Please review the contribution guide at https://skip.dev/docs/contributing/ for advice and guidance on making high-quality PRs.
Use this space to describe your change and add any labels (bug, enhancement, documentation, etc.) to help categorize your contribution.
Skip Pull Request Checklist:
swift test