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

Add more notification settings #105

Merged
merged 64 commits into from
Jan 2, 2024
Merged

Conversation

nucleus-ffm
Copy link
Owner

@nucleus-ffm nucleus-ffm commented Dec 18, 2023

@MatsG23
Copy link
Collaborator

MatsG23 commented Dec 20, 2023

I think we should only show the AlertSwiss source in the notification settings if it has been turned on in the general settings.
Now that we have a settings page for the notifications, would it not make sense to also move the "show status notification" settings there?

@MatsG23
Copy link
Collaborator

MatsG23 commented Dec 20, 2023

It would be also a cool addition if we could hide (collapse) the frequency of background updates setting when background updates are disabled in general.

@MatsG23
Copy link
Collaborator

MatsG23 commented Dec 23, 2023

Let me explain my quite big changes to the code for the notification source settings:

  • storing a Severity instead of NotificationLevel in the NotificationPreferences (removal of NotificationLevel enum)

  • adding a disabled property to NotificationProperties
    => makes re-enabling restore the last notification level for that source

  • removal of Severity.other (as it is has always been treated as Severity.minor)

  • make filling the NotificationProperties with initial data a loop and move this to the class with the default values for the app settings
    => as the app already needs these settings when it calls Place#_checkIfEventShouldBeNotified (used to have unprecise checks until you open the notification settings page)

  • reversing the order of severities on the slider (Severity.minor is now 0 etc.)

Instead of the NotificationLevel enum that had values like "upToMinor" (specified a range), the app now checks the enum index value of the stored notification level (a Severity) and the warning. This is a much simpler approach.

@MatsG23
Copy link
Collaborator

MatsG23 commented Dec 23, 2023

TODO: Also replace convertSourceToInt (services/sortWarnings.dart) with a method in the related enum class (use a for-loop as seen in enums/Severity.dart in the getIndexFromSeverity method for simplicity)
=> check the order of the enum values (especially LHP, AlertSwiss and Other) as they had no unique integer before (convert function returned 0 for everything except Mowas, Katwarn, Biwapp and DWD - although this likely aims for a stable order for all providers - see sortWarnings function)

lib/enums/Severity.dart Outdated Show resolved Hide resolved
lib/enums/WarningSource.dart Outdated Show resolved Hide resolved
nucleus-ffm and others added 26 commits January 1, 2024 18:43
Currently translated at 100.0% (223 of 223 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (227 of 227 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (227 of 227 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/de/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/fr/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/de/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/fr/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/en/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/de/
Currently translated at 100.0% (229 of 229 strings)

Translation: FOSS Warn/FOSS Warn-App
Translate-URL: https://hosted.weblate.org/projects/foss-warn/foss-warn-app/fr/
@nucleus-ffm nucleus-ffm merged commit c98831d into main Jan 2, 2024
1 check passed
@MatsG23 MatsG23 deleted the addMoreNotificationSettings branch June 6, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants