search

Rapptz / discord-ext-menus

MIT License
234 stars 87 forks source link

Support ignoring reaction removal #35

Closed boehs closed 3 years ago

boehs commented 3 years ago

Hey 👋🏻 , this is my first ever code change to a repository I don't manage so chances are I probably did something wrong here, and that there is a better way to do so. If you know a better way feel free to suggest!

Rapptz commented 3 years ago

This is purposefully unsupported.

boehs commented 3 years ago

This is purposefully unsupported.

Just wondering... why?

Rapptz commented 3 years ago

It's bad UX. The reasons below are why I don't like auto-removal (not in this PR) and ignoring reaction removal (which is basically the same thing)

  1. It doesn't work in DMs, since you can only remove your own reaction and not the other person. So they have to click twice anyway in that case.
  2. It doesn't work if you don't have permissions. In this case you have to click twice again.
  3. If you remove it then it causes an extra HTTP request for the removal if it even works. This route is heavily rate limited so having extra requests for this is undesirable.

Ultimately the current code offers a consistent UX -- 1 click or tap and the button works. With the PR then this stuff will always take two clicks (bad UX) and if there's auto removal in the user code somewhere then there's going to be an extra HTTP request that might work or not leading it to require 2 clicks for the user anyway.

I'm just not a fan and would rather not offer library users a footgun like this.

boehs commented 3 years ago

I'm just not a fan and would rather not offer library users a footgun like this.

While I agree that the two click is probably better in most cases, I am a firm believer in the power of choice. in this case, it defaults to accepting both removal and addition, so all it really does is give users the choice of if they wish to utilize this feature, and unless they explicitly want it, it will default to the "better" way.

What's wrong with choice?

Rapptz commented 3 years ago

Adding the choice gives the implication that the library is okay with the footgun, when it isn't. I don't want library users to get banned for removing reactions and I don't want other menu consumers to suffer from poor UX. I also don't want to maintain a potential race condition when someone inevitably decides to delete a reaction and they forgot to toggle on this setting. Removing my comment in the PR kind of makes it seem like the race isn't there when I've warned about it well in advance.

boehs commented 3 years ago

Adding the choice gives the implication that the library is okay with the footgun, when it isn't. I don't want library users to get banned for removing reactions and I don't want other menu consumers to suffer from poor UX. I also don't want to maintain a potential race condition when someone inevitably decides to delete a reaction and they forgot to toggle on this setting. Removing my comment in the PR kind of makes it seem like the race isn't there when I've warned about it well in advance.

I could reinstate the comment 👀 . what you said about banning is interesting though, at worst wouldn't a rate limit be applied?