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

Adding Brave Header component #2

Merged
merged 1 commit into from Jun 15, 2019
Merged

Adding Brave Header component #2

merged 1 commit into from Jun 15, 2019

Conversation

@ryanml
Copy link
Member

ryanml commented Jun 6, 2019

This pull request does a few things ahead of the initial packaging in to the browser:



  • Adds a Brave Header toolbar component for parity with other system pages

  • Updates the page title to Brave Wallet
  • Removes the custom extension popup (we don't need this)

A consideration to make about the extension popup is that even if this isn't specified, the browser will still add a grayed out icon to the toolbar for visibility (sec issue, we will probably have to override this from within chromium)

Screen Shot 2019-06-06 at 11 06 28 AM


Screen Shot 2019-06-06 at 11 04 37 AM


Screen Shot 2019-06-06 at 10 57 25 AM

@ryanml ryanml requested review from diracdeltas and bbondy Jun 6, 2019
@ryanml ryanml self-assigned this Jun 6, 2019
@diracdeltas
Copy link
Member

diracdeltas commented Jun 6, 2019

can't comment on design, but this lgtm security-wise

@ryanml ryanml force-pushed the toolbar-updates branch 3 times, most recently from c3b0166 to e6fa25a Jun 9, 2019
app/manifest.json Outdated Show resolved Hide resolved
@@ -0,0 +1,100 @@
.br-toolbar {

This comment has been minimized.

Copy link
@bbondy

bbondy Jun 11, 2019

Member

Can we have things in a single top level brave folder with the same duplicated hierarchy?

ui/app/pages/routes/index.js Outdated Show resolved Hide resolved
@@ -3,7 +3,7 @@
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1 user-scalable=no">
<title>MetaMask</title>
<title>Brave Wallet</title>

This comment has been minimized.

Copy link
@bbondy

bbondy Jun 11, 2019

Member

I think we can do this with webpack as part of the branding too. I think you can split this and the manifest.json out of this PR.

@bbondy bbondy force-pushed the master branch from b478847 to acc9e1e Jun 11, 2019
@ryanml ryanml force-pushed the toolbar-updates branch 2 times, most recently from f22f488 to 7f7b483 Jun 14, 2019
@ryanml ryanml force-pushed the toolbar-updates branch from 7f7b483 to 64ff473 Jun 14, 2019
@ryanml ryanml requested a review from bbondy Jun 14, 2019
@bbondy
bbondy approved these changes Jun 15, 2019
@ryanml ryanml merged commit adfcd62 into master Jun 15, 2019
@ryanml ryanml deleted the toolbar-updates branch Jun 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.