Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Notification badge shows on first run when rewards is not enabled #764

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Notification badge shows on first run when rewards is not enabled #764

merged 1 commit into from
Oct 31, 2018

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented Oct 30, 2018 •
edited by NejcZdovc
Loading

Fixes brave/brave-browser#1439

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

  1. Start Brave with clean profile
  2. Verify badge notification appears on BAT icon
  3. Close Brave and reopen.
  4. Verify badge still appears on BAT icon
  5. Click on BAT icon
  6. Verify badge notification goes away
  7. Close Brave and reopen.
  8. Verify badge notification does not appear
  1. Start Brave with clean profile
  2. Verify badge notification appears on BAT icon
  3. Enable Rewards
  4. Verify badge notification goes away (will be replaced by grant notification [look for slight flicker on badge])
  5. Close Brave and reopen.
  6. Verify badge notification is still set to 1 and is for grant

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

@jasonrsadler jasonrsadler self-assigned this Oct 30, 2018
@NejcZdovc
Copy link
Contributor

@jasonrsadler can you please fix linter

@jasonrsadler
Copy link
Contributor Author

Linter fixed

Notification badge shows on first run when rewards is not enabled
d8cf9dd 
fixes brave/brave-browser#1439

Fixing linter

Updated notification badge to clear on wallet create

Fix spacing
@NejcZdovc NejcZdovc merged commit a6f7ead into brave:master Oct 31, 2018
@NejcZdovc
Copy link
Contributor

master (0.58.x) a6f7ead

@jasonrsadler jasonrsadler deleted the opt-in-notification branch October 31, 2018 11:07
@LaurenWags LaurenWags mentioned this pull request Dec 11, 2018
Closed
@bbondy bbondy added this to the 0.58.x - Release milestone Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NejcZdovc NejcZdovc NejcZdovc approved these changes

Assignees

@jasonrsadler jasonrsadler

Labels
feature/rewards
Projects
None yet
Milestone
0.58.x - Release
Development

Successfully merging this pull request may close these issues.

initial notification to try Brave Rewards if user is not opted in
3 participants
@jasonrsadler @NejcZdovc @bbondy