Replace Button with BrowserButton, set common font properties for BrowserButton, etc. #10951

luixxiul · 2017-09-14T09:44:32Z

Requires #11547
Blocks #10881

Fixes #10815
Closes #9223
Closes #11114
Closes #11115
Addresses #7168
Addresses #10898

Test Plans

1st commit

Addresses #7168

Test Plan 1:

Open about:autofill
Click 'Add Address' and 'Add Credit Card'
Make sure the buttons are displayed

Test Plan 2:

Open about:preferences#payments
Click the setting icon
Make sure the close button on the modal dialog is displayed

Test Plan 3:

Open https://webtorrent.io/free-torrents
Click any link to a torrent file
Click 'Start Download'
Make sure the button is changed to 'Stop Download'

Test Plan 4:

Open clear browsing data panel
Make sure there is margin between the buttons
Open import browser data panel
Repeat 2
Open http://browserspy.dk/password.php
Click a link to password-dk.php
Repeat 2
Open https://mixed-script.badssl.com/
Click the lock icon
Repeat 2
Disable Widevine
Open https://netflix.com to display the widevine panel
Repeat 2

2nd commit

Fixes #10815
Closes #9223

Test Plan:

Open about:preferences and display a button
Open data import dialog
Make sure the browser label is rendered in the same font properties

3rd commit

Closes #11114
Closes #11115
Addresses #10898

Test Plan:

Open about:styles
Click 'buttons'
Make sure cursor becomes pointer on hover
Open about:preferences#payments
Click the advanced setting
Make sure the close button of the modal overlay becomes pointer

Test Plan 2:

Open brave.com
Click the shield button
Make sure that the close button, info button and reload button has pointer cursor on hover

Test Plan 3:

Open about:preferences#plugins
Make sure the info buttons under Widevine support has pointer cursor on hover

Test Plan 4:

Open about:brave
Click copy button
Make sure the version information is copied

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.

Test Plan:

Reviewer Checklist:

Tests

Adequate test coverage exists to prevent regressions
Tests should be independent and work correctly when run individually or as a suite ref
New files have MPL2 license header

luixxiul · 2017-09-14T09:47:57Z

app/renderer/components/main/siteInfo.js

