Add new tip dialog user interface #6626

zenparsing · 2020-09-11T16:02:41Z

Resolves brave/brave-browser#11361
Resolves brave/brave-browser#11393

Submitter Checklist:

Test Plan:

(All tests should be executed for both verified and unverified publishers.)

Case 1: One-time tipping

Navigate to a publisher page.
Open the Rewards panel and click "Send a tip".
Verify that banner matches designs for one time tip flow.
- NOTE: Credit card option is not currently displayed.
Verify banner functionality (see below).
Verify tipping options:
- If custom one-time tipping options have been set for the publisher, then those amounts are displayed
- Otherwise, a default set of amounts are displayed.
Verify that "add funds" link is displayed if the user does not have sufficient BAT in their wallet.
- The link should open the Rewards page
- NOTE: The Rewards page will not automatically open a modal; this will be addressed in Implement earn and give onboarding flow brave-browser#12138
Select a valid amount and click "Send Tip".
Verify tip completion display matches designs.
Verify that tip was successfully sent.
Click the "tweet" button.
Verify that a new tab is opened to twitter with the correct messaging.

Case 2: Setting a monthly contribution

Ensure that a monthly contribution has not been set for a publisher.
Open the Rewards panel and click the monthly contribution "Set" button.
Verify that "Monthly" is the selected tip kind.
Verify that monthly tip options are displayed.
Select a tip option and click the submit button.
Verify that tip completion display matches designs.
Verify that monthly contribution has been correctly set.

Case 3: One-time tipping with existing monthly contribution

Ensure that a monthly contribution has been set for a publisher.
Open the Rewards panel and click "Send a tip".
Verify that an asterisk is displayed next to the "Monthly" option and the correct messaging is displayed about the "tip kind" selector.
Click the "Monthly" option.
Verify that "cancel/change amount" display matches designs.
Verify that the asterisk and corresponding messaging has been hidden.
Test "cancel" flow:
- Click the "Cancel" button.
- Verify that the confirmation display matches designs.
- Click the "Confirm" button.
- Verify that display matches design.
- Verify that monthly contribution has been removed.
Test "change amount" flow:
- Click the "Change amount" button.
- Verify that monthly tip options are displayed.
- Select a tip option and click the submit button.
- Verify that tip completion display matches designs.
- Verify that monthly contribution has been removed.

Case 3: Inline tipping

Navigate to a verified publisher page on twitter.
Click an inline tipping button.
Verify that dialog matches designs for inline tipping flow.
Verify that social media post is correctly displayed.
Verify that monthly tipping option is not displayed.
Verify that one time tip is successfully processed.

Banner functionality:

Background image:
- Custom banner is displayed if available
- Otherwise, a default background is displayed
- NOTE: Custom banner images may appear overly cropped; see Improve display of custom background images in tip dialog brave-browser#12057
Logo:
- Custom logo is displayed if available
- Otherwise, the first letter of the publisher name is displayed
- If the publisher is verified or connected, a checkmark is displayed
- Otherwise, an x-mark is displayed
Unverified notice:
- Unverified notice is displayed if the publisher is not verified.
Title (site tipping only):
- If a custom title has been set, the custom title is displayed
- Otherwise, a default title (e.g. "Welcome!") is displayed.
Description (site tipping only):
- If a custom message has been set, the custom message is displayed
- Otherwise, a default message is displayed.
Social media post (twitter/reddit inline tipping only):
- The social media post text is displayed in a box.
- The post date or relative time is displayed in the box header.
- The correct social media icon is displayed in the box header.

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.

browser/ui/webui/brave_tip_ui.cc

petemill

For events, please call FireWebUIListener('event-name', data). We should not add any more places where we hope JS function names exist at the time.

In JS, you would subscribe to the event with

window.cr.addWebUIListener('event-name', function(data) { ... })

Alternatively just give this WebUI access to the chrome.braveRewards API since we're not showing this page on Android.

components/brave_rewards/resources/tip_dialog/components/app.style.ts

components/webpack/webpack.config.js

browser/ui/webui/brave_tip_ui.cc

components/brave_rewards/resources/tip/components/one_time_tip_form.tsx

components/brave_rewards/resources/tip/components/app.style.ts

components/brave_rewards/resources/tip/components/publisher_banner.tsx

components/brave_rewards/resources/tip/lib/host.ts

components/brave_rewards/resources/tip/lib/interfaces.ts

NejcZdovc

Probably would be good to add index.tsx in components folder so that we can just do this

import { PublisherBanner, TipForm } from './index'

instead of

import { PublisherBanner } from './publisher_banner'
import { TipForm } from './tip_form'

we should add OnUnblindedTokensReady observer, so that if you have banner open and you claim promotion in another tab we fetch balance in open banner
probably this should be 3 decimal points as well based on the fact that we have BAT with 3 points in every other place
page favicon is not shown for verified publisher
border shouldn't be shown
3 deciaml points for the amount
X here should probably just switch back to defaul tip screen and not hide banner
probably it would be good to add some shadow/border around logo
remove duplicated username in the title for inline tipping. It's listed bellow as well and we don't have it regular tip flow

