-
Notifications
You must be signed in to change notification settings - Fork 887
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
Rewards Greaselion - Github #6789
Conversation
4d33019
to
60e2627
Compare
@@ -134,7 +134,7 @@ class Banner extends React.Component<Props, State> { | |||
} else if (mediaMetaData.mediaType === 'reddit') { | |||
return `u/${mediaMetaData.userName}` | |||
} else if (mediaMetaData.mediaType === 'github') { | |||
return `@${mediaMetaData.userName}` | |||
return `@${mediaMetaData.publisherScreenName}` |
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 see that in tipMediaUser.tsx
we use '@' + mediaMetaData.publisherName
, where here we use mediaMetaData.publisherScreenName
. I would suggest that we do the same in both places and this way you don't need to introduce new parameter
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 think that's just a bug in tipMediaUser.tsx
(which is leading to issue #4
that you noticed below). The issue is that the Github tip dialog wants to show both the screen name (e.g. emerick
) and the publisher name (e.g., Emerick Rogul
). For some reason, the Twitter integration only ever showed one name so we were good.
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.
-
verified publisher don't get image https://github.com/jumde
-
regular site should have github.com https://github.com/team
-
@
should be the same. Now one is@yrliou
another is@Jocelyn Liu
3743116
to
08d1799
Compare
@NejcZdovc Scratch that, it would require putting localizations in brave-core. We'll just guard against empty translations in our content script. |
All feedback should be addressed. |
08d1799
to
9de3d28
Compare
Resolves brave/brave-browser#11463
Resolves brave/brave-browser#11766
Resolves brave/brave-browser#10434
Associated Greaselion script PR: brave/brave-site-specific-scripts#20
Submitter Checklist:
npm run lint
,npm run gn_check
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).QA/Yes
orQA/No
) to the associated issuerelease-notes/include
orrelease-notes/exclude
) to the associated issueTest Plan:
Note: In order to test this PR, you must manually install the Github Greaselion script at the moment. This involves pulling down the appropriate branch and running the following commands (the example is Windows specific, it will need to be modified slightly for Mac/Linux):
Desktop
Pull Request
https://github.com/brave/brave-core/pull/6743
)Issue
https://github.com/brave/brave-browser/issues/12013
)Commits
https://github.com/brave/brave-browser/commits/master
)Starred Repos
https://github.com/emerick?tab=stars
)Organization Members
https://github.com/orgs/brave/people
)Team Members
https://github.com/orgs/brave/teams/brave-core/members
)Gist
https://gist.github.com/darkdh/85fed2542190510aaee0f3ad91091818
)Excluded URL
https://github.com/about
)Activity
https://github.com/emerick
)activity_info
database table is updated as expectedHistory
Tab Activation
Tipping non-Github Publishers
Mobile
No changes were made to mobile platforms, so verify that Github navigation / tipping / Rewards panel continue to work as expected on those platforms.
Reviewer Checklist:
After-merge Checklist:
changes has landed on.