Rewards extension in brave bar #431

mkarolin · 2018-09-11T16:34:20Z

Adds a placeholder for Rewards extension (the real extension code is to be implemented separately).
Places the extension into resource pack and provides necessary wiring to load the extension from resources. The loading of extension is predicated on BRAVE_REWARDS_ENABLED flag due to pending port of anonize.
Removes the Rewards extension button from main extensions area (ToolbarActionsBar / BrowserActionsContainer) and places it into BraveActionsBar in the omnibox.
Rewards extension button inherits behavior of BraveActionsBar buttons: disabled context menu, no drag&drop of buttons within the bar.
Renames ShieldsActionViewController to BraveActionViewController as the controller is now used for both Shields and Rewards action views. Due to renaming patches/chrome-browser-ui-extensions-extension_action_view_controller.h.patch had to be updated with the new name.

This PR should not be merged because it contains placeholder UI for Rewards extension.

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).
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/Needed or QA/No-QA-Needed) to include the closed issue in milestone

Test Plan:

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

petemill

This all looks and works great to me. I'm going to get an updated icon to put in as the vertical alignment is off. Even though it's a placeholder, I think this PR can own the look on the toolbar and then brave/brave-browser#920 can own the content area.

In the meantime we should ask @yrliou to review, who just completed similar work on an extension.

mkarolin · 2018-09-12T02:31:23Z

@petemill, thanks, I was going to ping Ross about the icons. If you are going to get them, should we add all sizes like we do for the shields extension (vendor/brave-extension/app/assets/img)? Please, let me know if you want me to follow up.

yrliou

Extension resources packing and loading looks good to me, a few small comments left, nothing major.

yrliou · 2018-09-12T16:49:50Z

browser/extensions/brave_component_loader.cc

@@ -35,6 +37,15 @@ void BraveComponentLoader::AddDefaultComponentExtensions(
    Add(IDR_BRAVE_EXTENSON, brave_extension_path);
  }

+#if BUILDFLAG(BRAVE_REWARDS_ENABLED) && !defined(OFFICIAL_BUILD)
+  if (!command_line.HasSwitch(switches::kDisableBraveRewardsExtension)) {


Do we have corresponding changes in brave-browser for adding this command line switch?

yrliou · 2018-09-12T16:51:20Z

browser/extensions/brave_component_loader.cc

@@ -35,6 +37,15 @@ void BraveComponentLoader::AddDefaultComponentExtensions(
    Add(IDR_BRAVE_EXTENSON, brave_extension_path);
  }

+#if BUILDFLAG(BRAVE_REWARDS_ENABLED) && !defined(OFFICIAL_BUILD)


Could we have a note here that !defined(OFFICIAL_BUILD) should be removed when we have some actual UI?

Will add a comment

yrliou · 2018-09-12T17:01:50Z

browser/resources/resource_ids

@@ -22,4 +22,7 @@
    "includes": [36500],
  },
  # brave webtorrent 37500
