Skip to content

Commit

Permalink
Address code review feedback (squash)
Browse files Browse the repository at this point in the history
  • Loading branch information
@zenparsing
zenparsing committed Oct 7, 2020
1 parent 6deaf22 commit 3515f74
Show file tree
Hide file tree
Showing 27 changed files with 206 additions and 224 deletions.
150 changes: 82 additions & 68 deletions browser/ui/webui/brave_tip_ui.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,17 @@ class TipMessageHandler : public WebUIMessageHandler,
const ledger::type::RewardsType type,
const ledger::type::ContributionProcessor processor) override;

private:
RewardsService* GetRewardsService();
void OnUnblindedTokensReady(RewardsService* rewards_service) override;

private:
// Message handlers
void DialogReady(const base::ListValue* args);
void GetPublisherBanner(const base::ListValue* args);
void GetRewardsParameters(const base::ListValue* args);
void OnTip(const base::ListValue* args);
void GetRecurringTips(const base::ListValue* args);
void GetReconcileStamp(const base::ListValue* args);
void TweetTip(const base::ListValue *args);
void TweetTip(const base::ListValue* args);
void GetOnlyAnonWallet(const base::ListValue* args);
void GetExternalWallet(const base::ListValue* args);
void FetchBalance(const base::ListValue* args);
Expand Down Expand Up @@ -118,17 +118,6 @@ TipMessageHandler::~TipMessageHandler() {
}
}

RewardsService* TipMessageHandler::GetRewardsService() {
if (!rewards_service_) {
Profile* profile = Profile::FromWebUI(web_ui());
rewards_service_ = RewardsServiceFactory::GetForProfile(profile);
if (rewards_service_) {
rewards_service_->AddObserver(this);
}
}
return rewards_service_;
}

void TipMessageHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback(
"dialogReady",
Expand Down Expand Up @@ -205,7 +194,7 @@ void TipMessageHandler::OnRecurringTipRemoved(
RewardsService* rewards_service,
bool success) {
if (!IsJavascriptAllowed()) {
return;
return;
}

FireWebUIListener("recurringTipRemoved", base::Value(success));
Expand All @@ -215,7 +204,7 @@ void TipMessageHandler::OnRecurringTipSaved(
RewardsService* rewards_service,
bool success) {
if (!IsJavascriptAllowed()) {
return;
return;
}

FireWebUIListener("recurringTipSaved", base::Value(success));
Expand All @@ -229,7 +218,7 @@ void TipMessageHandler::OnReconcileComplete(
const ledger::type::RewardsType type,
const ledger::type::ContributionProcessor processor) {
if (!IsJavascriptAllowed()) {
return;
return;
}

base::Value data(base::Value::Type::DICTIONARY);
Expand All @@ -239,39 +228,59 @@ void TipMessageHandler::OnReconcileComplete(
FireWebUIListener("reconcileCompleted", data);
}

void TipMessageHandler::OnUnblindedTokensReady(
RewardsService* rewards_service) {
if (!IsJavascriptAllowed()) {
return;
}

FireWebUIListener("unblindedTokensReady");
}

void TipMessageHandler::DialogReady(const base::ListValue* args) {
AllowJavascript();
if (auto* service = GetRewardsService()) {
if (service->IsInitialized()) {
FireWebUIListener("rewardsInitialized");
// Initialize rewards service pointer on first "dialogReady" message
if (!rewards_service_) {
Profile* profile = Profile::FromWebUI(web_ui());
rewards_service_ = RewardsServiceFactory::GetForProfile(profile);
if (rewards_service_) {
rewards_service_->AddObserver(this);
}
}
AllowJavascript();
if (rewards_service_ && rewards_service_->IsInitialized()) {
FireWebUIListener("rewardsInitialized");
}
}

void TipMessageHandler::GetOnlyAnonWallet(const base::ListValue* args) {
if (auto* service = GetRewardsService()) {
const bool only_anon = service->OnlyAnonWallet();
FireWebUIListener("onlyAnonWalletUpdated", base::Value(only_anon));
if (!rewards_service_) {
return;
}
const bool only_anon = rewards_service_->OnlyAnonWallet();
FireWebUIListener("onlyAnonWalletUpdated", base::Value(only_anon));
}

void TipMessageHandler::GetPublisherBanner(const base::ListValue* args) {
CHECK_EQ(1U, args->GetSize());
const std::string publisher_key = args->GetList()[0].GetString();

if (auto* service = GetRewardsService()) {
service->GetPublisherBanner(publisher_key, base::Bind(
&TipMessageHandler::GetPublisherBannerCallback,
weak_factory_.GetWeakPtr()));
if (publisher_key.empty() || !rewards_service_) {
return;
}

rewards_service_->GetPublisherBanner(publisher_key, base::Bind(
&TipMessageHandler::GetPublisherBannerCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::GetRewardsParameters(const base::ListValue* args) {
if (auto* service = GetRewardsService()) {
service->GetRewardsParameters(base::Bind(
&TipMessageHandler::GetRewardsParametersCallback,
weak_factory_.GetWeakPtr()));
if (!rewards_service_) {
return;
}

rewards_service_->GetRewardsParameters(base::Bind(
&TipMessageHandler::GetRewardsParametersCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::OnTip(const base::ListValue* args) {
Expand All @@ -280,32 +289,36 @@ void TipMessageHandler::OnTip(const base::ListValue* args) {
const double amount = args->GetList()[1].GetDouble();
const bool recurring = args->GetList()[2].GetBool();

if (publisher_key.empty()) {
if (publisher_key.empty() || !rewards_service_) {
return;
}

if (auto* service = GetRewardsService()) {
if (recurring && amount <= 0) {
service->RemoveRecurringTip(publisher_key);
} else if (amount >= 1) {
service->OnTip(publisher_key, amount, recurring);
}
if (recurring && amount <= 0) {
rewards_service_->RemoveRecurringTip(publisher_key);
} else if (amount >= 1) {
rewards_service_->OnTip(publisher_key, amount, recurring);
}
}

void TipMessageHandler::GetReconcileStamp(const base::ListValue *args) {
if (auto* service = GetRewardsService()) {
service->GetReconcileStamp(base::Bind(
&TipMessageHandler::GetReconcileStampCallback,
weak_factory_.GetWeakPtr()));
void TipMessageHandler::GetReconcileStamp(const base::ListValue* args) {
if (!rewards_service_) {
return;
}

rewards_service_->GetReconcileStamp(base::Bind(
&TipMessageHandler::GetReconcileStampCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::TweetTip(const base::ListValue *args) {
void TipMessageHandler::TweetTip(const base::ListValue* args) {
CHECK_EQ(args->GetSize(), 2U);
std::string name = args->GetList()[0].GetString();
const std::string name = args->GetList()[0].GetString();
const std::string tweet_id = args->GetList()[1].GetString();

if (name.empty() || !rewards_service_) {
return;
}

const std::string comment = l10n_util::GetStringFUTF8(
IDS_BRAVE_REWARDS_LOCAL_COMPLIMENT_TWEET,
base::UTF8ToUTF16(name));
Expand All @@ -316,40 +329,41 @@ void TipMessageHandler::TweetTip(const base::ListValue *args) {
std::map<std::string, std::string> share_url_args;
share_url_args["comment"] = comment;
share_url_args["hashtag"] = hashtag;
share_url_args["name"] = name.erase(0, 1);
share_url_args["name"] = name.substr(1);
share_url_args["tweet_id"] = tweet_id;

if (auto* service = GetRewardsService()) {
service->GetShareURL(
share_url_args,
base::BindOnce(
&TipMessageHandler::GetShareURLCallback,
base::Unretained(this)));
}
rewards_service_->GetShareURL(
share_url_args,
base::BindOnce(
&TipMessageHandler::GetShareURLCallback,
base::Unretained(this)));
}

void TipMessageHandler::FetchBalance(const base::ListValue* args) {
if (auto* service = GetRewardsService()) {
service->FetchBalance(base::BindOnce(
&TipMessageHandler::FetchBalanceCallback,
weak_factory_.GetWeakPtr()));
if (!rewards_service_) {
return;
}
rewards_service_->FetchBalance(base::BindOnce(
&TipMessageHandler::FetchBalanceCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::GetExternalWallet(const base::ListValue* args) {
if (auto* service = GetRewardsService()) {
service->GetUpholdWallet(base::BindOnce(
&TipMessageHandler::GetUpholdWalletCallback,
weak_factory_.GetWeakPtr()));
if (!rewards_service_) {
return;
}
rewards_service_->GetUpholdWallet(base::BindOnce(
&TipMessageHandler::GetUpholdWalletCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::GetRecurringTips(const base::ListValue *args) {
if (auto* service = GetRewardsService()) {
service->GetRecurringTips(base::BindOnce(
&TipMessageHandler::GetRecurringTipsCallback,
weak_factory_.GetWeakPtr()));
void TipMessageHandler::GetRecurringTips(const base::ListValue* args) {
if (!rewards_service_) {
return;
}
rewards_service_->GetRecurringTips(base::BindOnce(
&TipMessageHandler::GetRecurringTipsCallback,
weak_factory_.GetWeakPtr()));
}

void TipMessageHandler::GetRewardsParametersCallback(
Expand Down Expand Up @@ -398,7 +412,7 @@ void TipMessageHandler::GetRecurringTipsCallback(
void TipMessageHandler::GetPublisherBannerCallback(
ledger::type::PublisherBannerPtr banner) {
if (!IsJavascriptAllowed()) {
return;
return;
}

base::Value result(base::Value::Type::DICTIONARY);
Expand Down
10 changes: 3 additions & 7 deletions browser/ui/webui/brave_webui_source.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -818,8 +818,6 @@ void CustomizeWebUIHTMLSource(const std::string &name,
{ "currentMonthlyContribution", IDS_BRAVE_REWARDS_TIP_CURRENT_MONTHLY_CONTRIBUTION }, // NOLINT
{ "doMonthly", IDS_BRAVE_UI_DO_MONTHLY },
{ "errorHasOccurred", IDS_BRAVE_REWARDS_TIP_ERROR_HAS_OCCURRED },
{ "githubTipTitle", IDS_BRAVE_UI_GITHUB_TIP_TITLE },
{ "githubTipTitleEmpty", IDS_BRAVE_UI_GITHUB_TIP_TITLE_EMPTY },
{ "monthlyContribution", IDS_BRAVE_UI_MONTHLY_CONTRIBUTION },
{ "monthlyContributionSet", IDS_BRAVE_REWARDS_TIP_MONTHLY_CONTRIBUTION_SET }, // NOLINT
{ "monthlyText", IDS_BRAVE_UI_MONTHLY_TEXT },
Expand All @@ -830,8 +828,8 @@ void CustomizeWebUIHTMLSource(const std::string &name,
{ "oneTimeTip", IDS_BRAVE_REWARDS_TIP_ONE_TIME_TIP },
{ "oneTimeTipAmount", IDS_BRAVE_REWARDS_TIP_ONE_TIME_TIP_AMOUNT },
{ "points", IDS_BRAVE_UI_POINTS },
{ "redditTipTitle", IDS_BRAVE_UI_REDDIT_TIP_TITLE },
{ "redditTipTitleEmpty", IDS_BRAVE_UI_REDDIT_TIP_TITLE_EMPTY },
{ "postHeader", IDS_BRAVE_REWARDS_TIP_POST_HEADER },
{ "postHeaderTwitter", IDS_BRAVE_REWARDS_TIP_POST_HEADER_TWITTER },
{ "rewardsBannerText1", IDS_BRAVE_UI_REWARDS_BANNER_TEXT1 },
{ "sendDonation", IDS_BRAVE_UI_SEND_DONATION },
{ "siteBannerConnectedText", IDS_BRAVE_UI_SITE_BANNER_CONNECTED_TEXT },
Expand All @@ -845,9 +843,7 @@ void CustomizeWebUIHTMLSource(const std::string &name,
{ "tipPostSubtitle", IDS_BRAVE_REWARDS_TIP_TIP_POST_SUBTITLE },
{ "tokens", IDS_BRAVE_UI_TOKENS },
{ "tweetAboutSupport", IDS_BRAVE_REWARDS_TIP_TWEET_ABOUT_SUPPORT },
{ "tweetTipTitle", IDS_BRAVE_UI_TWEET_TIP_TITLE },
{ "tweetTipTitleEmpty", IDS_BRAVE_UI_TWEET_TIP_TITLE_EMPTY },
{ "unVerifiedTextMore", IDS_BRAVE_UI_SITE_UNVERIFIED_TEXT_MORE },
{ "unverifiedTextMore", IDS_BRAVE_UI_SITE_UNVERIFIED_TEXT_MORE },
{ "welcome", IDS_BRAVE_UI_WELCOME },
}
}, {
Expand Down
2 changes: 1 addition & 1 deletion components/brave_rewards/browser/test/common/rewards_browsertest_contribution.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ void RewardsBrowserTestContribution::TipPublisher(
rewards_browsertest_util::WaitForElementToContain(
site_banner_contents,
"body",
base::StringPrintf("%.0f BAT", amount));
base::StringPrintf("%.3f BAT", amount));
}

const bool is_monthly =
Expand Down
3 changes: 0 additions & 3 deletions components/brave_rewards/resources/tip/components/app.style.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import styled from 'styled-components'
export const root = styled.div`
display: flex;
flex-flow: row wrap;
font-family: var(--brave-font-heading);
font-size: 14px;
line-height: 22px;
Expand All @@ -32,7 +31,6 @@ export const form = styled.div`
position: relative;
flex: 1 0 364px;
min-height: 454px;
background: #fff;
`

Expand Down Expand Up @@ -62,6 +60,5 @@ export const error = styled.div`

export const errorDetails = styled.div`
margin-top: 10px;
font-size: 12px;
`
5 changes: 3 additions & 2 deletions components/brave_rewards/resources/tip/components/app.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ export function App () {
})

function onMount (element: HTMLElement | null) {
if (element) {
injectThemeVariables(element)
if (!element) {
return
}
injectThemeVariables(element)
}

return (
Expand Down
5 changes: 2 additions & 3 deletions components/brave_rewards/resources/tip/components/bat_string.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ import * as React from 'react'

import { HostContext } from '../lib/host_context'
import { LocaleContext } from '../lib/locale_context'
import { StringKey } from '../lib/interfaces'

interface Props {
stringKey?: StringKey
stringKey?: string
}

function mapOnlyAnonWalletKey (key: StringKey) {
function mapOnlyAnonWalletKey (key: string) {
switch (key) {
case 'bat': return 'bap'
case 'batFunds': return 'bapFunds'
Expand Down
6 changes: 3 additions & 3 deletions components/brave_rewards/resources/tip/components/button_switch.style.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@ export const option = styled.div`
button {
width: 100%;
padding: var(--button-switch-padding, 6px 0);
font-size: 12px;
line-height: 21px;
border-radius: 30px;
border: 1px solid rgba(115, 122, 222, 0.55);
background: #fff;
color: var(--brave-color-brandBatInteracting);
cursor: pointer;
outline: 0;
}
button:active {
button:active, button:focus {
background: var(--brave-color-brandBatActive);
color: var(--brave-color-brandBatInteracting);
}
&.selected button {
Expand All @@ -38,7 +39,6 @@ export const option = styled.div`

export const caption = styled.div`
margin-top: 5px;
text-align: center;
font-size: 12px;
line-height: 18px;
Expand Down
Loading

0 comments on commit 3515f74

Please sign in to comment.