Skip to content

Added Repeat feature#264

Open
Nix-Ka-dev wants to merge 3 commits into
uphy:masterfrom
Nix-Ka-dev:contribute
Open

Added Repeat feature#264
Nix-Ka-dev wants to merge 3 commits into
uphy:masterfrom
Nix-Ka-dev:contribute

Conversation

@Nix-Ka-dev

Copy link
Copy Markdown

I added two new settings: the first one enables notifications for reminders that are already overdue, and the second one sets the repeat interval. I added this because I sometimes missed the Windows notifications and ended up forgetting the reminders.

The README commits are because i changed it for the fork but then made a separate branch for contribute.

I had the problem that if I just ignored the reminder messages, they got muted. This was a problem because the Windows messages disappear after a certain time.
edited the readme
changed the readme to the original because i want to commit
@uphy

uphy commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Thank you for the contribution — re-notifying overdue reminders at an interval is a reasonable feature request, and putting the logic in Reminders.getExpiredReminders() (the testable model layer) was the right instinct. That said, the PR needs substantial rework before it can merge:

  1. Rebase onto current master. The branch conflicts with recent refactors: NotificationWorker now receives a NotificationWorkerDeps interface instead of holding a plugin reference, and the settings layer moved to a type-safe DSL. The repeat settings should be injected through the deps interface (wired up in src/main.ts), not read via this.plugin.settings....
  2. Respect explicit mutes. muteNotification currently means both "modal is being shown" and "the user explicitly pressed Mute". The PR's repeat condition only checks muteNotification === true, so a reminder the user deliberately muted would be re-notified forever at the configured interval. Please distinguish the two states (e.g. a separate flag) so explicitly muted reminders are excluded.
  3. Add tests. New logic in Reminders.getExpiredReminders() needs cases in src/model/reminder.test.ts: first notification, interval not yet elapsed, interval elapsed, explicitly muted.
  4. English only. Per the repository convention, all comments must be in English — please translate the German comments (// Speichert den Zeitstempel..., // Benachrichtigung anzeigen, etc.) and remove placeholder markers like // --- NEU ---.
  5. Lint. npx eslint reports 14 prettier errors on the branch — please run npm run lint:fix before pushing.
  6. Please drop the unrelated README.md diff (trailing-newline-only change).

Happy to re-review once these are addressed.

@uphy uphy added awaiting-response Waiting for a response from the PR author; auto-closed after prolonged inactivity pinned Exempt from the stale auto-close workflow and removed pinned Exempt from the stale auto-close workflow labels Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-response Waiting for a response from the PR author; auto-closed after prolonged inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants