-
Notifications
You must be signed in to change notification settings - Fork 856
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
Include hide comment photos setting #3939
Include hide comment photos setting #3939
Conversation
Not sure what the other think about this but IMO if the creator of the video places a comment its maybe better to see their profile picture for more visibility I know their name is highlighted but that could be easily overlooked IMO |
@robjob1938 Thoughts on this? I would personally agree with @efb4f5ff-1298-471a-8973-3d47447115dc, in that you would already be being presented with the video creator's thumbnail photo in the video info section anyway. |
Head branch was pushed to by a user without write access
The work @Benjababe did looks perfect, thank you very much! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with https://youtu.be/0cTpTMl8kFY
static/locales/en_GB.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one can be removed like stated in #3938 (comment)
static/locales/en-US.yaml
Outdated
@@ -344,6 +344,7 @@ Settings: | |||
Hide Active Subscriptions: Hide Active Subscriptions | |||
Hide Video Description: Hide Video Description | |||
Hide Comments: Hide Comments | |||
Hide Comment Photos: Hide Comment Photos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this naming. Maybe something like Hide Profile Pictures in Comments but i like to hear the other team members on this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Hide Comment Profile Pictures" or "Hide Comment Author Thumbnails" would be my top two suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Hide Profile Pictures in Comments"
"Hide Comment Profile Pictures" looks more confusing ("Comment Profile Pictures" is too much to parse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've changed my mind, that does sound better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opinions needed
Head branch was pushed to by a user without write access
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Include hide comment photos setting
Pull Request Type
Related issue
Closes #3919
Description
Adds toggle switch to Settings>Distraction Free Settings>Watch Page>Hide Comment Photos
Essentially hides profile pictures in comments, replaces it with the default background color of the FreeTube profile on the top right (assuming styling is subject to change) and the first character of the commenter's username.
Livestream chat isn't affected, not sure to touch it or not.
Screenshots
Replaces this
with
Testing
Looked through a couple of videos and it seemed fine
Desktop
Additional context
Same query with this PR about the i18n stuff.