Add "wide URL bar" option #8714

luixxiul · 2017-05-05T05:11:06Z

Fix #8421

Auditors:

Test Plan 1:

Clear the browser profile
Open the browser
Open about:preferences#general
Make sure "Use wide URL bar" is disabled
Enable the option
Make sure the URL bar fills the margin around itself

Test Plan 2:

Open about:preferences#advanced
Enable/disable 'Always show the URL bar'
Make sure the setting is applied

Test Plan 3:

Open about:preferences#extensions
Enable LastPass extension
Make sure there is margin between the URL bar and the extension's icon

Test Plan 4:

Minimize the window's width
Make sure there does not appear extraDragArea between the URL bar and the extension's icon

Test Plan 5:

Open about:preferences#general
Change the window width from 600 px to 599 px
Make sure the URL bar width does not change greatly

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).

luixxiul · 2017-05-05T05:15:06Z

app/extensions/brave/locales/en-US/preferences.properties

@@ -325,6 +325,7 @@ alwaysDeny=Always deny
 alwaysAsk=Always ask
 autoHideMenuBar=Hide the menu bar by default
 disableTitleMode=Always show the URL bar
+wideURLbar=Use wide URL bar


I actually am not sure if it sounds native. Should it be rather something like Maximize the URL bar or Make the URL bar wide, etc?

@luixxiul I think that's a good name 😄

Other names might be too complex, like "Have URL bar fill entire width" (which sounds confusing)

cezaraugusto · 2017-05-16T09:26:50Z

code looks good but I'm not sure if we want the UI as-is. Mind adding this commit to a branch in the main repo so @bradleyrichter can check it out? some screenshots might help as well

luixxiul · 2017-05-16T14:24:19Z

np I'll upload gif later.

also:

git fetch upstream pull/8714/head:wideURLbar
git checkout wideURLbar

luixxiul · 2017-05-16T16:23:07Z

also it looks some media queries are working around 600px on about:preferences.

it does not happen with normal pages.

luixxiul · 2017-05-16T17:03:54Z

#8421 (comment)

yes, default should be as is to allow for easier window dragging so please add a pref in about:preferences#advanced.

It is disabled by default. cf: https://github.com/brave/browser-laptop/pull/8714/files#diff-18a136e10fa10b96b0c7256285b87f95R134

luixxiul · 2017-05-16T17:04:36Z

Also I found the fix for the issue above. I'll push another commit.

luixxiul · 2017-05-16T17:16:02Z

For @bradleyrichter:

luixxiul · 2017-05-16T17:21:11Z

ready for re-review.

NejcZdovc · 2017-05-16T17:30:15Z

app/renderer/components/navigation/navigationBar.js

