-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Add user notifications settings tab #31086
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
base: main
Are you sure you want to change the base?
Add user notifications settings tab #31086
Conversation
Adds a notification tab to the user's settings page, allowing a user to customize both the e-mail and in-UI notifications they receive. Additionally, this commit adds logic to handle UI notification options, similarly to what is already done for e-mail notifications. Refs: go-gitea#30537 Co-Authored-By: João Tiago <[email protected]> Signed-off-by: Rafael Girão <[email protected]> Signed-off-by: João Tiago <[email protected]>
Thank you for your contribution. Can you put some screenshots? |
Absolutely. Here's a quick showcase: Screencast.from.2024-05-28.18-37-24.webmRegarding your feedback on radio buttons: I can change to that if it's preferred, but I feel that it's more in line with the rest of the User's settings pages this way (I don't think there are any radio buttons there right now). I think it's also more versatile this way, if a future PR wants to introduce more options/customizability. (We deliberately kept the scope small so to not end up with a very large PR.) |
Yes, I mean convert the list menu to radio box list is better. |
Co-authored-by: João Tiago <[email protected]>
Co-authored-by: João Tiago <[email protected]>
options/locale/locale_en-US.ini
Outdated
@@ -680,6 +680,7 @@ block.list.none = You have not blocked any users. | |||
profile = Profile | |||
account = Account | |||
appearance = Appearance | |||
notifications = Notifications |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed there's already two entries for notifications
: notifications
and notification.notifications
, is there any explicit reason for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a history problem. Now we recommend to use existing translations if it is possible.
@@ -82,6 +83,7 @@ type User struct { | |||
Email string `xorm:"NOT NULL"` | |||
KeepEmailPrivate bool | |||
EmailNotificationsPreference string `xorm:"VARCHAR(20) NOT NULL DEFAULT 'enabled'"` | |||
UINotificationsPreference string `xorm:"VARCHAR(20) NOT NULL DEFAULT 'enabled'"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migration is missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you provide some pointers on how this should be done? I looked through some of the migration files/commits there and couldn't really understand what the general flow for creating a new migration is.
Besides the files themselves, I only found this page on the docs, but it only mentions on how to test new migrations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs is correct, but no details about how to do it.
What you need to do is that creating a new migration file in models/migrations/{latest version}
, and the file name should follow the order of other files.
Then add your newly added migration function in migrations file:
https://github.com/go-gitea/gitea/blob/main/models/migrations/migrations.go#L593
Then it will be auto executed after Gitea server started.
ctx.Data["PageIsSettingsNotifications"] = true | ||
|
||
// Set Email Notification Preference | ||
if ctx.FormString("_method") == "EMAIL" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified, as almost all codes are the same.
Co-authored-by: João Tiago <[email protected]>
Co-authored-by: João Tiago <[email protected]>
Co-authored-by: João Tiago <[email protected]>
Co-Authored-By: João Tiago <[email protected]>
Docs was moved to https://gitea.com/gitea/docs |
Adds a notification tab to the user's settings page, allowing a user to customize both the e-mail and in-UI notifications they receive.
Additionally, this adds logic to handle UI notification options, similarly to what is already done for e-mail notifications.
This was inspired by #30537.
"Inspired", since we don't implement the requested feature, but instead implement some of the groundwork needed for it, and other notification customization options.
Feedback appreciated! 🙂