-
Notifications
You must be signed in to change notification settings - Fork 859
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
Extension observer comparison code is now moved to front side #2233
Conversation
case types.ON_WALLET_CORRUPTED: | ||
state = { ...state } | ||
state.walletCorrupted = true | ||
if (result === 12) { // ledger::Result::WALLET_CREATED |
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.
Should these results be in some kind of enumeration?
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.
The problem I was seeing is that this result comes from an enum in Ledger. Currently we would have to keep track of making sure the Ledger enum and an enum here would stay in sync.
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.
we already do this and you have definition in rewards.d.ts
-> Rewards.Result
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.
Nice! Never saw that
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.
Sadly, per @NejcZdovc there is currently a problem using ts enum in reducer
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.
The enum declaration needs to be exported as const
in order to work properly. Done.
909a4ae
to
54e8e4e
Compare
components/brave_rewards/resources/extension/brave_rewards/actions/rewards_panel_actions.ts
Outdated
Show resolved
Hide resolved
components/brave_rewards/resources/extension/brave_rewards/background/events/rewardsEvents.ts
Outdated
Show resolved
Hide resolved
54e8e4e
to
e25358c
Compare
Updated enum declaration to const
e25358c
to
32dd80d
Compare
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
Fixes brave/brave-browser#3974
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:
Reviewer Checklist: