Add notification policies and notification requests in web UI #29433
Conversation
Gargron
force-pushed
the
feature-notifications-filter-ui
branch
3 times, most recently
from
13029ca
to
3951390
Compare
There was a problem hiding this comment.
This doesn't constitute a full review, but I tried this on my server to help me review/evaluate the backend changes in #29366. I noticed a few bugs, issues, and suggestions:
- the requests' accounts are not properly imported—I made inline code suggestions to fix this
- once fetched, the list of potentially-filtered notifications does not change when you change a request, misattributing them—I made inline code suggestions for this too
- the list of notification requests appears to never get updated during the application's lifetime, which is likely going to be an issue for people who rarely if ever reload their app
- I think there is an opportunity to hide all media by default in the new filtered notification column
Otherwise, I really like how it looks!
|
Those filter conditions are pretty good, but I'd be interested in bypassing the filter for "Notification from somebody followed by somebody I follow"... 2 degrees would be safer from spammer No idea about the performance overhead for that. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
Gargron
force-pushed
the
feature-notifications-filter-ui
branch
2 times, most recently
from
91e22a1
to
a26fca4
Compare
|
This pull request has resolved merge conflicts and is ready for review. |
Gargron
force-pushed
the
feature-notifications-filter-ui
branch
from
a26fca4
to
e81d369
Compare
Gargron
force-pushed
the
feature-notifications-filter-ui
branch
from
e81d369
to
7f6e24f
Compare
Gargron
force-pushed
the
feature-notifications-filter-ui
branch
from
7f6e24f
to
94e6c23
Compare
There was a problem hiding this comment.
This looks good overall, but I think there are two issues left worth addressing quickly (either in this PR or follow-up PRs):
- unlike mutes, there is currently no interface to review and undo “dismissed” notification requests
- accepting a notification request does not immediately backfill the notifications column with the newly-accepted notifications
| @@ -271,6 +272,8 @@ | |||
| "filter_modal.select_filter.subtitle": "Use an existing category or create a new one", | |||
| "filter_modal.select_filter.title": "Filter this post", | |||
| "filter_modal.title.status": "Filter a post", | |||
| "filtered_notifications_banner.pending_requests": "Notifications from {count, plural, =0 {no} one {one person} other {# people}} you may know", | |||
There was a problem hiding this comment.
when count is 0, source text says "Notifications from no you may know", which makes no sense
There was a problem hiding this comment.
That should've been "no one", but also, the banner is hidden when the count is 0.
Follow-up to #29366
Fixes MAS-39
Peek.2024-02-28.09-16.mp4