Emoji reactions support, take 4#2462
Conversation
|
I've been thinking about how showing who reacted to a post could work on the web UI; would it be better to show users like in the favorites/boosts lists in chronological order but with the emoji next to them somewhere (maybe next to the follow button?), or would it be better to have a list with separate tabs for different emojis? Here are a couple of mockups for a potential button to view them; one option I thought of was to put a clickable smiley icon next to the boost/favorite counts (taken with upstream's Material Icons set, but probably doable with FA4): |
See: #2221 (comment) Also Font Awesome will likely be removed. |
Wouldn't it be better for UX and general usability to do it "Misskey-Style" and show the usernames on hover? The list could be added anyway for a better mass-overview. |
|
If you do it MissKey-style (hover on mouse-over), how would you do that on mobile devices, where there is no mouse? |
|
Misskey and derivatives use both a mouse-over for a quick peek (shows only 11 people and then a "+[n]") and have either a link in the post's menu or a tab to list again all reactions and who used it, similarly to how Discord does it. ScreenshotsMouse-over: Misskey's variant: And on Firefish: Edit: Adding an icon + a total count next to the favorites and boosts, like the first pic from Essem, is a good idea and keeps it consistent with the other interactions. |
|
I'm still for removing modifications of the vanilla flavour. Also this modifies locales managed by Crowdin again |
Ah, apologies; I didn't realize that's why they were removed. Fixed. |
| before_action :process_params | ||
| before_action :set_status, only: [:mention, :favourite, :reblog] | ||
| before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request] | ||
| before_action :set_status, only: [:mention, :favourite, :reaction, :reblog] |
There was a problem hiding this comment.
From a server owner cost management and email reputation perspective, sending an email per reaction could get incredibly expensive really fast.
It would be great to have an option for
- roll reactions these up into 30-60min aggregate mails
- a server setting to outright disable email notifications for reactions
Personally, I would use (2) on my server, simply because the cost aspect of potentially sending 100s of mails to a user is not something I'm interested in bankrolling on behalf of my users, and it can have significant email deliverability impact as well if users get spammed and never opens/interacts with them.
There was a problem hiding this comment.
Reaction email notifications are disabled by default in user settings and notification emails of any type are only sent to inactive users anyways (unless a specific setting is manually enabled).
There was a problem hiding this comment.
It's great its disabled by default, but I do think it would be great to have a server setting to turn it off, with no way to turn it on - I really don't want to see someone (myself included) being burned by potentially expensive transactional mails due to malice of some trolls, or some random toot going viral and burning through mail credits.
It can be quite expensive for on-demand transactional emails, and can prohibit verification mails from going out if the server admin is using a pre-paid delivery plan with X mails per month.
There was a problem hiding this comment.
While the concern is understandable, this is not something this PR can or even should address.
As already stated, Mastodon rarely even sends e-mail notifications by default and most people don't enable those.
This just adds the option to receive e-mails for reactions, in line with other notifications.
Concerns about potentially burning e-mail credit should be brought up upstream, but this is pretty much a non-issue in practice.
|
Once the Material Symbols set is ported, I'm thinking that the current "plus" icon could be replaced with the set's own "Add Reaction" icon to better indicate what it's for and bring it more in line with other UI implementations. |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
58a54da to
1b0b4e9
Compare
|
This pull request has resolved merge conflicts and is ready for review. |
1b0b4e9 to
4d0e0e6
Compare
|
@TheEssem what're the blockers for un-drafting this, atm, btw? |
|
This pull request has merge conflicts that must be resolved before it can be merged. |
I'm mainly just waiting for further review. |
|
If you mean maintainer review then I'd suggest making it non-draft, since else (fme) maintainers won't look at it |
|
I'm sorry I really don't have much mental bandwidth right now, and my priority is bringing glitch-soc to a more maintainable state. There's a lot I've postponed, and it's becoming a lot. |
4d0e0e6 to
39ef8ca
Compare
|
This pull request has resolved merge conflicts and is ready for review. |
|
Is this suitable for being merged into a fork? |
|
Running in production at |
It's now broken and pointless due to the complete removal of legacy user settings. Since this effectively predates the current reaction branch as well, there is little chance that someone running an older iteration will need to use this migration.
…roller GHSA-gwhw-gcjx-72v8 seems to affect this as well. Let's patch that up.
|
This pull request has resolved merge conflicts and is ready for review. |
Replaces #1980, #2127, and #2221.
All previous commits have been merged and rebased. I've also fixed a couple of formatting issues and inconsistencies.
Let me know if there's anything missing. I hope to have this PR serve as a base for continued development of this patch.