@@ -68,8 +68,7 @@ class SiteInfo extends React.Component {
        <span className={cx({
          fa: true,
          'fa-lock': true,
-          [css(styles.secureIcon__fa)]: true,
-          [css(styles.secureIcon__extendedValidation)]: this.props.isExtendedValidation
+          [css(styles.secureIcon__fa, this.props.isExtendedValidation && styles.secureIcon__extendedValidation)]: true


This doesn't follow the BEM strictly; It'll be fixed with #10933

luixxiul · 2017-09-14T09:54:16Z

app/renderer/components/download/downloadItem.js

+          {
+            this.props.allowPause
+            ? <BrowserButton
+              iconOnly


iconOnly rocks 🎸🤘

haha w00t 👨‍🎤 🎶

luixxiul · 2017-09-21T15:50:06Z

Requesting a review from @srirambv on his Windows environment (the 2nd commit).

cezaraugusto · 2017-09-22T05:19:12Z

app/renderer/components/common/button.js

@@ -1,51 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public


YESSSSSSSS YOU DID IT SIR

There are normal <button> still left, which will be replaced with BrowserButton :-)

cezaraugusto · 2017-09-22T05:20:13Z

app/renderer/components/common/modalOverlay.js

+    return <div className={css(
+      styles.dialog__wrapper_modal,
+      this.state.last && styles.dialog__wrapper_last,
+      this.props.transparentBackground && styles.dialog__wrapper_transparentBackground


luixxiul · 2017-09-22T05:24:00Z

Rebasing...

cezaraugusto · 2017-09-22T05:24:26Z

app/renderer/components/navigation/navigator.js

+  },
+
+  braveMenuContainer__braveShield_disabled: {
+    filter: 'grayscale(100%)',


grayscale ftw

@luixxiul does this also fix #9696?

I don't think so as this does not change the color of the number block.

I have a fix for that issue. Is it fine to commit it to this PR?

I could save it for another PR.

Sure- add it in 😄

Added :-) 8615b5f

cezaraugusto · 2017-09-23T21:56:49Z

keen to review this but delaying to after 0.19.x, given it has some changes ledger-related. Setting as PR/blocked for now to keep track

luixxiul · 2017-09-24T02:43:34Z

Until 0.19 is released, I'm going to add other commits to normalize the buttons! :-)

@cezaraugusto

Closes brave#11111 Requires brave#10951 to make the shield button non-draggable Auditors: @cezaraugusto Test Plan: 1. Open brave.com 2. Enable NoScript 3. Enable an extension 4. Make sure NoScript button, the extension button, and publisher button is not draggable

Closes #10946 Addresses #7168 Addresses #10947 Addresses #10948 - Add browserButton_smallItem for delete confirmation button on downloadItem.js - Refactor downloadItem with Aphrodite (partially) Auditors: Test Plan 1: 1. Make sure the brave shield icon is displayed 2. Open a new tab 3. Make sure the icon is light gray 4. Open http://example.com 5. Make sure the icon is orange 6. Disable the shield 7. Make sure the icon is dark gray Test Plan 2: 1. Open about:autofill 2. Click 'Add Address' and 'Add Credit Card' 3. Make sure the buttons are displayed Test Plan 3: 1. Open about:preferences#payments 2. Click the setting icon 3. Make sure the close button on the modal dialog is displayed Test Plan 4: 1. Download a file 2. Make sure the close button is displayed on the download bar 3. Hover on the file item 4. Make sure the action buttons are displayed 5. Click the trash icon 6. Make sure the delete confirmation button works Test Plan 5: 1. Open https://webtorrent.io/free-torrents 2. Click any link to a torrent file 3. Click 'Start Download' 4. Make sure the button is changed to 'Stop Download' Test Plan 6: 1. Open clear browsing data panel 2. Make sure there is margin between the buttons 3. Open import browser data panel 4. Repeat 2 5. Open http://browserspy.dk/password.php 6. Click a link to password-dk.php 7. Repeat 2 8. Open https://mixed-script.badssl.com/ 9. Click the lock icon 10. Repeat 2 11. Disable Widevine 11. Open https://netflix.com to display the widevine panel 12. Repeat 2

…n dialog Closes #9223 Fixes #10815 Auditors: Test Plan: 1. Open about:preferences and display a button 2. Open data import dialog 3. Make sure the browser label is rendered in the same font properties

Apply cursor:pointer to browserButton globally Replace span with onClick with browserButton Closes #11114 Closes #11115 Addresses #10898 Auditors: Test Plan: 1. Open about:styles 2. Click 'buttons' 3. Make sure cursor becomes pointer on hover 4. Open about:preferences#payments 5. Click the advanced setting 6. Make sure the close button of the modal overlay becomes pointer Test Plan 2: 1. Open brave.com 2. Click the shield button 3. Make sure that the close button, info button and reload button has pointer cursor on hover Test Plan 3: 1. Open about:preferences#plugins 2. Make sure the info buttons under Widevine support has pointer cursor on hover Test Plan 4: 1. Open about:brave 2. Click copy button 3. Make sure the version information is copied Test Plan 5: 1. Open about:history 2. Search something 3. Make sure the clear button on the input form works 4. Open about:bookmarks 5. Make sure the clear button works

cezaraugusto · 2017-12-19T15:10:11Z

is this font the right one? I think there are some efforts from design to change the actual font but not following so cc @petemill

luixxiul · 2018-01-17T16:31:37Z

I haven't checked the source code for a while, so let me close once to try to find a better fix, thanks.

luixxiul added misc/button priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). labels Sep 14, 2017

luixxiul added this to the 0.21.x (Nightly Channel) milestone Sep 14, 2017

luixxiul self-assigned this Sep 14, 2017

luixxiul requested a review from cezaraugusto September 14, 2017 09:44

luixxiul commented Sep 14, 2017

View reviewed changes

This was referenced Sep 14, 2017

Add browserButton_smallItem #10946

Closed

Refactor buttons with Aphrodite #7168

Closed

Refactor downloadItem.js with Aphrodite #10947

Closed

Refactor downloadsBar.js with Aphrodite #10948

Closed

luixxiul mentioned this pull request Sep 21, 2017

button style inconsistency on about:preferences (button normalization) #9223

Closed

luixxiul added the PR/needs-QA-attention ☕ label Sep 21, 2017

luixxiul mentioned this pull request Sep 21, 2017

Button text looks blurry on Windows #10815

Closed

luixxiul requested a review from srirambv September 21, 2017 15:49

luixxiul changed the title ~~Replace Button with BrowserButton~~ Replace Button with BrowserButton and set common font properties for BrowserButton Sep 21, 2017

luixxiul removed the priority/P5 Cosmetic. Spelling, copy, layout. New features (which should also be part of an initiative). label Sep 22, 2017

cezaraugusto reviewed Sep 22, 2017

View reviewed changes

luixxiul changed the title ~~Replace Button with BrowserButton and set common font properties for BrowserButton~~ Replace Button with BrowserButton, set common font properties for BrowserButton, etc. Sep 23, 2017

luixxiul mentioned this pull request Sep 23, 2017

browserButton should have -webkit-app-region: no-drag by default #11111

Closed

cezaraugusto added the PR/blocked label Sep 23, 2017

luixxiul added the PR/work-in-progress ⚒ label Sep 24, 2017

luixxiul mentioned this pull request Sep 24, 2017

Reorganize about pages and less files #10881

Closed

8 tasks

luixxiul added the feature/about-pages label Sep 24, 2017

bbondy modified the milestones: 0.20.x (Beta Channel), Backlog Oct 25, 2017

luixxiul added the QA/test-plan-specified label Oct 31, 2017

luixxiul added the PR/needs-rebase ۞ label Nov 7, 2017

luixxiul added PR/work-in-progress ⚒ PR/needs-rebase ۞ and removed PR/needs-rebase ۞ PR/work-in-progress ⚒ labels Nov 13, 2017

cezaraugusto removed the PR/blocked label Nov 21, 2017

luixxiul mentioned this pull request Nov 21, 2017

Apply BrowserButton to clipboardButton #11374

Merged

8 tasks

luixxiul added PR/work-in-progress ⚒ PR/blocked PR/needs-rebase ۞ and removed PR/needs-rebase ۞ PR/work-in-progress ⚒ feature/bookmarks labels Nov 21, 2017

Suguru Hirahara added 3 commits December 2, 2017 12:00

Set common font properties for BrowserButton on about pages and commo…

355a537

…n dialog Closes #9223 Fixes #10815 Auditors: Test Plan: 1. Open about:preferences and display a button 2. Open data import dialog 3. Make sure the browser label is rendered in the same font properties

luixxiul removed the PR/needs-rebase ۞ label Dec 2, 2017

cezaraugusto added the needs-info Another team member needs information from the PR/issue opener. label Dec 19, 2017

luixxiul closed this Jan 17, 2018

luixxiul removed this from the Triage Backlog milestone Jan 17, 2018

luixxiul removed their assignment Jan 28, 2018

luixxiul removed PR/blocked needs-info Another team member needs information from the PR/issue opener. labels Jan 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Button with BrowserButton, set common font properties for BrowserButton, etc. #10951

Replace Button with BrowserButton, set common font properties for BrowserButton, etc. #10951

luixxiul commented Sep 14, 2017 •

edited

Loading

luixxiul Sep 14, 2017 •

edited

Loading

luixxiul Sep 14, 2017 •

edited

Loading

cezaraugusto Sep 22, 2017

luixxiul commented Sep 21, 2017 •

edited

Loading

cezaraugusto Sep 22, 2017 •

edited

Loading

luixxiul Sep 22, 2017

cezaraugusto Sep 22, 2017

luixxiul commented Sep 22, 2017

cezaraugusto Sep 22, 2017

bsclifton Sep 22, 2017

luixxiul Sep 22, 2017

luixxiul Sep 22, 2017 •

edited

Loading

bsclifton Sep 22, 2017

luixxiul Sep 23, 2017

cezaraugusto commented Sep 23, 2017

luixxiul commented Sep 24, 2017

cezaraugusto commented Dec 19, 2017

luixxiul commented Jan 17, 2018

		@@ -1,51 +0,0 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public

Replace Button with BrowserButton, set common font properties for BrowserButton, etc. #10951

Replace Button with BrowserButton, set common font properties for BrowserButton, etc. #10951

Conversation

luixxiul commented Sep 14, 2017 • edited Loading

Test Plans

1st commit

2nd commit

3rd commit

luixxiul Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

luixxiul Sep 14, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luixxiul commented Sep 21, 2017 • edited Loading

cezaraugusto Sep 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luixxiul commented Sep 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luixxiul Sep 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cezaraugusto commented Sep 23, 2017

luixxiul commented Sep 24, 2017

cezaraugusto commented Dec 19, 2017

luixxiul commented Jan 17, 2018

luixxiul commented Sep 14, 2017 •

edited

Loading

luixxiul Sep 14, 2017 •

edited

Loading

luixxiul Sep 14, 2017 •

edited

Loading

luixxiul commented Sep 21, 2017 •

edited

Loading

cezaraugusto Sep 22, 2017 •

edited

Loading

luixxiul Sep 22, 2017 •

edited

Loading