-
Notifications
You must be signed in to change notification settings - Fork 869
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
Add provider to tip compliment #2555
Conversation
Remove deprecated API and replace with args->GetList() Fixes brave/brave-browser#4586
a07fe59
to
422c647
Compare
return; | ||
std::string tweet_id = args->GetList()[1].GetString(); |
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.
tweet_id
should be allowed to be empty so removed equivalent check used in old API
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.
why should be allowed to be empty?
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.
It appears that tweet_id
is given as empty in:
class TipSite extends React.Component<Props, {}> { | |
get actions () { | |
return this.props.actions | |
} | |
onTweet = () => { | |
let name = this.props.publisher.name | |
if (this.props.publisher.provider === 'twitter') { | |
const url = this.props.url | |
if (url && url.length > 0) { | |
name = `@${url.replace(/^.*\/(.*)$/, '$1')}` | |
} | |
} | |
this.actions.onTweet(name, '') | |
this.actions.onCloseDialog() | |
} |
I believe the field may be used in the case of a twitter tip to attach the complement as a reply (may be completely wrong) and is not applicable in the case where we tip a site?
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.
tipSite.tsx:35
updated below
@@ -260,7 +260,7 @@ | |||
<message name="IDS_BRAVE_REWARDS_LOCAL_GRANT_ALREADY_CLAIMED_TEXT" desc="">Sorry, it appears that this grant has already been claimed.</message> | |||
<message name="IDS_BRAVE_REWARDS_LOCAL_DONAT_NEXT_DATE" desc="Description of the date in the donation box">Next monthly tip date</message> | |||
<message name="IDS_BRAVE_REWARDS_LOCAL_COMPLIMENT_TWEET" desc=""> | |||
I just tipped <ph name="NAME">$1<ex>user</ex></ph> using the Brave Browser. Check it out at https://brave.com/tips. | |||
I just tipped <ph name="NAME">$1<ex>user</ex></ph> on <ph name="PROVIDER">$2<ex>site</ex></ph> using the Brave Browser. Check it out at https://brave.com/tips. |
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.
Requires proper formatting, @NejcZdovc will suggest fix
closing as stale, let's revisit when needed |
Fixes brave/brave-browser#4586
Added publisher provider to tweetTip compliment string displayed when sharing a tip via tweet.
Compliment string now reads "I just tipped [publisher] on [provider]"
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security
) onnpm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
See steps at brave/brave-browser#4586
Reviewer Checklist:
After-merge Checklist:
changes has landed on.