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

Adds new Monthly Banner Variant to tipping process #3429

Merged
merged 1 commit into from
Sep 26, 2019
Merged

Adds new Monthly Banner Variant to tipping process #3429

merged 1 commit into from
Sep 26, 2019

Conversation

ryanml
Copy link
Contributor

@ryanml ryanml commented Sep 13, 2019

Fixes: brave/brave-browser#5996

Screen Shot 2019-09-13 at 3 13 14 PM

Screen Shot 2019-09-13 at 3 46 39 PM

Submitter Checklist:

Test Plan:

Defined in issue

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.

@ryanml ryanml self-assigned this Sep 13, 2019
@ryanml ryanml force-pushed the fix-5996 branch 2 times, most recently from a735a3d to 79500df Compare September 13, 2019 22:43
@ryanml ryanml marked this pull request as ready for review September 13, 2019 22:43
@ryanml ryanml added this to the 0.72.x - Nightly milestone Sep 13, 2019
@ryanml ryanml force-pushed the fix-5996 branch 2 times, most recently from b770dcf to 5eee878 Compare September 13, 2019 22:59
masparrow
masparrow previously approved these changes Sep 17, 2019
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.

  1. spacing at the bottom is soo small

Screen Shot 2019-09-18 at 7 55 04 AM

  1. can we remove scrollbar that is now visible
    image

  2. clicking on contribution date in the banner shouldn't trigger button click

  3. when you click on select 0 BAT is always selected and not your actual amount
    image

  4. send a tip is not working

[56385:775:0918/075850.531083:ERROR:CONSOLE(0)] "Error handling response: TypeError: Error in invocation of braveRewards.tipSite(integer tabID, string publisherKey, boolean monthly): No matching signature.
    at chrome-extension://jidkidbbcafjabdphckchenhfomhnfma/out/brave_rewards_panel.bundle.js:1656:37", source: chrome-extension://jidkidbbcafjabdphckchenhfomhnfma/brave_rewards_panel.html (0)

@ryanml ryanml force-pushed the fix-5996 branch 2 times, most recently from 0d083bd to 7f17dce Compare September 18, 2019 18:23
@ryanml
Copy link
Contributor Author

ryanml commented Sep 18, 2019

@NejcZdovc

  1. Bottom padding increased
    Screen Shot 2019-09-18 at 11 33 22 AM

  2. I could not reproduce a scrollbar, but I tried to tighten up padding a little. Truth is this is another element added in the panel and unless we increase the max height of the extension window this will happen, cc: @petemill for thoughts, I think we are fine for this PR though

  3. I could not reproduce this, can you confirm that you have the latest hash of brave-ui installed for this PR?

  4. This problem is currently present on release build and all other channels, I will file an issue and fix separately.

  5. This issue has been fixed

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.

we need to polish tip banner as spacing is off a little bit, but this PR looks good

@ryanml ryanml merged commit f76d60d into master Sep 26, 2019
@ryanml ryanml deleted the fix-5996 branch September 26, 2019 17:38
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.

Add new Monthly Contribution banner for publishers
3 participants