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

Allow notification filter policy to also mute notifications #2799

Conversation

sneakers-the-rat
Copy link

Related to:

The new notification filters are great! unfortunately they remove some ability to completely hide unsolicited DMs, which is a problem as described above^

This PR makes the new notification filters behave like the existing status content filters so that one can either filter them with a message or mute them entirely. I took some of the existing components and styles from the other toggle settings so that it looks like this:

Screenshot 2024-07-29 at 12 12 04 AM

I have confirmed that it works, unsolicited dms simply don't show up in notifications or in the DM page, but i receive them as normal if i follow an account. same with the other settings. I am holding off on writing tests until i hear if this would be something y'all would be interested in, but if so then i would be happy to write them/improve the implementation however would be good :)

This is definitely not the best implementation, but i did it in such a way that i touch the existing behavior from upstream masto as little as possible - eg. ideally this would be an enum rather than two sets of boolean values, and i wouldn't be reusing the FilterCondition class like that but would instead refactor those methods out into a mixin, but thus is the fork of a fork lifestyle.

anyway lmk if this is unwanted and we can just test and deploy it on our fork, or any other changes!

@ClearlyClaire
Copy link

Thanks for your contribution, but for now we'll follow upstream.

If we decide to offer additional controls that upstream does not to outright dismiss some notifications, I think we'd have to be careful how we expose that in the API, and it may make more sense to have what you called mutes require and extend filters than have them mutually exclusive, otherwise this risks being very confusing with apps not aware of these extra settings.

But I won't ask you to make changes now since upstream isn't completely settled either.

Copy link

github-actions bot commented Aug 2, 2024

This pull request has merge conflicts that must be resolved before it can be merged.

@sneakers-the-rat
Copy link
Author

sneakers-the-rat commented Aug 2, 2024

Ok! I can hold off for now until upstream gets settled, but happy to make any changes once that happens!

Agreed it would be better as an enum, or something that would make the relationship of those options less confusing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants