Refactor navigationBar with Aphrodite - first try #9299

luixxiul · 2017-06-07T07:27:28Z

Refactor navigationBar with Aphrodite

Addresses #9283

After first review I ~~will~~ did squash commits semantically, before switching the base branch from navigationBar-aphrodite to master. It will likely to create conflicts, so after fixing them I would like to ask @NejcZdovc for another review to check if it would not regress the recent Redux updates which have landed on master already.

Test Plans

Test Plan (obsolete styles)

Search inputbar-wrapper on your repo
Search navbarMenubarBlockContainer
Search your repo for @siteEVColor
Search your repo for @loadTimeColor
Make sure nothing is found

Test Plan (automated tests):

npm run test -- --grep='enables back button on first nav'
npm run test -- --grep='back forward actions'
npm run test -- --grep='loading same URL as current page with changed input'

Since automated tests cannot find visual regressions, please conduct these tests carefully from perspectives of clickability, draggability, animation, color, etc. Thanks!

Test Plan (top panel):

Open about:preferences#advanced
Enable Always show the URL bar
Make sure the navigation bar is displayed, aligned horizontally
Make sure the URL bar is displayed
Make sure the buttons on the navigation bar are displayed
Input something on the URL bar
Make sure the URL bar is highlighted
Set focus to input area on URL bar
Make sure the blue outline is displayed
Make sure margin-right:3px is applied
Make sure border-radius:0 is applied

Test Plan (add bookmark icon)

Make sure clicking the star icon bookmarks a page
Make sure the bookmark star icon is filled
Remove the bookmark
Make sure bookmark star icon becomes blank

Test Plan (buttons inside the top panel):

Open about:preferences#advanced
Enable Always show the URL bar
Open https://brave.com
Make sure both bookmark button and add publisher button are rendered
Click the bookmark star button container
Make sure the page is bookmarked
Remove the bookmark
Make sure bookmark star icon becomes blank
Make sure the whole container of the publisher button is clickable (click the edge of the button)

Test Plan (Publisher Toggle):

Disable Brave Payments
Open brave.com
Make sure the empty container does not appear on the right of the URL bar

Test Plan (navigation buttons):

Make sure go back and forward button do not have the background and box-shadow when they are disabled
Click the edge of the reload button
Make sure the button is clicked
Click the edge of the load stop button
Make sure the button is clicked
Show the home button
Click the edge of the button
Make sure your home page is loaded

Test Plan (fullscreen mode)

Enable the fullscreen mode
Make sure the navigation buttons (go back/forward) are aligned to left

Test Plan (elements around the URL bar):

Move your mouse pointer out of the top panel
Make sure .urlbarForm does not have a white background
Open about:preferences#advanced
Disable the wide URL bar
Change window width
Make sure there is margin between the URL bar and the lion icon
Make sure the URL bar appears at the center on the navigation bar

Test plan (loadTime)

Open https://brave.com
Click the loading timer
Make sure the loading timer is displayed
Make sure the loading timer cannot be selected

Test Plan (URL bar un/lock icon)

Enter data:text/html,hi in the URL bar (ee79fc7)
Make sure the triangle is red
Open https://brave.com
Make sure the fa-lock icon is green
Open a new dashboard
Make sure the search icon is grey
Disable Always show the URL bar if not
Open http://http.badssl.com/
Make sure the unlock icon is not transparent in the title mode
Make sure the urlbar icon (fa-un/lock or fa-search) is placed at the center of the icon's container
Input :a to the URL bar
Make sure styles.urlbarIcon is applied to the amazon favicon
Move your mouse pointer out of the top panel
Make sure the .urlbarForm does not have a white backgroundf
Open about:preferenes
Make sure the fa-list icon is moved down 1px on URL bar

Test Plan (NoScript):

Block scripts
Make sure NoScript icon on the URL bar is clickable
Make sure the icon is large enough to be clicked easily
Make sure the un/lock icon, bookmark icon, NoScript icon, and publisher button are aligned horizontally
Make sure the padding between the URL bar edge and noScript button is 3px
Allow scripts
Make sure the padding between the URL bar edge and load timer is 10px

Test Plan (bookmark hanger)

Bookmark a page
Make sure arrowUp appears under the bookmark star icon
Enable Home button
Repeat the step 2

Test plan (for Windows)

On Windows, make sure the specified styles are applied
On Windows, make sure .topLevelStartButtons has padding-left:5px

Test Plan 8 (for macOS):

On macOS, make sure .topLevelStartButtons has padding-left:76px
On macOS, make the browser window to fullscreen
Make sure the left buttons on the navigation bar is moved to the left
Make sure the margin-right on navigator is gone

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-06-11T09:50:24Z

app/renderer/components/navigation/urlBar.js

+    display: 'flex',
+    alignItems: 'center',
+    justifyContent: 'center',
+    height: globalStyles.navigationBar.urlbarForm.height,


height, not width, is the basis.

luixxiul · 2017-06-11T09:57:42Z

app/renderer/components/styles/global.js

@@ -134,8 +134,6 @@ const globalStyles = {
    switchNubTopMargin: '2px',
    switchNubLeftMargin: '2px',
    switchNubRightMargin: '2px',
-    buttonHeight: '25px',
-    buttonWidth: '25px',


Please search for buttonHeight and buttonWidth in case where they be used on anywhere.

luixxiul · 2017-06-12T02:49:12Z

.gitignore

@@ -70,3 +70,5 @@ httpse.leveldb

 # sync bundle file should be built and copied from the brave/sync repo for now
 app/extensions/brave/content/scripts/sync.js
+
+package-lock.json


TODO: drop the commit to remove this.

luixxiul · 2017-06-12T02:49:20Z

.travis.yml

@@ -14,6 +14,7 @@ branches:
  only:
    - master
    - /\d+\.\d+\.x/
+    - navigationBar-aphrodite


TODO: drop the commit to remove this.

luixxiul · 2017-06-13T13:15:19Z

app/renderer/components/navigation/urlBarIcon.js

+    const relativeIcon = this.isAboutPage
+
+    const insecure = isPotentialPhishingUrl(this.props.location) || (this.props.isHTTPPage && !this.props.active && (this.props.isSecure === false || this.props.isSecure === 2))
+    const large = isPotentialPhishingUrl(this.props.location) || (this.props.isHTTPPage && !this.props.active)


These cause a bug on fa-search icon. I'll fix it later.

Fixed with 4ea852d

cezaraugusto · 2017-06-15T08:14:51Z

@luixxiul are those TODOs made for a follow-up?

luixxiul · 2017-06-15T09:01:03Z

are those TODOs made for a follow-up?

I could work on them on this PR or another one. Still, I think it should be worked on this PR as merging the long names to master branch is not good for contributors (it might look too confusing and complicated for them).

Yet since the refactoring with Aphrodite is on lower priority, I have set navigationBar-aphrodite as a base branch, so I can work on the TODOs with a follow up PR on that branch, if that is better.

cezaraugusto · 2017-06-15T09:26:38Z

I'm ok either way, up to you ✌(◕‿-)✌

luixxiul · 2017-06-15T09:55:07Z

@cezaraugusto I wouldn't squash the commits until your review is done, so would you start working on review soon? thanks!

@bsclifton

Resolves #9449 Auditors: @bsclifton @bridiver Test Plan:

This reverts commit 7b0f7d5.

This reverts commit 065b51a.

Fix url parsing bug

check for destroyed before sending to webContents

"Select its parent tab" setting now works as expected

…alues fix #9506 along with brave/muon@4189da5 change is backwards compatible and muon commit is not required for merging

fix #9508

@bsclifton

Auditors: @bsclifton Close #9466 Test Plan: covered by automated tests

Convert NoScriptInfo into redux

Create components etc -- navigator.js, navigationBar.js, urlBar.js etc Full changelog: 8997c76 Test Plan: available on #9299 (comment)

Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

.navbarMenubarBlockContainer (introduced with f2426f1#diff-303f0b6a297506f2cc18bf7b4cb370c5R789, which no longer exists on the master branch.) .inputbar-wrapper (introduced with 0e57690#diff-02c4b23ad267fe636760179e32fa29ceR141 with no clear reason. Since .inputbar-wrapper has not been used, it can be removed safely.) Test Plan: available on #9299 (comment)

- Add styles.navigator - Add reloadButton testId to .reloadButton - Convert .navigatorWrapper - Convert .navbarMenubarFlexContainer - Convert .urlbarForm - Convert .urlbarIconContainer - Convert #titleBar - Convert bookmarkButton and removeBookmarkButton with Aphrodite - Convert 'navigationButton stopButton' - Convert 'navigationButton reloadButton' - Convert 'navigationButton homeButton' - Convert .bookmarkButtonContainer - Convert .topLevelEndButtons - Convert .endButtons - Rename .backforward to .topLevelStartButtons (as we have already had 'topLevelEndButtons' on the same level) - Convert braveMenu Full changelog: b76f995 Test Plan: available on #9299 (comment)

Setting navigator__buttonContainer_outsideOfURLbar to apply border to the bookmark button and publisher button only, normalize size, placement, draggability etc of - navigator__buttonContainer - navigator__urlbarForm__buttonContainer_showNoScriptInfo - navigator__urlbarForm__urlbarIconContainer Test Plan: available on #9299 (comment)

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

- Increase size of noScriptButton 1px - Add urlbarForm_noScriptEnabled - Add title mode animation with navigator_titleMode - Convert .navigationButtonContainer - Convert topLevelStartButtons - Convert backButton and forwardButton - Confert box-shadow of urlbarForm - Convert input to urlbarForm__input - Convert -webkit-app-region of form and input - Convert urlbarForm_notTitleMode - Convert urlbarForm.isPublisherButtonEnabled - Convert input:focus + legend:before - Convert #navigator .urlbarForm input (for Windows) - Replace .loadTime with urlbarForm__titleBar__loadTime - Replace loadtime data-test-id (urlBar.js etc) - Refactor braveMenuButton - Replace height:24px with height:globalStyles.navigationBar.urlbarForm.height, which is 25px Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

Create components etc -- navigator.js, navigationBar.js, urlBar.js etc Full changelog: 8997c76 Test Plan: available on #9299 (comment)

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

Full changelog: b76f995 Test Plan: available on #9299 (comment)

Setting navigator__buttonContainer_outsideOfURLbar to apply border to the bookmark button and publisher button only, normalize size, placement, draggability etc of - navigator__buttonContainer - navigator__urlbarForm__buttonContainer_showNoScriptInfo - navigator__urlbarForm__urlbarIconContainer Test Plan: available on #9299 (comment)

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

Full changelog: b76f995 Test Plan: available on #9299 (comment)

Setting navigator__buttonContainer_outsideOfURLbar to apply border to the bookmark button and publisher button only, normalize size, placement, draggability etc of - navigator__buttonContainer - navigator__urlbarForm__buttonContainer_showNoScriptInfo - navigator__urlbarForm__urlbarIconContainer Test Plan: available on #9299 (comment)

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

Full changelog: b76f995 Test Plan: available on #9299 (comment)

Setting navigator__buttonContainer_outsideOfURLbar to apply border to the bookmark button and publisher button only, normalize size, placement, draggability etc of - navigator__buttonContainer - navigator__urlbarForm__buttonContainer_showNoScriptInfo - navigator__urlbarForm__urlbarIconContainer Test Plan: available on #9299 (comment)

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul added feature/navbar feature/titlebar feature/URLbar PR/work-in-progress ⚒ refactoring/aphrodite labels Jun 7, 2017

luixxiul added this to the 0.18.x milestone Jun 7, 2017

luixxiul self-assigned this Jun 7, 2017

luixxiul modified the milestones: 0.19.x, 0.18.x (Frozen, only critical adds from here) Jun 11, 2017

luixxiul commented Jun 11, 2017

View reviewed changes

luixxiul commented Jun 12, 2017

View reviewed changes

luixxiul commented Jun 13, 2017

View reviewed changes

NejcZdovc and others added 12 commits June 15, 2017 17:12

Converts CheckDefaultBrowserDialog into redux component

260272c

Resolves #9449 Auditors: @bsclifton @bridiver Test Plan:

Revert "Use fast-url-parser"

d329334

This reverts commit 7b0f7d5.

Revert "Handle query values as objects for cached parsed urls"

77873c3

This reverts commit 065b51a.

Merge pull request #9501 from brave/fix/urlparse

91c8074

Fix url parsing bug

Merge pull request #9462 from brave/web-contents-send

e8f4f25

check for destroyed before sending to webContents

Review feedback

b536afb

Merge pull request #9400 from bsclifton/fix-close-tab-behavior

bf34c40

"Select its parent tab" setting now works as expected

don’t add the tab to tab state until the tab_helper has updated the v…

e261b9e

…alues fix #9506 along with brave/muon@4189da5 change is backwards compatible and muon commit is not required for merging

check for tab destoyed

a36189a

fix #9508

check for null frame

c1effd9

Tweak tabtitle threshold w/ secondary icons on small tabs

485042c

Auditors: @bsclifton Close #9466 Test Plan: covered by automated tests

Merge pull request #9494 from NejcZdovc/redux/noScriptInfo

ee90736

Convert NoScriptInfo into redux

luixxiul pushed a commit that referenced this pull request Jul 3, 2017

Refactor navigationBar (part 3)

b19e881

Create components etc -- navigator.js, navigationBar.js, urlBar.js etc Full changelog: 8997c76 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 3, 2017

Misc change

fa1980c

Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Misc change

0a4e138

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor urlBarIcon

bc00133

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor navigationBar (part 3)

d69e778

Create components etc -- navigator.js, navigationBar.js, urlBar.js etc Full changelog: 8997c76 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Misc change

4290d4b

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor navigationBar (part 1)

088922d

Full changelog: b76f995 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor urlBarIcon

41d2994

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor navigationBar (part 2)

ca189d4

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Refactor navigationBar (part 3)

8694192

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 4, 2017

Misc change

852193a

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 11, 2017

Refactor navigationBar (part 1)

758fa8c

Full changelog: b76f995 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 11, 2017

Refactor urlBarIcon

e928f88

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 11, 2017

Refactor navigationBar (part 2)

bad4469

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 11, 2017

Refactor navigationBar (part 3)

912d067

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 11, 2017

Misc change

6ebd52f

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 12, 2017

Refactor navigationBar (part 1)

4a0f3f0

Full changelog: b76f995 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 12, 2017

Refactor urlBarIcon

5a7d65e

Full changelog: f81ea52 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 12, 2017

Refactor navigationBar (part 2)

f78e465

Full changelog: 9b6c22d Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 12, 2017

Refactor navigationBar (part 3)

d996200

Full changelog: 8997c76 Test Plan: available on #9299 (comment)

luixxiul pushed a commit that referenced this pull request Jul 12, 2017

Misc change

1985612

- Convert 'buttonSeparator' Full changelog: 25f02d4 Test Plan: available on #9299 (comment)

cezaraugusto mentioned this pull request Aug 13, 2017

navigationBar refactor tracking issue #9283

Closed

luixxiul deleted the navigationBar-aphrodite branch August 15, 2017 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor navigationBar with Aphrodite - first try #9299

Refactor navigationBar with Aphrodite - first try #9299

luixxiul commented Jun 7, 2017 •

edited

Loading

luixxiul Jun 11, 2017

luixxiul Jun 11, 2017 •

edited

Loading

luixxiul Jun 12, 2017

luixxiul Jun 12, 2017

luixxiul Jun 13, 2017

luixxiul Jun 17, 2017

cezaraugusto commented Jun 15, 2017

luixxiul commented Jun 15, 2017 •

edited

Loading

cezaraugusto commented Jun 15, 2017

luixxiul commented Jun 15, 2017

Refactor navigationBar with Aphrodite - first try #9299

Refactor navigationBar with Aphrodite - first try #9299

Conversation

luixxiul commented Jun 7, 2017 • edited Loading

Test Plans

luixxiul Jun 11, 2017

Choose a reason for hiding this comment

luixxiul Jun 11, 2017 • edited Loading

Choose a reason for hiding this comment

luixxiul Jun 12, 2017

Choose a reason for hiding this comment

luixxiul Jun 12, 2017

Choose a reason for hiding this comment

luixxiul Jun 13, 2017

Choose a reason for hiding this comment

luixxiul Jun 17, 2017

Choose a reason for hiding this comment

cezaraugusto commented Jun 15, 2017

luixxiul commented Jun 15, 2017 • edited Loading

cezaraugusto commented Jun 15, 2017

luixxiul commented Jun 15, 2017

luixxiul commented Jun 7, 2017 •

edited

Loading

luixxiul Jun 11, 2017 •

edited

Loading

luixxiul commented Jun 15, 2017 •

edited

Loading