Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

"Never include" messaging needs awareness #12296

Closed
jonathansampson opened this issue Dec 14, 2017 · 18 comments · Fixed by #12960
Closed

"Never include" messaging needs awareness #12296

jonathansampson opened this issue Dec 14, 2017 · 18 comments · Fixed by #12960

Comments

@jonathansampson
Copy link
Collaborator

jonathansampson commented Dec 14, 2017

Test plan

#12960 (comment)


Description

When right-clicking a YouTube Channel in Brave Payments, the user is provided the option to Never include this site. Instead, the option should be aware that the user has not selected a site, but a YouTube Channel. Perhaps a general term (Never include this content) should be adopted.

Steps to Reproduce

  1. Right-click a YouTube channel in Brave Payments.

Actual result:
Never include this site

Expected result:
Never include this content

Reproduces how often:
100%

Brave Version

0.19.116

image

@srirambv srirambv added feature/rewards polish Nice to have — usually related to front-end/visual tasks. suggestion labels Dec 15, 2017
@srirambv srirambv added this to the Triage Backlog milestone Dec 15, 2017
@NejcZdovc
Copy link
Contributor

@jonathansampson what if we would say Never include this publisher?

@jonathansampson
Copy link
Collaborator Author

@NejcZdovc That would be better, but I don't think Publisher really describes YouTube Channels very well. I think we should either come up with a single word/phrase that describes both, or conditionally check the type of item before printing the label.

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 20, 2017

@jonathansampson yeah we can do type check as well, but why I said publisher is because when communicating to the public we always say publishers and we have publishers.basicattentiontoken.org

@jonathansampson
Copy link
Collaborator Author

jonathansampson commented Dec 20, 2017

@NejcZdovc Publishers was the original term used, when we were supporting only sites. But when we added support for YouTube, we made https://brave.com/creators. It could go either way. To me, "publishers" refers only to text. Something vague like Content Creator can be any type of media and/or format.

@NejcZdovc
Copy link
Contributor

maybe Never include this creator?

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Jan 15, 2018

To fix this problem we need to modify string deleteLedgerEntry in menu.properties file.

@NejcZdovc NejcZdovc added copy/content/text includes hints ╭(◔ ◡ ◔)/ A good first bugs w/ hints made by someone from the team. labels Jan 15, 2018
@MargarytaChepiga
Copy link
Contributor

Hi, I am new to Brave and I would love to work on this if this is still available!

@NejcZdovc
Copy link
Contributor

@MargarytaChepiga yes it's still available. Will assign issue to me, so that no one will take it. If you need any help let me know

@NejcZdovc NejcZdovc self-assigned this Jan 22, 2018
@MargarytaChepiga
Copy link
Contributor

@NejcZdovc Great! Thank you so much!

@MargarytaChepiga
Copy link
Contributor

Hello, I am all set to fix this. The only thing is that I am not sure is to which message I should change the existing one. How about Never include this content creator? Or should I stick to the mentioned above Never include this creator ? Thanks!

@MargarytaChepiga
Copy link
Contributor

I know Ukrainian and Russian, so I might as well change the string in those languages as well. Is that okay?

@bsclifton
Copy link
Member

@MargarytaChepiga no need to modify Ukrainian or Russian- the process for translations is a little different. In this repo, you only need to update the en-US version

For more information on our translations (and to sign up to help 😄 ), please see:
https://github.com/brave/browser-laptop/blob/master/docs/translations.md

@MargarytaChepiga
Copy link
Contributor

@bsclifton Great! Thank you so much!

MargarytaChepiga added a commit to MargarytaChepiga/browser-laptop that referenced this issue Jan 30, 2018
MargarytaChepiga added a commit to MargarytaChepiga/browser-laptop that referenced this issue Jan 31, 2018
@NejcZdovc NejcZdovc modified the milestones: Triage Backlog, 0.20.x Hotfix 3 (Ledger improvments) Feb 6, 2018
@alexwykoff alexwykoff modified the milestones: 0.20.x Hotfix 3 (Ledger improvments), 0.21.x (Beta Channel) Feb 6, 2018
NejcZdovc added a commit that referenced this issue Feb 7, 2018
…nts-issue

Changed "Never include" message on payments page. Fix #12296
NejcZdovc added a commit that referenced this issue Feb 7, 2018
…nts-issue

Changed "Never include" message on payments page. Fix #12296
NejcZdovc added a commit that referenced this issue Feb 7, 2018
…nts-issue

Changed "Never include" message on payments page. Fix #12296
@btlechowski
Copy link
Contributor

Verified 0.21.9 Win64

@kjozwiak
Copy link
Member

As per our earlier conversation with @NejcZdovc in the #testers channel, ledger functionality won't work until the twitch code is pulled in which includes the updated BAT libraries.

Removed QA/checked-Win64 as we should re-test this just incase once Twitch lands.

@NejcZdovc NejcZdovc modified the milestones: 0.21.x (Beta Channel), 0.21.x (Twitch) Feb 24, 2018
NejcZdovc added a commit that referenced this issue Feb 24, 2018
…nts-issue

Changed "Never include" message on payments page. Fix #12296
@LaurenWags
Copy link
Member

Removed QA/checked labels due to changes in milestones. Please recheck with 0.21.15 and higher.

@srirambv srirambv added the 0.21.x issue first seen in 0.21.x label Feb 27, 2018
@srirambv
Copy link
Collaborator

Reopening based on #12960 (comment)

@srirambv srirambv reopened this Feb 27, 2018
@srirambv
Copy link
Collaborator

Closing the issue as its an issue with missing translations

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

Successfully merging a pull request may close this issue.

10 participants