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

feat: Include following count in hide follower count setting #1897

Merged
merged 2 commits into from
Mar 16, 2023

Conversation

kkoyung
Copy link
Contributor

@kkoyung kkoyung commented Mar 16, 2023

Before changes: Enabling "Hide follower count" in the wellbeing setting can hide the follower count in the account profile page.

I changed that setting to "Hide following/follower count" which can hide both following count and follower count.

Close #1878

@stackblitz
Copy link

stackblitz bot commented Mar 16, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Mar 16, 2023

Deploy Preview for elk-zone ready!

Name Link
🔨 Latest commit ed32c91
🔍 Latest deploy log https://app.netlify.com/sites/elk-zone/deploys/641324407971a0000757bc08
😎 Deploy Preview https://deploy-preview-1897--elk-zone.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Mar 16, 2023

Deploy Preview for elk-docs canceled.

Name Link
🔨 Latest commit ed32c91
🔍 Latest deploy log https://app.netlify.com/sites/elk-docs/deploys/641324406d69c200083ae13e

Comment on lines 81 to 82
:checked="getPreferences(userSettings, 'hideFollowingFollowerCount')"
@click="togglePreferences('hideFollowingFollowerCount')"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I think we should leave the name of the preference as it was before though because if not, everyone will lose this preference and will need to re-enable it. We could add a backward-compatible scheme, but I think the previous name is good enough here.

@kkoyung
Copy link
Contributor Author

kkoyung commented Mar 16, 2023

@patak-dev I reversed the name of preference to hideFollowerCount and the name of the displayed text to hide_follower_count. I kept the text displayed at the setting page as "Hide following/follower count" in order to precisely describe the setting.

Since the name of the displayed text remains unchanged and the non-English displayed texts have not yet updated in this PR. User who uses non-English interface may not notice the changes. Remember to update the language file for other language later.

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Let's merge this.

But I wonder if we should also only hide the followers/following counts in hover cards and then on profile still show "Followers" / "Following" without numbers but at least let the user click on them so they can explore. Checking who a person follows is a good way to learn about new good follows, and I think it doesn't go against the health preference.

@patak-dev patak-dev merged commit c7558ee into elk-zone:main Mar 16, 2023
@kkoyung kkoyung deleted the hide-follower-following-count branch March 17, 2023 04:22
@kkoyung
Copy link
Contributor Author

kkoyung commented Mar 17, 2023

@patak-dev
I prefer hiding the counts in both hover cards and full profile page, since we don't have hover cards on mobile browsers. If we only hide the counts on hover cards, it means mobile user will lose this hiding count function.

But I agree to keep showing "Followers" / "Following" label without numbers (in both hover cards and full profile page) so that user can click on them to see the following list and the follower list.

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

Successfully merging this pull request may close these issues.

Add "hide following count" to Wellbeing options
2 participants