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

Deprecated /(un)follow commands and respective usercard action #3078

Merged
merged 10 commits into from Aug 4, 2021

Conversation

zneix
Copy link
Collaborator

@zneix zneix commented Jul 27, 2021

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

As announced, /helix/users/follows endpoints have been decomissioned today.

farewell, beautiful Helix code

CC #3076 #2629

@zneix
Copy link
Collaborator Author

zneix commented Jul 27, 2021

For now, /follow and /unfollow have been marked as deprecated:
saj

And for the time being, follow checkbox in usercard remains but it's permanently disabled:
CoolCat

This maybe isn't the ideal solution as it could confuse users, making them think they can still (un)follow and try to click the checkbox.
We could either:

  1. Remove the checkbox and checking if current user follows the user of whom usercard is opened altogether.
  2. Think of some alternative, like the one that @Mm2PL suggested:
Click to expand

GIMP masters

Copy link
Collaborator

@Felanbird Felanbird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor English loophole:
Twitch has taken away our ability to manage follows, not the quality of, or possibility to make changes for the endpoint.
https://wikidiff.com/ability/possibility

As for the changelog I'm indifferent in the usage of (un)follow 🤷

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved
src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@Felanbird
Copy link
Collaborator

As for the future of the button, I'm going to put my vote for keeping the button with its ability show follow status as currently handled, but making the functionality return an error message, as done with the slash commands in this PR.

Possibly removing it in the future if we need button room for some reason.

@zneix
Copy link
Collaborator Author

zneix commented Jul 29, 2021

but making the functionality return an error message, as done with the slash commands in this PR.

As I've stated in my comment under this PR:

follow checkbox in usercard remains but it's permanently disabled

Do you mean we should keep the functionality of the button but add error messages notifying users that (un)following failed or did you just overlook that part where I said it? I believe there should be no logic for event actions related to that button as it's gonna be literally cosmetic at this point (if we decide to keep it in the first place that is).

@Felanbird
Copy link
Collaborator

Felanbird commented Jul 29, 2021

Do you mean we should keep the functionality of the button but add error messages notifying users that (un)following failed

Yes, I think if we keep the button it should at least do something indicating to users that the attempt failed, possibly something like this:

(I followed off-screen in browser chat)

I believe there should be no logic for event actions related to that button as it's gonna be literally cosmetic at this point (if we decide to keep it in the first place that is).

If we to remove the all logic we should just remove the button in its entirety, no need for a cosmetic button.

pajlada and others added 2 commits July 31, 2021 12:47
Co-authored-by: Felanbird <41973452+Felanbird@users.noreply.github.com>
Co-authored-by: Felanbird <41973452+Felanbird@users.noreply.github.com>
@Felanbird
Copy link
Collaborator

Works as intended, the only problem seems to be the loading time, you have hover the button for 1.5~ seconds before the prompt appears. It took me a while to actually get the prompt because I was clicking the "button" and and doing so resets the loading.

image

@pajlada
Copy link
Member

pajlada commented Aug 1, 2021

Works as intended, the only problem seems to be the loading time, you have hover the button for 1.5~ seconds before the prompt appears. It took me a while to actually get the prompt because I was clicking the "button" and and doing so resets the loading.

image

That's unfortunately fairly tricky for us to solve without creating a new QCheckBox type and re-implementing it's click handler making it show the tooltip.

@Mm2PL
Copy link
Collaborator

Mm2PL commented Aug 1, 2021

About the usercard: maybe we could change the text color/effects. For example gray out the Follow text or cross it out. It's impossible to tell that you cannot follow people anymore

@pajlada
Copy link
Member

pajlada commented Aug 1, 2021

All in favour of removing the checkbox?

@zneix zneix requested a review from pajlada August 1, 2021 22:44
Copy link
Collaborator

@Mm2PL Mm2PL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@zneix zneix enabled auto-merge (squash) August 4, 2021 20:24
@zneix zneix merged commit 0c5abb8 into master Aug 4, 2021
@zneix zneix deleted the zneix/chore/remove-follow-endpoints branch August 4, 2021 20:41
zneix referenced this pull request in SevenTV/chatterino7 Aug 4, 2021
Now we're on commit de4f6a9; Changes from upstream we've pulled:

- Major: Fixed constant disconnections with more than 20 channels by rate-limiting outgoing JOIN messages. (Chatterino#3112, Chatterino#3115)
LosFarmosCTL added a commit to LosFarmosCTL/chatterino2 that referenced this pull request Apr 6, 2022
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.

None yet

4 participants