@@ -167,6 +168,8 @@ class NavigationBar extends React.Component {
 }

 render () {
+ const wideURLbar = getSetting(settings.WIDE_URL_BAR)


All variables that are used in renderer needs to be defined in mergeProps

Fixed with 9edeee4

perfect thank you

NejcZdovc · 2017-05-16T17:30:31Z

app/renderer/components/navigation/urlBar.js

@@ -483,9 +483,12 @@ class UrlBar extends React.Component {
 }

 render () {
+ const wideURLbar = getSetting(settings.WIDE_URL_BAR)


Fixed with 9edeee4

bradleyrichter · 2017-05-17T18:30:59Z

I only see one change needed for this PR:

Let's move both URL bar options from general prefs to advanced prefs. General is becoming heavy and should be reserved for the most basic options rather than overrides.

luixxiul · 2017-05-17T18:39:02Z

OK I'll address that.

luixxiul · 2017-05-18T16:39:44Z

Updated.

luixxiul · 2017-05-18T16:40:25Z

docs/state.md

@@ -183,7 +183,8 @@ AppStore
 'extensions.pocket.enabled': boolean, // true if pocket is enabled
 'extensions.vimium.enabled': boolean, // true if vimium is enabled
 'general.autohide-menu': boolean, // true if the Windows menu should be autohidden
- 'general.bookmarks-toolbar-mode': boolean, // true to show bookmakrs toolbar
+ 'general.wide-url-bar': boolean, // true to use wide URL bar
+ 'general.bookmarks-toolbar-mode': boolean, // true to show bookmarks toolbar


just fixed typo

Fix #8421 Auditors: Test Plan 1: 1. Clear the browser profile 2. Open the browser 3. Open about:preferences#general 4. Make sure "Use wide URL bar" is disabled 5. Enable the option 6. Make sure the URL bar fills the margin around itself Test Plan 2: 1. Open about:preferences#advanced 2. Enable/disable 'Always show the URL bar' 3. Make sure the setting is applied Test Plan 3: 1. Open about:preferences#extensions 2. Enable LastPass extension 3. Make sure there is margin between the URL bar and the extension's icon Test Plan 4: 1. Minimize the window's width 2. Make sure there does not appear extraDragArea between the URL bar and the extension's icon Test Plan 5: 1. Open about:preferences#general 2. Change the window width from 600 px to 599 px 3. Make sure the URL bar width does not change greatly

luixxiul · 2017-05-19T16:48:24Z

Since the issue has been addressed, I'm going to merge this. Let's address other issues if any with follow-ups.

bsclifton · 2017-05-19T22:42:37Z

Great job, @luixxiul! This looks amazing. I'll definitely be using it 😄

luixxiul added the feature/URLbar label May 5, 2017

luixxiul added this to the 0.15.3 milestone May 5, 2017

luixxiul self-assigned this May 5, 2017

luixxiul requested review from bsclifton and srirambv May 5, 2017 05:11

ShalokShalom mentioned this pull request May 5, 2017

Offer option to have URL bar fill empty space ("wide URL bar") #8421

Closed

luixxiul added the PR/needs-QA-attention ☕ label May 5, 2017

luixxiul commented May 5, 2017

View reviewed changes

alexwykoff modified the milestones: 0.15.4, 0.15.3 May 5, 2017

luixxiul modified the milestones: 0.15.3, 0.15.4 May 5, 2017

luixxiul added PR/needs-rebase ۞ PR/work-in-progress ⚒ PR/needs-QA-attention ☕ and removed PR/needs-QA-attention ☕ PR/needs-rebase ۞ PR/work-in-progress ⚒ labels May 6, 2017

luixxiul modified the milestones: 0.15.3, 0.15.4 May 7, 2017

bbondy modified the milestones: 0.15.4, 0.15.3 May 8, 2017

luixxiul requested a review from cezaraugusto May 13, 2017 15:42

NejcZdovc suggested changes May 16, 2017

View reviewed changes

luixxiul added PR/work-in-progress ⚒ PR/needs-QA-attention ☕ and removed PR/needs-QA-attention ☕ PR/work-in-progress ⚒ labels May 17, 2017

luixxiul commented May 18, 2017

View reviewed changes

luixxiul added PR/needs-rebase ۞ PR/work-in-progress ⚒ and removed PR/needs-QA-attention ☕ labels May 19, 2017

luixxiul removed PR/needs-rebase ۞ PR/work-in-progress ⚒ labels May 19, 2017

luixxiul merged commit 1362dbc into brave:master May 19, 2017

luixxiul removed request for bsclifton, srirambv and cezaraugusto May 19, 2017 16:50

luixxiul deleted the navigationBar-wide-url-bar branch May 19, 2017 16:50

luixxiul mentioned this pull request Jul 22, 2017

Search fails for terms with "." (ex: "json.parse" or "location.href") #3317

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "wide URL bar" option #8714

Add "wide URL bar" option #8714

luixxiul commented May 5, 2017 •

edited

Loading

luixxiul May 5, 2017

bsclifton May 9, 2017

cezaraugusto commented May 16, 2017 •

edited

Loading

luixxiul commented May 16, 2017 •

edited

Loading

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017 •

edited

Loading

luixxiul commented May 16, 2017

NejcZdovc May 16, 2017

luixxiul May 16, 2017

NejcZdovc May 16, 2017

NejcZdovc May 16, 2017

luixxiul May 16, 2017

bradleyrichter commented May 17, 2017

luixxiul commented May 17, 2017

luixxiul commented May 18, 2017

luixxiul May 18, 2017

luixxiul commented May 19, 2017

bsclifton commented May 19, 2017

Add "wide URL bar" option #8714

Add "wide URL bar" option #8714

Conversation

luixxiul commented May 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cezaraugusto commented May 16, 2017 • edited Loading

luixxiul commented May 16, 2017 • edited Loading

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017

luixxiul commented May 16, 2017 • edited Loading

luixxiul commented May 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradleyrichter commented May 17, 2017

luixxiul commented May 17, 2017

luixxiul commented May 18, 2017

Choose a reason for hiding this comment

luixxiul commented May 19, 2017

bsclifton commented May 19, 2017

luixxiul commented May 5, 2017 •

edited

Loading

cezaraugusto commented May 16, 2017 •

edited

Loading

luixxiul commented May 16, 2017 •

edited

Loading

luixxiul commented May 16, 2017 •

edited

Loading