+  "brave/components/brave_rewards/resources.grd": {
+    "includes": [37600],


As discussed offline, let's do 38000 here, thanks!

yrliou · 2018-09-12T17:02:54Z

browser/ui/toolbar/brave_toolbar_actions_model.cc

+  // Don't show the Brave and Rewards 'extensions' in the ToolbarActions
+  // extensions area. They will instead be shown in the BraveActions area.
+  if (extension->id() == brave_extension_id || 
+	  extension->id() == brave_rewards_extension_id) {


nit: align with previous line?

yrliou · 2018-09-12T17:06:14Z

components/brave_rewards/extension/manifest.json

+  "browser_action": {
+    "default_popup": "extension/popup.html",
+    "default_icon": "extension/img/BAT_icon_placeholder_32.png"
+  },


extensions should have CSP defined, tho probably fine since it's still just a stub right now.

Right, my assumption is that whomever implements the actual extension will add the appropriate CSP.

Fixes #1103 Related: brave/brave-core#431 You can do this with: ``` npm run start -- --disable_brave_rewards_extension ``` or ``` yarn start --disable_brave_rewards_extension ```

petemill · 2018-09-13T02:13:10Z

@mkarolin here's some updated icons for the extension if we want to get that in here before it's merged https://www.dropbox.com/s/370cc0dtowvh45p/brave-payments-extension-icon.zip?dl=0

Additionally, it would be great to squash some of these commits so that anything fixed or any changes in response to reviews are squashed in to the commit that the change originated in.

…sources.

mkarolin · 2018-09-13T13:31:10Z

Updated with actual extension icons. Squashed code review commits.

petemill

Really nice - works perfectly! 🎉

Rewards extension in brave bar

petemill · 2018-09-13T16:41:10Z

0.55.x 45cc8b3

bbondy · 2018-09-14T17:15:42Z

added merged/0.55.x label

Revert "add toggle for enable_distro_version_check"

Fixes #1103 Related: #431 You can do this with: ``` npm run start -- --disable_brave_rewards_extension ``` or ``` yarn start --disable_brave_rewards_extension ```

Revert "add toggle for enable_distro_version_check"

Fixes #1103 Related: #431 You can do this with: ``` npm run start -- --disable_brave_rewards_extension ``` or ``` yarn start --disable_brave_rewards_extension ```

mkarolin added the pr/work-in-progress label Sep 11, 2018

mkarolin requested review from petemill and bsclifton September 11, 2018 16:34

bsclifton assigned mkarolin Sep 11, 2018

mkarolin changed the title ~~WIP: Rewards extension in brave bar~~ Rewards extension in brave bar Sep 11, 2018

mkarolin removed pr/work-in-progress labels Sep 11, 2018

petemill reviewed Sep 12, 2018

View reviewed changes

petemill requested a review from yrliou September 12, 2018 01:32

yrliou reviewed Sep 12, 2018

View reviewed changes

mkarolin added 2 commits September 13, 2018 09:09

Added Brave Rewards extension placeholder and code to load it from re…

08df357

…sources.

Moved Rewards Extension button to BraveActionsContainer

e4149ec

mkarolin force-pushed the maxk-rewards-extension-in-brave-bar branch from e2a1a43 to e4149ec Compare September 13, 2018 13:28

petemill approved these changes Sep 13, 2018

View reviewed changes

petemill merged commit e8468ee into master Sep 13, 2018

petemill deleted the maxk-rewards-extension-in-brave-bar branch September 13, 2018 16:40

petemill added a commit that referenced this pull request Sep 13, 2018

Merge pull request #431 from brave/maxk-rewards-extension-in-brave-bar

45cc8b3

Rewards extension in brave bar

bbondy added the merged/0.55.x label Sep 14, 2018

bbondy added this to the 0.55.x - Release milestone Jan 14, 2019

bsclifton mentioned this pull request Jul 4, 2019

The UI should allow for advanced customization brave/brave-browser#774

Open

fmarier pushed a commit that referenced this pull request Oct 29, 2019

Merge pull request #431 from brave/revert-306-distro-check

9c7ebce

Revert "add toggle for enable_distro_version_check"

bsclifton mentioned this pull request Jan 13, 2020

Shields should be visible when Brave is used in PWA context brave/brave-browser#7715

Open

petemill pushed a commit that referenced this pull request Jul 27, 2020

Merge pull request #431 from brave/revert-306-distro-check

571ea42

Revert "add toggle for enable_distro_version_check"

petemill pushed a commit that referenced this pull request Jul 28, 2020

Merge pull request #431 from brave/revert-306-distro-check

12690f0

Revert "add toggle for enable_distro_version_check"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewards extension in brave bar #431

Rewards extension in brave bar #431

mkarolin commented Sep 11, 2018 •

edited by NejcZdovc

Loading

petemill left a comment

mkarolin commented Sep 12, 2018

yrliou left a comment

yrliou Sep 12, 2018

yrliou Sep 12, 2018

mkarolin Sep 12, 2018

yrliou Sep 12, 2018

yrliou Sep 12, 2018

yrliou Sep 12, 2018

mkarolin Sep 12, 2018

petemill commented Sep 13, 2018

mkarolin commented Sep 13, 2018

petemill left a comment

petemill commented Sep 13, 2018

bbondy commented Sep 14, 2018

Rewards extension in brave bar #431

Rewards extension in brave bar #431

Conversation

mkarolin commented Sep 11, 2018 • edited by NejcZdovc Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

petemill left a comment

Choose a reason for hiding this comment

mkarolin commented Sep 12, 2018

yrliou left a comment

Choose a reason for hiding this comment

yrliou Sep 12, 2018

Choose a reason for hiding this comment

yrliou Sep 12, 2018

Choose a reason for hiding this comment

mkarolin Sep 12, 2018

Choose a reason for hiding this comment

yrliou Sep 12, 2018

Choose a reason for hiding this comment

yrliou Sep 12, 2018

Choose a reason for hiding this comment

yrliou Sep 12, 2018

Choose a reason for hiding this comment

mkarolin Sep 12, 2018

Choose a reason for hiding this comment

petemill commented Sep 13, 2018

mkarolin commented Sep 13, 2018

petemill left a comment

Choose a reason for hiding this comment

petemill commented Sep 13, 2018

bbondy commented Sep 14, 2018

mkarolin commented Sep 11, 2018 •

edited by NejcZdovc

Loading