search

Rayquaza01 / HistoryCleaner

Firefox addon that deletes history older than a specified amount of days.
MIT License
78 stars 6 forks source link

re-enable notification checkbox #33

Closed esanchma closed 2 weeks ago

esanchma commented 4 months ago

In d5822e the notification checkbox was disabled, so no matter what the user selects, a notification is always shown. Why was that changed? What is needed to re-enable the notification checkbox?

Rayquaza01 commented 4 months ago

Previously, an optional permission was used for notifications. When this permission was granted, the checkbox would be enabled.

In the new version, the notification permission is changed from being an optional permission to a required permission. Because of this, there is no longer a situation where the checkbox is disabled.

The reason I changed the notification permission to required is mainly because having an optional permission AND a toggle for notifications is redundant. Enabling notifications involved setting two options that both essentially mean the same thing. I think it should only be one or the other.

Notifications are still only sent when the checkbox is checked. If you do not enable notifications, History Cleaner will not send notifications.

Rayquaza01 commented 2 weeks ago

I beleive this was fixed in 1.5.1. I had accidentally forgotten to check the notification setting when clearing ALL history. See https://github.com/Rayquaza01/HistoryCleaner/commit/7cf43a507051d25319ffcf8e5dcd5fabf89263df