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.

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

Separates monthly and one time tips in to two panels #3394

Merged
merged 1 commit into from
Sep 30, 2019
Merged

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Sep 10, 2019

Fixes: brave/brave-browser#5957

Screen Shot 2019-09-11 at 6 33 53 PM

Submitter Checklist:

Test Plan:

  1. Enable Rewards, Claim Grant
  2. Send a few tips, set some to monthly, the rest one-time
  3. Navigate to brave://rewards
  4. Confirm the monthly tips are in the Monthly Contributions box
  5. Confirm the one-time tips are in the tips box
  6. Confirm that the BAT totals between the boxes reflect the total of each tip type

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

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

cc: @jsecretan

@ryanml ryanml added this to the 0.72.x - Nightly milestone Sep 10, 2019
@ryanml ryanml requested a review from NejcZdovc September 10, 2019 02:15
@ryanml ryanml self-assigned this Sep 10, 2019
Copy link
Contributor Author

@ryanml ryanml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The old method for getting the total amount of tips used reports, which contain all tip types. We now just enumerate over recurringList and tipsList individually so each pane can have their own accurate total

@ryanml ryanml force-pushed the tip-split branch 2 times, most recently from 4438577 to 27c2f17 Compare September 10, 2019 03:11
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change Monthly Tips to Monthly Contribution everywhere in the code

@ryanml ryanml force-pushed the tip-split branch 2 times, most recently from 4409e78 to 23ef599 Compare September 12, 2019 01:47
@ryanml ryanml requested a review from NejcZdovc September 12, 2019 01:49
@ryanml ryanml dismissed NejcZdovc’s stale review September 12, 2019 01:50

Screenshot added, addressed feedback

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Title for modal needs new title. It should be Monthly Contributions and not Tips when you click see all in Monthly Contributions box

image

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

we are showing One time where in issue we only show date. We need to remove one time for tips box (for table and see all modal)

@ryanml ryanml force-pushed the tip-split branch 2 times, most recently from 8abe6b9 to 519e6a3 Compare September 26, 2019 18:24
@ryanml ryanml requested a review from NejcZdovc September 26, 2019 18:30
@ryanml ryanml dismissed NejcZdovc’s stale review September 26, 2019 18:31

'One Time' copy removed

@ryanml
Copy link
Contributor Author

ryanml commented Sep 30, 2019

@NejcZdovc this one is good to go - CI failure unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tips should be split in to two panels on brave://rewards
3 participants