browser/ui/webui/brave_tip_ui.cc

components/brave_rewards/resources/tip/components/app.style.ts

components/brave_rewards/resources/tip/assets/background_2.svg

components/brave_rewards/resources/tip/assets/background_4.svg

components/brave_rewards/resources/tip/assets/background_3.svg

components/brave_rewards/resources/tip/assets/background_5.svg

components/brave_rewards/resources/tip/components/app.tsx

components/brave_rewards/resources/tip/components/monthly_tip_form.tsx

zenparsing · 2020-10-08T17:26:48Z

Working on one more potential design change to better deal with custom banner backgrounds that are different sizes.

NejcZdovc

clicking add funds doesn't do anything and return error in console [3005:775:1012/145517.242542:ERROR:CONSOLE(0)] "Not allowed to load local resource: brave://rewards/#add-funds", source: chrome://tip/ (0)

is not working correctly on master either. Even though that it opens new window in which you can see rewards page nothing happens. Should be fixed with new flow for ads opt in that will be part of follow up

petemill · 2020-10-12T19:49:09Z

components/brave_rewards/resources/tip/lib/theme_loader.ts

+export function injectThemeVariables (element: HTMLElement) {
+  for (const [key, value] of Object.entries(defaultTheme.color)) {
+    element.style.setProperty(`--brave-color-${key}`, String(value))
+  }


This is great.

Could you put it in components/common/ so that we can use it for other UI. I for certain would.

I'm not sure that we should be using defaultTheme directly. We should be ascertaining what the current theme is based on whether we're in light or dark mode, and then switching the theme (and replacing the variable values) when the user selected theme changes. We used to have elaborate listeners, but I think these days we can use window.matchMedia('(prefers-color-scheme: dark)') and put a listener on that. The idea is that you can either use the default dark and light themes, or subclass them and make specific overrides for each for your specific UI, if applicable.

Moving this out so that other UIs can use it was part of the plan (hope?), but I think we should do it as a follow-on, so that we'll have a bit more time to think about how to generalize it properly. We should be able to easily migrate the code in here to use a helper that we promote to "common".

Really interesting point about matchMedia!

zenparsing · 2020-10-13T02:56:33Z

@petemill I've updated the "tip completed" animation gif - it's been reduced from 1.4MB to ~600kb.

zenparsing · 2020-10-13T14:51:59Z

CI failure on "SonarCloud Code Analysis" is due to the tip dialog HTML page not have a "lang" attribute on the html tag, but the "lang" attribute is set correctly for WebUI via:

<html i18n-values="dir:textdirection;lang:language">

emerick

LGTM, one minor nit.

components/resources/brave_components_strings.grd

petemill

This is awesome. Thanks for bringing some new ideas to code organization and both state and API management. Looking forward to sharing and evolving some of these.

Just some minor accessibility feedback.
And needs a test plan.

But 1 issue that's preventing me approving is that nothing is happening when I click "Add funds" if I try to tip an amount higher than I have.

Clean profile
Visit petemill.com (or any site that has 100BAT option $$$)
Choose 100BAT
Click Add Funds
Observe nothing happens

components/brave_rewards/resources/tip/components/slider_switch.style.ts

components/brave_rewards/resources/tip/components/form_submit_button.tsx

components/brave_rewards/resources/tip/components/icons/unverified_icon.tsx

- Gif bundling is required for updated rewards tip dialog designs. - Webpack was previously bundling a polyfill for Node's stream module due to a detected dependency in "styled-components". - Removed unused variables.

NejcZdovc · 2020-10-15T20:25:42Z

@petemill add funds on master doesn't do anything either. This will be fixed in brave/brave-browser#12138 when we add new flows

zenparsing · 2020-10-15T22:14:44Z

@petemill The "add funds" link was pointed to a "brave://" url which was blocked. Changing it to a "chrome://" url works and matches the behavior on master (it opens the rewards page).

petemill

Nice work. Thanks for addressing feedback.

zenparsing added the feature/rewards label Sep 11, 2020

zenparsing requested review from bridiver and NejcZdovc as code owners September 11, 2020 16:02

zenparsing self-assigned this Sep 11, 2020

zenparsing force-pushed the ksmith-tip-dialog branch 3 times, most recently from 5d33e22 to c0eeb37 Compare September 11, 2020 20:12

petemill reviewed Sep 16, 2020

View reviewed changes

browser/ui/webui/brave_tip_ui.cc Outdated Show resolved Hide resolved

petemill requested changes Sep 16, 2020

View reviewed changes

petemill reviewed Sep 16, 2020

View reviewed changes

components/brave_rewards/resources/tip_dialog/components/app.style.ts Outdated Show resolved Hide resolved

petemill reviewed Sep 16, 2020

View reviewed changes

components/webpack/webpack.config.js Show resolved Hide resolved

zenparsing force-pushed the ksmith-tip-dialog branch 14 times, most recently from 7f42a7d to e7ec8b9 Compare September 28, 2020 03:40

zenparsing force-pushed the ksmith-tip-dialog branch from e7ec8b9 to 5b39c4b Compare September 28, 2020 14:20

NejcZdovc requested a review from emerick September 28, 2020 16:56

zenparsing requested a review from petemill September 28, 2020 20:45

emerick reviewed Sep 28, 2020

View reviewed changes

NejcZdovc suggested changes Oct 1, 2020

View reviewed changes

zenparsing force-pushed the ksmith-tip-dialog branch from 1cb2046 to 3515f74 Compare October 7, 2020 21:21

emerick reviewed Oct 8, 2020

View reviewed changes

components/brave_rewards/resources/tip/components/monthly_tip_form.tsx Show resolved Hide resolved

zenparsing force-pushed the ksmith-tip-dialog branch from 3515f74 to b6bdd6f Compare October 8, 2020 16:13

NejcZdovc requested review from emerick and NejcZdovc October 12, 2020 12:39

NejcZdovc previously requested changes Oct 12, 2020

View reviewed changes

NejcZdovc approved these changes Oct 12, 2020

View reviewed changes

NejcZdovc added this to the 1.17.x - Nightly milestone Oct 12, 2020

zenparsing force-pushed the ksmith-tip-dialog branch from b6bdd6f to 74409c2 Compare October 12, 2020 17:10

petemill reviewed Oct 12, 2020

View reviewed changes

zenparsing force-pushed the ksmith-tip-dialog branch from 74409c2 to a103c5a Compare October 13, 2020 02:52

emerick approved these changes Oct 13, 2020

View reviewed changes

components/resources/brave_components_strings.grd Outdated Show resolved Hide resolved

zenparsing force-pushed the ksmith-tip-dialog branch 2 times, most recently from 1abac91 to cb1324e Compare October 13, 2020 20:45

petemill requested changes Oct 15, 2020

View reviewed changes

Update webpack to bundle gifs and skip polyfill for Node streams

7da1848

- Gif bundling is required for updated rewards tip dialog designs. - Webpack was previously bundling a polyfill for Node's stream module due to a detected dependency in "styled-components". - Removed unused variables.

Implement tip dialog redesign

31e2a47

zenparsing force-pushed the ksmith-tip-dialog branch from cb1324e to 31e2a47 Compare October 15, 2020 22:10

zenparsing requested a review from a team as a code owner October 15, 2020 22:10

zenparsing requested a review from petemill October 15, 2020 22:14

petemill approved these changes Oct 16, 2020

View reviewed changes

zenparsing merged commit a86d84f into master Oct 16, 2020

zenparsing deleted the ksmith-tip-dialog branch October 16, 2020 14:26

LaurenWags mentioned this pull request Nov 9, 2020

[Desktop] Implement design for tipping banner brave/brave-browser#11393

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new tip dialog user interface #6626

Add new tip dialog user interface #6626

zenparsing commented Sep 11, 2020 •

edited

Loading

petemill left a comment

NejcZdovc left a comment

zenparsing commented Oct 8, 2020

NejcZdovc left a comment

petemill Oct 12, 2020

zenparsing Oct 12, 2020

zenparsing commented Oct 13, 2020

zenparsing commented Oct 13, 2020

emerick left a comment

petemill left a comment •

edited

Loading

NejcZdovc commented Oct 15, 2020

zenparsing commented Oct 15, 2020

petemill left a comment

Add new tip dialog user interface #6626

Add new tip dialog user interface #6626

Conversation

zenparsing commented Sep 11, 2020 • edited Loading

Submitter Checklist:

Test Plan:

Case 1: One-time tipping

Case 2: Setting a monthly contribution

Case 3: One-time tipping with existing monthly contribution

Case 3: Inline tipping

Banner functionality:

Reviewer Checklist:

After-merge Checklist:

petemill left a comment

Choose a reason for hiding this comment

NejcZdovc left a comment

Choose a reason for hiding this comment

zenparsing commented Oct 8, 2020

NejcZdovc left a comment

Choose a reason for hiding this comment

petemill Oct 12, 2020

Choose a reason for hiding this comment

zenparsing Oct 12, 2020

Choose a reason for hiding this comment

zenparsing commented Oct 13, 2020

zenparsing commented Oct 13, 2020

emerick left a comment

Choose a reason for hiding this comment

petemill left a comment • edited Loading

Choose a reason for hiding this comment

NejcZdovc commented Oct 15, 2020

zenparsing commented Oct 15, 2020

petemill left a comment

Choose a reason for hiding this comment

zenparsing commented Sep 11, 2020 •

edited

Loading

petemill left a comment •

edited

Loading