From 63914e2380183e6c149592d1bdedff8835bad361 Mon Sep 17 00:00:00 2001 From: Radoslav Vitanov Date: Thu, 11 Aug 2016 23:44:38 +0300 Subject: [PATCH 01/17] Custom window action bar to address #3126 #3036 --- .../linux/titlebutton-close-hover.png | Bin 0 -> 606 bytes .../linux/titlebutton-close.png | Bin 0 -> 472 bytes .../linux/titlebutton-maximize-hover.png | Bin 0 -> 589 bytes .../linux/titlebutton-maximize.png | Bin 0 -> 464 bytes .../linux/titlebutton-minimize-hover.png | Bin 0 -> 485 bytes .../linux/titlebutton-minimize.png | Bin 0 -> 462 bytes .../windows/10/close-btn-white.svg | 63 +++++++++ img/window-actions/windows/10/close-btn.svg | 62 +++++++++ .../windows/10/max-btn-fullscreen.svg | 79 +++++++++++ img/window-actions/windows/10/max-btn.svg | 74 ++++++++++ img/window-actions/windows/10/min-btn.svg | 74 ++++++++++ js/components/main.js | 8 +- js/components/windowActionBar.js | 71 ++++++++++ js/stores/appStore.js | 3 +- less/navigationBar.less | 131 ++++++++++++++++++ less/variables.less | 2 +- test/components/windowTest.js | 52 ++++++- test/lib/selectors.js | 3 + 18 files changed, 614 insertions(+), 8 deletions(-) create mode 100755 img/window-actions/linux/titlebutton-close-hover.png create mode 100755 img/window-actions/linux/titlebutton-close.png create mode 100755 img/window-actions/linux/titlebutton-maximize-hover.png create mode 100755 img/window-actions/linux/titlebutton-maximize.png create mode 100755 img/window-actions/linux/titlebutton-minimize-hover.png create mode 100755 img/window-actions/linux/titlebutton-minimize.png create mode 100644 img/window-actions/windows/10/close-btn-white.svg create mode 100644 img/window-actions/windows/10/close-btn.svg create mode 100644 img/window-actions/windows/10/max-btn-fullscreen.svg create mode 100644 img/window-actions/windows/10/max-btn.svg create mode 100644 img/window-actions/windows/10/min-btn.svg create mode 100644 js/components/windowActionBar.js diff --git a/img/window-actions/linux/titlebutton-close-hover.png b/img/window-actions/linux/titlebutton-close-hover.png new file mode 100755 index 0000000000000000000000000000000000000000..73405fc18816276d32184e67e1431502a7f2910f GIT binary patch literal 606 zcmV-k0-^nhP)kjesU*2*9B7*&f5M4EmZZt*JVapx)0jQl#`JLS zs@GWpFB%7$Sl({4d;$N8xoKnaZKUm8^*X73y(txCb|6Tltm(LLy#Vqb14fpOJVU|d za+r-L?&H{~k_;0ZbE1$10M`$2b~C{J#9V^zCnnCW2XMRq0KjbtxrkwoXuiORUF+d2=yVLP{M}f>OSP(+M6!IFSnM`?=R;9=zju!v`P-_aFmH6MNrzx$_ z81Y&m)>ogNY2=4GfEP(+I5p9`)1mGtAOuU=Rq^T9FbH(=!=87dFj}hj=~jnYQ&1Io zREj**nu7DMI!2ZaU~yB=cH60by(tMYJ19z}tmV3JykIHB`g0hL^DwLaGJx7DmE{i` z|6u^*`}pntW%qQ&a%yW+ES_*%LXMEXEoT{1$C{Xj`6P)NlO?{0f{HW;-|Uh#-coq^g{S03exBVoVg9 zBrZ$!-#?uSAq1iv*9b2N`LoqN9M2cX4!cVo0PF}0=mkrl;{zb0sU~Sor(%4uWRqm;tP}ALHMu`#fw{mN9 z>8&lh@$mZc3>|<0i%wMPkuPuw++ESI$!m(Sy0aX*=$(`}m0VZ$Oy^{T?>`9@JuGjYNAx{GORyaFN+P~L50 zL)b*WXU`v;woXTj=uD90Oj?=WlQI0&#`G?M36enN;~qq*n03d_DVgI~Bc5q=-NnKW zKr$3|j}O3Z7ZSBa1OT8Y3ORPmBsXZL9KU(334U8Y6WBPw){gLER+?cn5{RB9pTRVR z*Pr*-z(hp)VbJX!Z2KBNMb$AP$;}n0WAM?=1prvsa{pM zJl~KNHRF#w^nTodNVUINDL{CuuI_!iJF^G2zBIspZ~T7^Fh-}-K6Z|7FA~dL z>4qp2&qb-2U2C~Pf98yw6X)pm0!Vok0MzfQ9Ff`69Lt$BLu<=L1kIWEQ1kr9ukMHT bAg=ub9qq@0D=Wz700000NkvXXu0mjfxtIrj literal 0 HcmV?d00001 diff --git a/img/window-actions/linux/titlebutton-maximize.png b/img/window-actions/linux/titlebutton-maximize.png new file mode 100755 index 0000000000000000000000000000000000000000..9fb27510706cf376531a3b1c43aa6bf85234c3e4 GIT binary patch literal 464 zcmV;>0WbcEP)00!@ZkX!LeU8LC)Tj>7{(?ZLUtU&cn`gU zJ41R1+jtBYHEOHCqksiLp068iP19KXRma8;ZTH6H%fGc8$*9tn)}Vha+aAY>5NkOADC>rKzhJK*-oU@VrUP z1%0*Ut`5E2p_mROEq@`8Bt_u~A;TY2nsQklXY)Cn!nGaf zZ%eJ#wkF(clr81*#g{^{$e*;n*sR}ow*G!YIPg75Q4&c2Ajxu?P`*pjg4p=|(iMKJv;Cud<#9Yufbv;Nb+J>=ZyZwNuEqCQ?jHYWl|4Oc9!!nI#z2mHZBKIgY z@&$P$DRP>S)SvTg!`ND;(Y(92o`9pQ0>JbkTO^e43Zkf`D6Ieh3&XkZ`D3=*59%Ff bedzrJAHb4B3YWa^00000NkvXXu0mjf6d%r) literal 0 HcmV?d00001 diff --git a/img/window-actions/linux/titlebutton-minimize.png b/img/window-actions/linux/titlebutton-minimize.png new file mode 100755 index 0000000000000000000000000000000000000000..6ced316d8202e43a1a87366761f6bf8231aa0ea2 GIT binary patch literal 462 zcmV;<0WtoGP)eFP2u?6cZ$_K%a>2N@T^F@E-66 zO1dMBPXsFi#sDq!3dNq*ja8xvmVY&i`6e^TFmRNaCDj<<2E1_9CWO7s0U)Jxoo3717LQxwy&V#wF3rDz(5h{y9F%QDzHP7-Wfv(@$NZ#W$*hIUh*NOA>ECG-Pu z-rtN?*Rz?KCDkgj{#w;_?xgz4X1&_`&gu>s;0B7Ol|ldjimLKRxIXfN*#3RfsUWf- zh@yey@}gc^6_}6)LY|-NfC0#nex!?YAx+Z-?&8?avaCF!6yhkJV9q4Nt>YwRq3^mo zx<8Dt*$J1y#x+Af_!otq58l=_A9}(1=jEQ2wp>*$MN@f1_Ro60>+Za*Yu-P4F91is z3IH&Dl5`|oUl2uuC8PlWK+|*{NAZO252J@(ur7K(0m_JreGPJO#Q*>R07*qoM6N<$ Ef)tv=^#A|> literal 0 HcmV?d00001 diff --git a/img/window-actions/windows/10/close-btn-white.svg b/img/window-actions/windows/10/close-btn-white.svg new file mode 100644 index 00000000000..72e603cbd55 --- /dev/null +++ b/img/window-actions/windows/10/close-btn-white.svg @@ -0,0 +1,63 @@ + + + + + + image/svg+xml + + + + + + + + + + + diff --git a/img/window-actions/windows/10/close-btn.svg b/img/window-actions/windows/10/close-btn.svg new file mode 100644 index 00000000000..8dc9e16250f --- /dev/null +++ b/img/window-actions/windows/10/close-btn.svg @@ -0,0 +1,62 @@ + + + + + + image/svg+xml + + + + + + + + + + + diff --git a/img/window-actions/windows/10/max-btn-fullscreen.svg b/img/window-actions/windows/10/max-btn-fullscreen.svg new file mode 100644 index 00000000000..c534778f540 --- /dev/null +++ b/img/window-actions/windows/10/max-btn-fullscreen.svg @@ -0,0 +1,79 @@ + + + + + + image/svg+xml + + + + + + + + + + + + + + + + diff --git a/img/window-actions/windows/10/max-btn.svg b/img/window-actions/windows/10/max-btn.svg new file mode 100644 index 00000000000..c6beccbfddb --- /dev/null +++ b/img/window-actions/windows/10/max-btn.svg @@ -0,0 +1,74 @@ + + + + + + image/svg+xml + + + + + + + + + + + + + + + diff --git a/img/window-actions/windows/10/min-btn.svg b/img/window-actions/windows/10/min-btn.svg new file mode 100644 index 00000000000..22f67f36884 --- /dev/null +++ b/img/window-actions/windows/10/min-btn.svg @@ -0,0 +1,74 @@ + + + + + + image/svg+xml + + + + + + + + + + + + + + + diff --git a/js/components/main.js b/js/components/main.js index 78cf6e1f190..2905286d848 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -39,6 +39,7 @@ const ContextMenu = require('./contextMenu') const PopupWindow = require('./popupWindow') const NoScriptInfo = require('./noScriptInfo') const LongPressButton = require('./longPressButton') +const WindowActionBar = require('./windowActionBar') // Constants const config = require('../constants/config') @@ -585,11 +586,7 @@ class Main extends ImmutableComponent { if (!e.target.className.includes('navigatorWrapper')) { return } - if (currentWindow.isMaximized()) { - currentWindow.maximize() - } else { - currentWindow.unmaximize() - } + return (!currentWindow.isMaximized()) ? currentWindow.maximize() : currentWindow.unmaximize() } onMouseDown (e) { @@ -896,6 +893,7 @@ class Main extends ImmutableComponent { braveShieldsDown: !braverySettings.shieldsUp })} onClick={this.onBraveMenu} /> + {process.platform === 'darwin' ? null : } diff --git a/js/components/windowActionBar.js b/js/components/windowActionBar.js new file mode 100644 index 00000000000..3a763a06986 --- /dev/null +++ b/js/components/windowActionBar.js @@ -0,0 +1,71 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const os = require('os') +const React = require('react') +const ImmutableComponent = require('./immutableComponent') +const currentWindow = require('../../app/renderer/currentWindow') + +class WindowActionBar extends ImmutableComponent { + constructor () { + super() + this.onDoubleClick = this.onDoubleClick.bind(this) + this.onMinimizeClick = this.onMinimizeClick.bind(this) + this.onMaximizeClick = this.onMaximizeClick.bind(this) + this.onCloseClick = this.onCloseClick.bind(this) + this.osClass = this.getPlatformCssClass() + } + + get buttonClass () { + return (this.props.windowMaximized ? 'fullscreen' : '') + } + + getPlatformCssClass () { + switch (os.platform()) { + case 'linux': + return 'linux' + case 'win32': + if (/10.0./.test(os.release())) { + return 'win-10' + } else if (/6.1./.test(os.release())) { + return 'win-7' + } else { + return 'win' + } + default: + return 'hidden' + } + } + + onMinimizeClick (e) { + currentWindow.minimize() + } + + onMaximizeClick (e) { + return (!currentWindow.isMaximized()) ? currentWindow.maximize() : currentWindow.unmaximize() + } + + onCloseClick (e) { + currentWindow.close() + } + + onDoubleClick (e) { + if (!e.target.className.includes('navigatorWrapper')) { + return + } + this.onMaximizeClick(e) + } + + render () { + return
+
+ + + +
+
+ } +} + +module.exports = WindowActionBar diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 96495ea52ec..d23d5a80b74 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -136,7 +136,8 @@ const createWindow = (browserOpts, defaults, frameOpts, windowState) => { titleBarStyle: 'hidden-inset', autoHideMenuBar: autoHideMenuBarSetting, title: appConfig.name, - webPreferences: defaults.webPreferences + webPreferences: defaults.webPreferences, + frame: (process.platform === 'darwin') } if (process.platform === 'linux') { diff --git a/less/navigationBar.less b/less/navigationBar.less index 954e66a4029..3f5f2ed2b91 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -29,6 +29,137 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } +// Window Header +.window-header { + -webkit-user-select: none; + -webkit-app-region: drag; + height: 18px; + padding: 3px 0; + margin: 0px; + display: flex; + line-height: 30px; + + .hidden { + display: none; + } + + .title-bar-btns { + -webkit-app-region: no-drag; + } + + .win-7, .win-10 { + margin: -13px 3px 0 0; + + button.win-action-btn { + outline: 0; + cursor: pointer; + width: 45px; + height: 23px; + color: #b1acac; + background-size: 12px 12px!important; + background-position: center!important; + border: 1px solid #e1e1e1; + display: inline-block; + padding: 12px; + + &.fullscreen { + height:21px; + padding:6px; + } + + &.close-btn { + background: url('../img/window-actions/windows/10/close-btn.svg') no-repeat; + &:hover { + background: url('../img/window-actions/windows/10/close-btn-white.svg') no-repeat; + background-color: #e5182c; + } + &:active { + background: url('../img/window-actions/windows/10/close-btn-white.svg') no-repeat; + background-color: #ef717c; + } + } + + &.min-btn { + border-right: 0px; + border-bottom-left-radius: 5px; + background: url('../img/window-actions/windows/10/min-btn.svg') no-repeat; + &:hover { + background-color: #e5e5e5; + } + &:active { + background-color: #cacacb; + } + } + + &.max-btn { + background: url('../img/window-actions/windows/10/max-btn.svg') no-repeat; + border-right: 0px; + &:hover { + background-color: #e5e5e5; + } + &:active { + background-color: #cacacb; + } + &.fullscreen { + background: url('../img/window-actions/windows/10/max-btn-fullscreen.svg') no-repeat; + &:hover { + background-color: #e5e5e5; + } + &:active { + background-color: #cacacb; + } + } + } + } + + } + + .linux { + button { + outline: 0; + cursor: pointer; + width: 14px; + height: 14px; + color: transparent; + background-color: transparent; + background-position: center; + background-repeat: no-repeat; + border-width: 0; + border-radius: 50%; + display: inline-block; + padding: 0px; + margin-right: 15px; + } + + min-btn:backdrop, .max-btn:backdrop, .close-btn:backdrop { + opacity: 1; + } + + .close-btn { + background: url('../img/window-actions/linux/titlebutton-close.png') 0 0 / contain no-repeat; + } + + .min-btn { + background: url('../img/window-actions/linux/titlebutton-minimize.png') 0 0 / contain no-repeat; + } + + .max-btn { + background: url('../img/window-actions/linux/titlebutton-maximize.png') 0 0 / contain no-repeat; + } + + .close-btn:hover { + background: url('../img/window-actions/linux/titlebutton-close-hover.png') 0 0 / contain no-repeat; + } + + .min-btn:hover { + background: url('../img/window-actions/linux/titlebutton-minimize-hover.png') 0 0 / contain no-repeat; + } + + .max-btn:hover { + background: url('../img/window-actions/linux/titlebutton-maximize-hover.png') 0 0 / contain no-repeat; + } + } +} // Here I grouped the rules to keep the layout of the top bar consistent. // The tricky part is to keep the title in `#navigator` centered when the diff --git a/less/variables.less b/less/variables.less index 0389891118b..1ac51bf2686 100644 --- a/less/variables.less +++ b/less/variables.less @@ -69,7 +69,7 @@ @bookmarksFileIconSize: 13px; @bookmarksFolderIconSize: 15px; -@navbarButtonSpacing: 4px; +@navbarButtonSpacing: 14px; @navbarButtonWidth: 35px; @navbarBraveButtonWidth: 23px; @navbarBraveButtonMarginLeft: 80px; diff --git a/test/components/windowTest.js b/test/components/windowTest.js index fb255f5fec2..596322e3355 100644 --- a/test/components/windowTest.js +++ b/test/components/windowTest.js @@ -1,7 +1,7 @@ /* global describe, it, before */ const Brave = require('../lib/brave') -const {activeWebview} = require('../lib/selectors') +const {activeWebview, minimizeButton, maximizeButton, closeButton} = require('../lib/selectors') describe('application window', function () { describe('application launch', function () { @@ -146,6 +146,56 @@ describe('application window', function () { }) }) + if (process.platform !== 'darwin') { + describe('window top action buttons', function () { + Brave.beforeAll(this) + + before(function * () { + yield this.app.client + .waitUntilWindowLoaded() + .waitForUrl(Brave.newTabUrl) + .windowByIndex(0) + .resizeWindow(600, 700) + .waitUntilWindowLoaded() + }) + + it('should be maximized when maximize button is clicked', function * () { + yield this.app.client + .click(maximizeButton) + .windowByIndex(0) + .getWindowWidth().should.eventually.be.getPrimaryDisplayWidth() + .getWindowHeight().should.eventually.be.getPrimaryDisplayHeight() + }) + + it('should be minimized when minimize button is clicked', function * () { + yield this.app.client + .click(minimizeButton) + .waitUntil(function () { + return this.windowByIndex(0).isWindowMinimized() + }) + }) + + it('should close the new window when close button is clicked', function * () { + yield this.app.client + .windowByIndex(0) + .newWindowAction() + .waitUntil(function () { + return this.getWindowCount().then((count) => { + return count === 2 + }) + }) + .windowByIndex(1) + .waitUntilWindowLoaded() + .click(closeButton) + .waitUntil(function () { + return this.getWindowCount().then((count) => { + return count === 1 + }) + }) + }) + }) + } + describe('windw.open with click', function () { describe('with features', function () { Brave.beforeAll(this) diff --git a/test/lib/selectors.js b/test/lib/selectors.js index 4ff78271d1f..7a748caa0ad 100644 --- a/test/lib/selectors.js +++ b/test/lib/selectors.js @@ -1,4 +1,7 @@ module.exports = { + minimizeButton: '.min-btn', + maximizeButton: '.max-btn', + closeButton: '.close-btn', urlInput: '#urlInput', activeWebview: '.frameWrapper.isActive webview', activeTab: '.tab.active', From 86f2f53d98d325b2cb45985562ca97bd65bc63db Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 22 Aug 2016 13:04:21 -0700 Subject: [PATCH 02/17] Reverted Linux specific changes. Frameless mode only triggered on Windows. --- .../linux/titlebutton-close-hover.png | Bin 606 -> 0 bytes .../linux/titlebutton-close.png | Bin 472 -> 0 bytes .../linux/titlebutton-maximize-hover.png | Bin 589 -> 0 bytes .../linux/titlebutton-maximize.png | Bin 464 -> 0 bytes .../linux/titlebutton-minimize-hover.png | Bin 485 -> 0 bytes .../linux/titlebutton-minimize.png | Bin 462 -> 0 bytes js/components/main.js | 3 +- js/stores/appStore.js | 9 ++-- less/navigationBar.less | 48 +----------------- test/components/windowTest.js | 3 +- 10 files changed, 11 insertions(+), 52 deletions(-) delete mode 100755 img/window-actions/linux/titlebutton-close-hover.png delete mode 100755 img/window-actions/linux/titlebutton-close.png delete mode 100755 img/window-actions/linux/titlebutton-maximize-hover.png delete mode 100755 img/window-actions/linux/titlebutton-maximize.png delete mode 100755 img/window-actions/linux/titlebutton-minimize-hover.png delete mode 100755 img/window-actions/linux/titlebutton-minimize.png diff --git a/img/window-actions/linux/titlebutton-close-hover.png b/img/window-actions/linux/titlebutton-close-hover.png deleted file mode 100755 index 73405fc18816276d32184e67e1431502a7f2910f..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 606 zcmV-k0-^nhP)kjesU*2*9B7*&f5M4EmZZt*JVapx)0jQl#`JLS zs@GWpFB%7$Sl({4d;$N8xoKnaZKUm8^*X73y(txCb|6Tltm(LLy#Vqb14fpOJVU|d za+r-L?&H{~k_;0ZbE1$10M`$2b~C{J#9V^zCnnCW2XMRq0KjbtxrkwoXuiORUF+d2=yVLP{M}f>OSP(+M6!IFSnM`?=R;9=zju!v`P-_aFmH6MNrzx$_ z81Y&m)>ogNY2=4GfEP(+I5p9`)1mGtAOuU=Rq^T9FbH(=!=87dFj}hj=~jnYQ&1Io zREj**nu7DMI!2ZaU~yB=cH60by(tMYJ19z}tmV3JykIHB`g0hL^DwLaGJx7DmE{i` z|6u^*`}pntW%qQ&a%yW+ES_*%LXMEXEoT{1$C{Xj`6P)NlO?{0f{HW;-|Uh#-coq^g{S03exBVoVg9 zBrZ$!-#?uSAq1iv*9b2N`LoqN9M2cX4!cVo0PF}0=mkrl;{zb0sU~Sor(%4uWRqm;tP}ALHMu`#fw{mN9 z>8&lh@$mZc3>|<0i%wMPkuPuw++ESI$!m(Sy0aX*=$(`}m0VZ$Oy^{T?>`9@JuGjYNAx{GORyaFN+P~L50 zL)b*WXU`v;woXTj=uD90Oj?=WlQI0&#`G?M36enN;~qq*n03d_DVgI~Bc5q=-NnKW zKr$3|j}O3Z7ZSBa1OT8Y3ORPmBsXZL9KU(334U8Y6WBPw){gLER+?cn5{RB9pTRVR z*Pr*-z(hp)VbJX!Z2KBNMb$AP$;}n0WAM?=1prvsa{pM zJl~KNHRF#w^nTodNVUINDL{CuuI_!iJF^G2zBIspZ~T7^Fh-}-K6Z|7FA~dL z>4qp2&qb-2U2C~Pf98yw6X)pm0!Vok0MzfQ9Ff`69Lt$BLu<=L1kIWEQ1kr9ukMHT bAg=ub9qq@0D=Wz700000NkvXXu0mjfxtIrj diff --git a/img/window-actions/linux/titlebutton-maximize.png b/img/window-actions/linux/titlebutton-maximize.png deleted file mode 100755 index 9fb27510706cf376531a3b1c43aa6bf85234c3e4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 464 zcmV;>0WbcEP)00!@ZkX!LeU8LC)Tj>7{(?ZLUtU&cn`gU zJ41R1+jtBYHEOHCqksiLp068iP19KXRma8;ZTH6H%fGc8$*9tn)}Vha+aAY>5NkOADC>rKzhJK*-oU@VrUP z1%0*Ut`5E2p_mROEq@`8Bt_u~A;TY2nsQklXY)Cn!nGaf zZ%eJ#wkF(clr81*#g{^{$e*;n*sR}ow*G!YIPg75Q4&c2Ajxu?P`*pjg4p=|(iMKJv;Cud<#9Yufbv;Nb+J>=ZyZwNuEqCQ?jHYWl|4Oc9!!nI#z2mHZBKIgY z@&$P$DRP>S)SvTg!`ND;(Y(92o`9pQ0>JbkTO^e43Zkf`D6Ieh3&XkZ`D3=*59%Ff bedzrJAHb4B3YWa^00000NkvXXu0mjf6d%r) diff --git a/img/window-actions/linux/titlebutton-minimize.png b/img/window-actions/linux/titlebutton-minimize.png deleted file mode 100755 index 6ced316d8202e43a1a87366761f6bf8231aa0ea2..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 462 zcmV;<0WtoGP)eFP2u?6cZ$_K%a>2N@T^F@E-66 zO1dMBPXsFi#sDq!3dNq*ja8xvmVY&i`6e^TFmRNaCDj<<2E1_9CWO7s0U)Jxoo3717LQxwy&V#wF3rDz(5h{y9F%QDzHP7-Wfv(@$NZ#W$*hIUh*NOA>ECG-Pu z-rtN?*Rz?KCDkgj{#w;_?xgz4X1&_`&gu>s;0B7Ol|ldjimLKRxIXfN*#3RfsUWf- zh@yey@}gc^6_}6)LY|-NfC0#nex!?YAx+Z-?&8?avaCF!6yhkJV9q4Nt>YwRq3^mo zx<8Dt*$J1y#x+Af_!otq58l=_A9}(1=jEQ2wp>*$MN@f1_Ro60>+Za*Yu-P4F91is z3IH&Dl5`|oUl2uuC8PlWK+|*{NAZO252J@(ur7K(0m_JreGPJO#Q*>R07*qoM6N<$ Ef)tv=^#A|> diff --git a/js/components/main.js b/js/components/main.js index 2905286d848..e20c38cd82e 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -49,6 +49,7 @@ const settings = require('../constants/settings') const siteTags = require('../constants/siteTags') const dragTypes = require('../constants/dragTypes') const keyCodes = require('../constants/keyCodes') +const isWindows = process.platform === 'win32' // State handling const basicAuthState = require('../../app/common/state/basicAuthState') @@ -893,7 +894,7 @@ class Main extends ImmutableComponent { braveShieldsDown: !braverySettings.shieldsUp })} onClick={this.onBraveMenu} /> - {process.platform === 'darwin' ? null : } + { isWindows ? : null } diff --git a/js/stores/appStore.js b/js/stores/appStore.js index d23d5a80b74..cccf8cd595c 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -28,7 +28,6 @@ const EventEmitter = require('events').EventEmitter const Immutable = require('immutable') const diff = require('immutablediff') const debounce = require('../lib/debounce.js') -const isDarwin = process.platform === 'darwin' const locale = require('../../app/locale') const path = require('path') @@ -36,6 +35,8 @@ const path = require('path') const basicAuthState = require('../../app/common/state/basicAuthState') const extensionState = require('../../app/common/state/extensionState') const tabState = require('../../app/common/state/tabState') +const isDarwin = process.platform === 'darwin' +const isWindows = process.platform === 'win32' // Only used internally const CHANGE_EVENT = 'app-state-change' @@ -134,10 +135,10 @@ const createWindow = (browserOpts, defaults, frameOpts, windowState) => { // frame: false, // A frame but no title bar and windows buttons in titlebar 10.10 OSX and up only? titleBarStyle: 'hidden-inset', - autoHideMenuBar: autoHideMenuBarSetting, + autoHideMenuBar: false,//autoHideMenuBarSetting, title: appConfig.name, webPreferences: defaults.webPreferences, - frame: (process.platform === 'darwin') + frame: !isWindows } if (process.platform === 'linux') { @@ -146,6 +147,8 @@ const createWindow = (browserOpts, defaults, frameOpts, windowState) => { let mainWindow = new BrowserWindow(Object.assign(windowProps, browserOpts)) + mainWindow.setMenuBarVisibility(true) + if (windowState.ui && windowState.ui.isMaximized) { mainWindow.maximize() } diff --git a/less/navigationBar.less b/less/navigationBar.less index 3f5f2ed2b91..9936fb15028 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -33,7 +33,7 @@ .window-header { -webkit-user-select: none; -webkit-app-region: drag; - height: 18px; + height: 18px; padding: 3px 0; margin: 0px; display: flex; @@ -113,52 +113,6 @@ } } - - .linux { - button { - outline: 0; - cursor: pointer; - width: 14px; - height: 14px; - color: transparent; - background-color: transparent; - background-position: center; - background-repeat: no-repeat; - border-width: 0; - border-radius: 50%; - display: inline-block; - padding: 0px; - margin-right: 15px; - } - - min-btn:backdrop, .max-btn:backdrop, .close-btn:backdrop { - opacity: 1; - } - - .close-btn { - background: url('../img/window-actions/linux/titlebutton-close.png') 0 0 / contain no-repeat; - } - - .min-btn { - background: url('../img/window-actions/linux/titlebutton-minimize.png') 0 0 / contain no-repeat; - } - - .max-btn { - background: url('../img/window-actions/linux/titlebutton-maximize.png') 0 0 / contain no-repeat; - } - - .close-btn:hover { - background: url('../img/window-actions/linux/titlebutton-close-hover.png') 0 0 / contain no-repeat; - } - - .min-btn:hover { - background: url('../img/window-actions/linux/titlebutton-minimize-hover.png') 0 0 / contain no-repeat; - } - - .max-btn:hover { - background: url('../img/window-actions/linux/titlebutton-maximize-hover.png') 0 0 / contain no-repeat; - } - } } // Here I grouped the rules to keep the layout of the top bar consistent. diff --git a/test/components/windowTest.js b/test/components/windowTest.js index 596322e3355..b8afa5de7ed 100644 --- a/test/components/windowTest.js +++ b/test/components/windowTest.js @@ -2,6 +2,7 @@ const Brave = require('../lib/brave') const {activeWebview, minimizeButton, maximizeButton, closeButton} = require('../lib/selectors') +const isWindows = process.platform === 'win32' describe('application window', function () { describe('application launch', function () { @@ -146,7 +147,7 @@ describe('application window', function () { }) }) - if (process.platform !== 'darwin') { + if (isWindows) { describe('window top action buttons', function () { Brave.beforeAll(this) From a20ab35311f9c3819ebf0c5cc047473130c6330d Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 5 Sep 2016 16:17:12 -0700 Subject: [PATCH 03/17] Optimize titlebar space on Windows (initial implementation) Reworked styles for caption buttons. - Windows 10 styles are implemented - Windows 7 styles are implemented - Styles have minor issues, but are really close. The close button on both needs work. - Removed the now unused SVG files New Menubar control and styles. Fixes https://github.com/brave/browser-laptop/issues/3036 This includes: - AppStore has value for menubarTemplate - WindowStore has value for menubarVisible - Updates to `states.md` - Toggle now works via ALT; menubar can be shown/hidden. - `main.js` will now show only the caption buttons (min/max/close) OR the menu and caption buttons. - Clicking MenubarItem control will pop up a contextMenu control - Wired up ContextMenu (popped up by menubar) to call new `clickMenubarItem` action Also included: - Removed unused getParentMenuDetails method from `siteUtils.js` --- app/browser/lib/menuUtil.js | 25 -- app/browser/menu.js | 11 + app/renderer/components/menubar.js | 63 ++++ app/sessionStore.js | 3 +- docs/state.md | 6 + .../windows/10/close-btn-white.svg | 63 ---- img/window-actions/windows/10/close-btn.svg | 62 ---- .../windows/10/max-btn-fullscreen.svg | 79 ----- img/window-actions/windows/10/max-btn.svg | 74 ---- img/window-actions/windows/10/min-btn.svg | 74 ---- js/actions/appActions.js | 11 + js/actions/windowActions.js | 23 ++ js/components/main.js | 26 +- ...owActionBar.js => windowCaptionButtons.js} | 24 +- js/constants/appConstants.js | 3 +- js/constants/keyCodes.js | 1 + js/constants/windowConstants.js | 2 + js/stores/appStore.js | 4 +- js/stores/windowStore.js | 10 +- less/contextMenu.less | 25 ++ less/navigationBar.less | 335 ++++++++++++++++-- test/unit/lib/menuUtilTest.js | 24 -- 22 files changed, 494 insertions(+), 454 deletions(-) create mode 100644 app/renderer/components/menubar.js delete mode 100644 img/window-actions/windows/10/close-btn-white.svg delete mode 100644 img/window-actions/windows/10/close-btn.svg delete mode 100644 img/window-actions/windows/10/max-btn-fullscreen.svg delete mode 100644 img/window-actions/windows/10/max-btn.svg delete mode 100644 img/window-actions/windows/10/min-btn.svg rename js/components/{windowActionBar.js => windowCaptionButtons.js} (58%) diff --git a/app/browser/lib/menuUtil.js b/app/browser/lib/menuUtil.js index c8b6b55be2e..dc07b7515e2 100644 --- a/app/browser/lib/menuUtil.js +++ b/app/browser/lib/menuUtil.js @@ -10,31 +10,6 @@ const eventUtil = require('../../../js/lib/eventUtil') const siteUtil = require('../../../js/state/siteUtil') const locale = require('../../locale') -/** - * Get an electron MenuItem object for the PARENT menu (File, Edit, etc) based on its label - * @param {string} label - the text associated with the menu - * NOTE: label may be a localized string - */ -module.exports.getParentMenuDetails = (appMenu, label) => { - let menuIndex = -1 - let menuItem = null - - if (label && appMenu && appMenu.items && appMenu.items.length > 0) { - menuIndex = appMenu.items.findIndex(function (item, index) { - return item && item.label === label - }) - - if (menuIndex !== -1) { - menuItem = appMenu.items[menuIndex] - } - } - - return { - menu: menuItem, - index: menuIndex - } -} - /** * Get the an electron MenuItem object from a Menu based on its label * @param {string} label - the text associated with the menu diff --git a/app/browser/menu.js b/app/browser/menu.js index 8b50f402de8..4951f88cb6a 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -18,6 +18,7 @@ const messages = require('../../js/constants/messages') const settings = require('../../js/constants/settings') const siteTags = require('../../js/constants/siteTags') const dialog = electron.dialog +const BrowserWindow = electron.BrowserWindow const { fileUrl } = require('../../js/lib/appUrlUtil') const menuUtil = require('./lib/menuUtil') const getSetting = require('../../js/settings').getSetting @@ -586,6 +587,8 @@ const createMenu = () => { }) } + appActions.setMenubarTemplate(Immutable.fromJS(template)) + let oldMenu = appMenu appMenu = Menu.buildFromTemplate(template) Menu.setApplicationMenu(appMenu) @@ -643,6 +646,14 @@ const doAction = (action) => { }) } break + case windowConstants.WINDOW_CLICK_MENUBAR_ITEM: + appDispatcher.waitFor([appStore.dispatchToken], () => { + const clickedMenuItem = menuUtil.getMenuItem(appMenu, action.label) + if (clickedMenuItem) { + clickedMenuItem.click(clickedMenuItem, BrowserWindow.getFocusedWindow()) + } + }) + break default: } } diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js new file mode 100644 index 00000000000..3ef73fcfaab --- /dev/null +++ b/app/renderer/components/menubar.js @@ -0,0 +1,63 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const React = require('react') +const Immutable = require('immutable') +const ImmutableComponent = require('../../../js/components/immutableComponent') +const windowActions = require('../../../js/actions/windowActions') +const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem + +class MenubarItem extends ImmutableComponent { + constructor () { + super() + this.onClick = this.onClick.bind(this) + } + + onClick (e) { + if (e && e.stopPropagation) { + e.stopPropagation() + } + + const rect = e.target.getBoundingClientRect() + windowActions.setContextMenuDetail(Immutable.fromJS({ + left: rect.left, + top: rect.bottom, + template: this.props.submenu.map((submenuItem) => { + if (submenuItem.type === separatorMenuItem.type) { + return submenuItem + } + submenuItem.click = function (e) { + windowActions.clickMenubarItem(submenuItem.label) + } + return submenuItem + }) + })) + } + + render () { + return + { this.props.label } + + } +} + +/** + * Menubar that can be optionally be displayed at the top of a window (in favor of the system menu). + * First intended use is with Windows to enable a slim titlebar. + * NOTE: the system menu is still created and used in order to keep the accelerators working. + */ +class Menubar extends ImmutableComponent { + render () { + return
+ { + this.props.template.map((menubarItem) => + ) + } +
+ } +} + +module.exports = Menubar diff --git a/app/sessionStore.js b/app/sessionStore.js index 0df60aa1a22..63bc076f908 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -412,6 +412,7 @@ module.exports.defaultAppState = () => { guid: [], timestamp: 0 } - } + }, + menubar: {} } } diff --git a/docs/state.md b/docs/state.md index b6cf143c2ee..672bd291236 100644 --- a/docs/state.md +++ b/docs/state.md @@ -219,6 +219,9 @@ AppStore newtab: { gridLayout: string // 'small', 'medium', 'large' } + }, + menubar: { + template: object // windows only: template object with Menubar control } } ``` @@ -367,6 +370,9 @@ WindowStore }, releaseNotes: { isVisible: boolean, // Whether or not to show release notes + }, + menubar: { + isVisible: boolean // windows only: true if Menubar control is visible } }, searchDetail: { diff --git a/img/window-actions/windows/10/close-btn-white.svg b/img/window-actions/windows/10/close-btn-white.svg deleted file mode 100644 index 72e603cbd55..00000000000 --- a/img/window-actions/windows/10/close-btn-white.svg +++ /dev/null @@ -1,63 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - diff --git a/img/window-actions/windows/10/close-btn.svg b/img/window-actions/windows/10/close-btn.svg deleted file mode 100644 index 8dc9e16250f..00000000000 --- a/img/window-actions/windows/10/close-btn.svg +++ /dev/null @@ -1,62 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - diff --git a/img/window-actions/windows/10/max-btn-fullscreen.svg b/img/window-actions/windows/10/max-btn-fullscreen.svg deleted file mode 100644 index c534778f540..00000000000 --- a/img/window-actions/windows/10/max-btn-fullscreen.svg +++ /dev/null @@ -1,79 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - - - - - - diff --git a/img/window-actions/windows/10/max-btn.svg b/img/window-actions/windows/10/max-btn.svg deleted file mode 100644 index c6beccbfddb..00000000000 --- a/img/window-actions/windows/10/max-btn.svg +++ /dev/null @@ -1,74 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - - - - - diff --git a/img/window-actions/windows/10/min-btn.svg b/img/window-actions/windows/10/min-btn.svg deleted file mode 100644 index 22f67f36884..00000000000 --- a/img/window-actions/windows/10/min-btn.svg +++ /dev/null @@ -1,74 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - - - - - diff --git a/js/actions/appActions.js b/js/actions/appActions.js index b32c7e478c0..ce8fbb88ae7 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -466,6 +466,17 @@ const appActions = { }) }, + /** + * Saves current menubar template for use w/ Windows titlebar + * @param {Object} menubarTemplate - JSON used to build the menu + */ + setMenubarTemplate: function (menubarTemplate) { + AppDispatcher.dispatch({ + actionType: AppConstants.APP_SET_MENUBAR_TEMPLATE, + menubarTemplate + }) + }, + /** * Dispatches a message when the network is re-connected * after being disconnected diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 820159d4675..0d114dc975a 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1007,6 +1007,29 @@ const windowActions = { }) }, + /** + * (Windows only) + * Dispatches a message to indicate the custom rendered Menubar should be toggled (shown/hidden) + */ + toggleMenubarVisible: function () { + dispatch({ + actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE + }) + }, + + /** + * (Windows only) + * Used to trigger the click() action for a menu + * Called from the Menubar control, handled in menu.js + * @param {string} itemLabel - text of the label that was clicked + */ + clickMenubarItem: function (label) { + dispatch({ + actionType: WindowConstants.WINDOW_CLICK_MENUBAR_ITEM, + label + }) + }, + gotResponseDetails: function (tabId, details) { dispatch({ actionType: WindowConstants.WINDOW_GOT_RESPONSE_DETAILS, diff --git a/js/components/main.js b/js/components/main.js index e20c38cd82e..4eaac077e1c 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -24,6 +24,7 @@ const FindBar = require('./findbar.js') const UpdateBar = require('./updateBar') const NotificationBar = require('./notificationBar') const DownloadsBar = require('./downloadsBar') +const Menubar = require('../../app/renderer/components/menubar') const Button = require('./button') const BrowserActionButton = require('../../app/renderer/components/browserActionButton') const SiteInfo = require('./siteInfo') @@ -39,7 +40,7 @@ const ContextMenu = require('./contextMenu') const PopupWindow = require('./popupWindow') const NoScriptInfo = require('./noScriptInfo') const LongPressButton = require('./longPressButton') -const WindowActionBar = require('./windowActionBar') +const WindowCaptionButtons = require('./windowCaptionButtons') // Constants const config = require('../constants/config') @@ -118,6 +119,11 @@ class Main extends ImmutableComponent { ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_ZOOM_OUT) } break + case keyCodes.ALT: + if (isWindows) { + windowActions.toggleMenubarVisible() + } + break } }) } @@ -742,6 +748,10 @@ class Main extends ImmutableComponent { const braverySettings = siteSettings.activeSettings(activeSiteSettings, this.props.appState, appConfig) const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null + const menubarEnabled = isWindows + const menubarVisible = menubarEnabled && this.props.windowState.getIn(['ui', 'menubar', 'isVisible']) + const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null + const shouldAllowWindowDrag = !this.props.windowState.get('contextMenuDetail') && !this.props.windowState.get('bookmarkDetail') && !siteInfoIsVisible && @@ -774,6 +784,14 @@ class Main extends ImmutableComponent { : null }
+ { + menubarVisible + ?
+ + +
+ : null + }
- { isWindows ? : null }
+ { + menubarEnabled && !menubarVisible + ? + : null + }
{ diff --git a/js/components/windowActionBar.js b/js/components/windowCaptionButtons.js similarity index 58% rename from js/components/windowActionBar.js rename to js/components/windowCaptionButtons.js index 3a763a06986..001e1176111 100644 --- a/js/components/windowActionBar.js +++ b/js/components/windowCaptionButtons.js @@ -7,7 +7,7 @@ const React = require('react') const ImmutableComponent = require('./immutableComponent') const currentWindow = require('../../app/renderer/currentWindow') -class WindowActionBar extends ImmutableComponent { +class WindowCaptionButtons extends ImmutableComponent { constructor () { super() this.onDoubleClick = this.onDoubleClick.bind(this) @@ -23,15 +23,11 @@ class WindowActionBar extends ImmutableComponent { getPlatformCssClass () { switch (os.platform()) { - case 'linux': - return 'linux' case 'win32': - if (/10.0./.test(os.release())) { - return 'win-10' - } else if (/6.1./.test(os.release())) { - return 'win-7' + if (/6.1./.test(os.release())) { + return 'win7' } else { - return 'win' + return 'win10' } default: return 'hidden' @@ -58,14 +54,14 @@ class WindowActionBar extends ImmutableComponent { } render () { - return
-
- - - + return
+
+ + +
} } -module.exports = WindowActionBar +module.exports = WindowCaptionButtons diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 7fd79a236a4..6e3db31edc1 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -48,7 +48,8 @@ const AppConstants = { APP_NETWORK_DISCONNECTED: _, APP_CHANGE_NEW_TAB_DETAIL: _, APP_TAB_CREATED: _, - APP_TAB_DESTROYED: _ + APP_TAB_DESTROYED: _, + APP_SET_MENUBAR_TEMPLATE: _ } module.exports = mapValuesByKeys(AppConstants) diff --git a/js/constants/keyCodes.js b/js/constants/keyCodes.js index 0c79d3b2002..9c89654fb9d 100644 --- a/js/constants/keyCodes.js +++ b/js/constants/keyCodes.js @@ -10,6 +10,7 @@ const KeyCodes = { SHIFT: 16, BACKSPACE: 8, CTRL: 17, + ALT: 18, DELETE: 46, CMD1: 91, CMD2: 93, diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 3d9ea49b148..fade210b82f 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -69,6 +69,8 @@ const windowConstants = { WINDOW_SET_AUTOFILL_ADDRESS_DETAIL: _, WINDOW_SET_AUTOFILL_CREDIT_CARD_DETAIL: _, WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT: _, + WINDOW_TOGGLE_MENUBAR_VISIBLE: _, + WINDOW_CLICK_MENUBAR_ITEM: _, WINDOW_GOT_RESPONSE_DETAILS: _ } diff --git a/js/stores/appStore.js b/js/stores/appStore.js index cccf8cd595c..4bc5eba9df6 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -135,7 +135,7 @@ const createWindow = (browserOpts, defaults, frameOpts, windowState) => { // frame: false, // A frame but no title bar and windows buttons in titlebar 10.10 OSX and up only? titleBarStyle: 'hidden-inset', - autoHideMenuBar: false,//autoHideMenuBarSetting, + autoHideMenuBar: autoHideMenuBarSetting, title: appConfig.name, webPreferences: defaults.webPreferences, frame: !isWindows @@ -678,6 +678,8 @@ const handleAppAction = (action) => { break case ExtensionConstants.EXTENSION_DISABLED: appState = extensionState.extensionDisabled(appState, action) + case AppConstants.APP_SET_MENUBAR_TEMPLATE: + appState = appState.setIn(['menu', 'template'], action.menubarTemplate) break default: } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index dc2a002ee2f..6aec176d5d6 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -31,7 +31,9 @@ let windowState = Immutable.fromJS({ ui: { tabs: { }, - mouseInTitlebar: false + mouseInTitlebar: false, + menubar: { + } }, searchDetail: null }) @@ -777,6 +779,12 @@ const doAction = (action) => { windowState.deleteIn(blockedRunInsecureContentPath.concat(['security', 'blockedRunInsecureContent'])) } break + case WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE: + // BSCTODO: ignore if always show menu is enabled + const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) + windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + break + default: } diff --git a/less/contextMenu.less b/less/contextMenu.less index 1b2549d5e37..5e504a11990 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -145,3 +145,28 @@ font-size: @bookmarksFolderIconSize; } } + +// Menubar (for use w/ slim titlebar) +.titlebar { + -webkit-user-select: none; + -webkit-app-region: drag; + + .menubar { + display: inline-block; + cursor: default; + + .menubarItem { + color: black; + font: menu; + margin-right: 2px; + -webkit-app-region: no-drag; + padding: 0 4px 1px; + border: 1px solid transparent; + + &:hover { + background-color: #e5f3ff; + border: 1px solid #cce8ff; + } + } + } +} diff --git a/less/navigationBar.less b/less/navigationBar.less index 9936fb15028..43341b5762e 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -29,70 +29,236 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } -// Window Header -.window-header { +// Window Caption Buttons (for use w/ slim titlebar) +.windowCaptionButtons { + display: inline-block; + float: right; -webkit-user-select: none; -webkit-app-region: drag; - height: 18px; - padding: 3px 0; - margin: 0px; - display: flex; - line-height: 30px; + height: 100%; .hidden { display: none; } - .title-bar-btns { + .container { -webkit-app-region: no-drag; } - .win-7, .win-10 { - margin: -13px 3px 0 0; + button.captionButton { + outline: none; + vertical-align: top; + } + + &.fullscreen { + .win7 { + margin-top: 1px; + } + } + + .win7 { + margin-right: 5px; - button.win-action-btn { + button.captionButton { outline: 0; - cursor: pointer; - width: 45px; - height: 23px; + height: 18px; color: #b1acac; background-size: 12px 12px!important; background-position: center!important; - border: 1px solid #e1e1e1; + border: 1px solid #838383; display: inline-block; - padding: 12px; + background-color: #dfdfdf; + box-shadow: inset 1px 1px rgba(255, 255, 255, 0.9); + width: 25px; - &.fullscreen { - height:21px; - padding:6px; + &.minimize { + border-right: 0px; + &:hover { + background-color: #f5f5f5; + } + &:active { + background-color: #cacacb; + } + border-radius: 0 0 0 4px; + + .widget { + width: 10px; + height: 3px; + border: 1px solid #838383; + background: #f0f0f0; + display: inline-block; + border-radius: 1px; + } } - &.close-btn { - background: url('../img/window-actions/windows/10/close-btn.svg') no-repeat; + &.maximize { + border-right: 0px; &:hover { - background: url('../img/window-actions/windows/10/close-btn-white.svg') no-repeat; - background-color: #e5182c; + background-color: #f5f5f5; + .widget { + .widget2 { + background-color: #f5f5f5; + } + } } &:active { - background: url('../img/window-actions/windows/10/close-btn-white.svg') no-repeat; - background-color: #ef717c; + background-color: #cacacb; + .widget { + .widget2 { + background-color: #cacacb; + } + } + } + &.fullscreen { + &:hover { + background-color: #e5e5e5; + } + &:active { + background-color: #cacacb; + } + .widget { + .widget1 { + width: 8px; + top: 2px; + left: 8px; + } + .widget2 { + width: 8px; + height: 8px; + top: -5px; + left: 4px; + background: #f0f0f0; + border-radius: 1px; + } + .widget3 { + display: inline-block; + width: 2px; + height: 2px; + border: 1px solid #838383; + background: #f0f0f0; + position: relative; + top: -20px; + left: -3px; + } + } + } + + .widget { + .widget1 { + width: 10px; + height: 8px; + border: 1px solid #838383; + background: #f0f0f0; + position: relative; + top: 2px; + left: 6px; + border-radius: 1px; + } + .widget2 { + width: 4px; + height: 2px; + border: 1px solid #838383; + background-color: #dfdfdf; + position: relative; + top: -5px; + left: 9px; + border-radius: 0; + } + .widget3 { display: none; } + .widget4 { display: none; } + .widget5 { display: none; } } } - &.min-btn { - border-right: 0px; - border-bottom-left-radius: 5px; - background: url('../img/window-actions/windows/10/min-btn.svg') no-repeat; + &.close { + width: 43px; + border-radius: 0 0 4px 0; + &:hover { + background-color: #d9504e; + .widget { + .widget1 { border: 1px solid black; } + .widget2 { border: 1px solid black; } + } + } + &:active { + background-color: #d7393d; + .widget { + .widget1 { border: 1px solid black; } + .widget2 { border: 1px solid black; } + } + } + + .widget { + width: 11px; + height: 11px; + display: inline-block; + vertical-align: middle; + + .widget1 { + width: 10px; + height: 2px; + background: #f0f0f0; + display: inline-block; + transform: rotate(45deg); + position: relative; + top: -6px; + left: -2px; + border: 1px solid #838383; + } + .widget2 { + width: 10px; + height: 2px; + background: #f0f0f0; + display: inline-block; + transform: rotate(315deg); + position: relative; + top: -21px; + left: -2px; + border: 1px solid #838383; + } + .widget3 { + width: 10px; + height: 2px; + background: #f0f0f0; + display: inline-block; + transform: rotate(45deg); + position: relative; + top: -37px; + left: -2px; + } + } + } + } + } + + .win10 { + button.captionButton { + width: 45px; + height: 29px; + border: 0; + background-color: transparent; + + &.fullscreen { + height:21px; + } + + &.minimize { &:hover { background-color: #e5e5e5; } &:active { background-color: #cacacb; } + .widget { + width: 10px; + height: 1px; + background: black; + display: inline-block; + vertical-align: middle; + } } - &.max-btn { - background: url('../img/window-actions/windows/10/max-btn.svg') no-repeat; + &.maximize { border-right: 0px; &:hover { background-color: #e5e5e5; @@ -101,17 +267,118 @@ background-color: #cacacb; } &.fullscreen { - background: url('../img/window-actions/windows/10/max-btn-fullscreen.svg') no-repeat; &:hover { background-color: #e5e5e5; } &:active { background-color: #cacacb; } + .widget { + margin-top: 8px; + .widget1 { width: 6px; height: 6px; } + .widget2 { display: inline-block; } + .widget3 { + display: inline-block; + width: 8px; + height: 1px; + background: black; + position: relative; + top: -21px; + left: 1px; + } + .widget4 { + display: inline-block; + width: 1px; + height: 8px; + background: black; + position: relative; + top: -14px; + left: 0px; + } + .widget5 { + display: inline-block; + width: 2px; + height: 1px; + background: black; + position: relative; + top: -14px; + left: -2px; + } + } + } + .widget { + width: 12px; + height: 12px; + display: inline-block; + vertical-align: middle; + + .widget1 { + width: 8px; + height: 8px; + border: 1px solid black; + background: none; + position: relative; + } + .widget2 { + display: none; + width: 1px; + height: 2px; + background: black; + position: relative; + top: -20px; + left: 2px; + } + .widget3 { display: none; } + .widget4 { display: none; } + .widget5 { display: none; } } } - } + &.close { + &:hover { + background-color: #e5182c; + .widget { + .widget1{ background:white; } + .widget2{ background:white; } + } + } + &:active { + background-color: #ef717c; + .widget { + .widget1{ background:white; } + .widget2{ background:white; } + } + } + .widget { + width: 11px; + height: 11px; + display: inline-block; + vertical-align: middle; + + .widget1 { + width: 14px; + height: 1px; + background: black; + display: inline-block; + transform: rotate(45deg); + position: relative; + top: -6px; + left: -2px; + } + .widget2 { + width: 14px; + height: 1px; + background: black; + display: inline-block; + transform: rotate(315deg); + position: relative; + top: -21px; + left: -2px; + } + .widget3 { display: none; } + } + } + } } } @@ -120,6 +387,8 @@ // box on its left has different size than the one on the right. // This is achieved by the local variable `@centerOffset`. .navigatorWrapper { + clear:both; + // Since we want to keep the navigator centered, we need to calculate the // difference between the width of the left box and the width of the right box. @centerOffset: 2 * (@navbarButtonWidth + @navbarButtonSpacing) // width area on the left diff --git a/test/unit/lib/menuUtilTest.js b/test/unit/lib/menuUtilTest.js index e5ad3b06934..6e786572f0a 100644 --- a/test/unit/lib/menuUtilTest.js +++ b/test/unit/lib/menuUtilTest.js @@ -58,30 +58,6 @@ describe('menuUtil', function () { mockery.disable() }) - describe('getParentMenuDetails', function () { - const emptyValue = { - menu: null, - index: -1 - } - it('returns an object with the electron MenuItem/index based on the label', function () { - const menu = menuUtil.getParentMenuDetails(defaultMenu, 'Edit') - assert.equal(menu.index, 1) - assert.equal(menu.menu, defaultMenu.items[1]) - }) - it('returns an object with null/-1 if input menu is not truthy', function () { - const menu = menuUtil.getParentMenuDetails(null, 'Edit') - assert.deepEqual(menu, emptyValue) - }) - it('returns an object with null/-1 if label is not truthy', function () { - const menu = menuUtil.getParentMenuDetails(defaultMenu, undefined) - assert.deepEqual(menu, emptyValue) - }) - it('returns an object with null/-1 if label is not found', function () { - const menu = menuUtil.getParentMenuDetails(defaultMenu, 'History') - assert.deepEqual(menu, emptyValue) - }) - }) - describe('getMenuItem', function () { it('returns the electron MenuItem based on the label', function () { const menuItem = menuUtil.getMenuItem(defaultMenu, 'quit') From 8fcf66a506321115dcf784bfda86233564370186 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 9 Sep 2016 23:44:52 -0700 Subject: [PATCH 04/17] Fixing known issues: - Moved windowCaptionButtons control to new preferred location - Added title element to min/max/restore/close caption buttons - Menu font is now fixed 12pt; It's probably a good idea to set a face (currently it defaults to using system font) - Menu now lines up properly with caption buttons. It also has proper padding on top/bottom per Brad's mockup - Set a height for the menubar control (same height as the win10 caption buttons. This allows the draggable CSS properties to properly work, making the window easy to move. - Menubar now respects the AUTO_HIDE_MENU setting --- .../brave/locales/en-US/app.properties | 4 +++ app/locale.js | 7 ++++- .../components/windowCaptionButtons.js | 26 +++++++++++++++---- js/components/main.js | 6 ++--- js/stores/windowStore.js | 7 ++--- less/contextMenu.less | 7 +++-- 6 files changed, 43 insertions(+), 14 deletions(-) rename {js => app/renderer}/components/windowCaptionButtons.js (56%) diff --git a/app/extensions/brave/locales/en-US/app.properties b/app/extensions/brave/locales/en-US/app.properties index 03c1b48e0b9..bffc6ff33f5 100644 --- a/app/extensions/brave/locales/en-US/app.properties +++ b/app/extensions/brave/locales/en-US/app.properties @@ -202,3 +202,7 @@ dismissDenyRunInsecureContent=Stay Insecure denyRunInsecureContent=Stop Loading Unsafe Scripts runInsecureContentWarning=This page is trying to load scripts from insecure sources. If you allow this content to run it will not be encrypted and it may transmit unencrypted data to other sites. denyRunInsecureContentWarning=This page is currently loading scripts from insecure sources. +windowCaptionButtonMinimize=Minimize +windowCaptionButtonMaximize=Maximize +windowCaptionButtonRestore=Restore Down +windowCaptionButtonClose=Close diff --git a/app/locale.js b/app/locale.js index fd0b648d0fb..14006d0b5b8 100644 --- a/app/locale.js +++ b/app/locale.js @@ -207,7 +207,12 @@ var rendererIdentifiers = function () { 'downloadItemDelete', 'downloadItemClear', 'downloadToolbarHide', - 'downloadItemClearCompleted' + 'downloadItemClearCompleted', + // Caption buttons in titlebar (min/max/close - Windows only) + 'windowCaptionButtonMinimize', + 'windowCaptionButtonMaximize', + 'windowCaptionButtonRestore', + 'windowCaptionButtonClose' ] } diff --git a/js/components/windowCaptionButtons.js b/app/renderer/components/windowCaptionButtons.js similarity index 56% rename from js/components/windowCaptionButtons.js rename to app/renderer/components/windowCaptionButtons.js index 001e1176111..4b91ddae83c 100644 --- a/js/components/windowCaptionButtons.js +++ b/app/renderer/components/windowCaptionButtons.js @@ -4,8 +4,9 @@ const os = require('os') const React = require('react') -const ImmutableComponent = require('./immutableComponent') -const currentWindow = require('../../app/renderer/currentWindow') +const ImmutableComponent = require('../../../js/components/immutableComponent') +const locale = require('../../../js/l10n') +const currentWindow = require('../currentWindow') class WindowCaptionButtons extends ImmutableComponent { constructor () { @@ -56,9 +57,24 @@ class WindowCaptionButtons extends ImmutableComponent { render () { return
- - - + + +
} diff --git a/js/components/main.js b/js/components/main.js index 4eaac077e1c..2234ddae9f4 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -24,7 +24,6 @@ const FindBar = require('./findbar.js') const UpdateBar = require('./updateBar') const NotificationBar = require('./notificationBar') const DownloadsBar = require('./downloadsBar') -const Menubar = require('../../app/renderer/components/menubar') const Button = require('./button') const BrowserActionButton = require('../../app/renderer/components/browserActionButton') const SiteInfo = require('./siteInfo') @@ -40,7 +39,8 @@ const ContextMenu = require('./contextMenu') const PopupWindow = require('./popupWindow') const NoScriptInfo = require('./noScriptInfo') const LongPressButton = require('./longPressButton') -const WindowCaptionButtons = require('./windowCaptionButtons') +const Menubar = require('../../app/renderer/components/menubar') +const WindowCaptionButtons = require('../../app/renderer/components/windowCaptionButtons') // Constants const config = require('../constants/config') @@ -749,7 +749,7 @@ class Main extends ImmutableComponent { const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null const menubarEnabled = isWindows - const menubarVisible = menubarEnabled && this.props.windowState.getIn(['ui', 'menubar', 'isVisible']) + const menubarVisible = menubarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null const shouldAllowWindowDrag = !this.props.windowState.get('contextMenuDetail') && diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 6aec176d5d6..e849274e35a 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -780,9 +780,10 @@ const doAction = (action) => { } break case WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE: - // BSCTODO: ignore if always show menu is enabled - const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) - windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + if (getSetting(settings.AUTO_HIDE_MENU)) { + const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) + windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + } break default: diff --git a/less/contextMenu.less b/less/contextMenu.less index 5e504a11990..a5de7385e6e 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -150,17 +150,20 @@ .titlebar { -webkit-user-select: none; -webkit-app-region: drag; + height: 29px; .menubar { display: inline-block; cursor: default; + margin-top: 5px; + margin-left: 4px; .menubarItem { color: black; font: menu; - margin-right: 2px; + font-size: 12px; -webkit-app-region: no-drag; - padding: 0 4px 1px; + padding: 0 10px 1px; border: 1px solid transparent; &:hover { From f2426f1c357fc9e30a0efeae7c2c8f0f5e4c16af Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sun, 11 Sep 2016 13:08:51 -0700 Subject: [PATCH 05/17] Navigation bar now is properly contained separate of the caption buttons. This works exactly like we want. However, the modals that can be popped up are showing up inside the container. Looking at a fix for that... --- js/components/main.js | 114 +++++++++++++++++++++++----------------- less/contextMenu.less | 28 ---------- less/navigationBar.less | 49 ++++++++++++++++- 3 files changed, 112 insertions(+), 79 deletions(-) diff --git a/js/components/main.js b/js/components/main.js index 2234ddae9f4..e57113d4c72 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -784,57 +784,72 @@ class Main extends ImmutableComponent { : null }
- { - menubarVisible - ?
- - +
+
+
+ { + menubarVisible + ?
+ +
+ : null + } +
+
+ + +
+ { this.navBar = node }} + navbar={activeFrame && activeFrame.get('navbar')} + frames={this.props.windowState.get('frames')} + sites={this.props.appState.get('sites')} + activeFrameKey={activeFrame && activeFrame.get('key') || undefined} + location={activeFrame && activeFrame.get('location') || ''} + title={activeFrame && activeFrame.get('title') || ''} + scriptsBlocked={activeFrame && activeFrame.getIn(['noScript', 'blocked'])} + partitionNumber={activeFrame && activeFrame.get('partitionNumber') || 0} + history={activeFrame && activeFrame.get('history') || emptyList} + suggestionIndex={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) || 0} + isSecure={activeFrame && activeFrame.getIn(['security', 'isSecure'])} + locationValueSuffix={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'urlSuffix']) || ''} + startLoadTime={activeFrame && activeFrame.get('startLoadTime') || undefined} + endLoadTime={activeFrame && activeFrame.get('endLoadTime') || undefined} + loading={activeFrame && activeFrame.get('loading')} + mouseInTitlebar={this.props.windowState.getIn(['ui', 'mouseInTitlebar'])} + searchDetail={this.props.windowState.get('searchDetail')} + enableNoScript={this.enableNoScript(activeSiteSettings)} + settings={this.props.appState.get('settings')} + noScriptIsVisible={noScriptIsVisible} + /> +
+
+
- : null - } -
-
- -
- { this.navBar = node }} - navbar={activeFrame && activeFrame.get('navbar')} - frames={this.props.windowState.get('frames')} - sites={this.props.appState.get('sites')} - activeFrameKey={activeFrame && activeFrame.get('key') || undefined} - location={activeFrame && activeFrame.get('location') || ''} - title={activeFrame && activeFrame.get('title') || ''} - scriptsBlocked={activeFrame && activeFrame.getIn(['noScript', 'blocked'])} - partitionNumber={activeFrame && activeFrame.get('partitionNumber') || 0} - history={activeFrame && activeFrame.get('history') || emptyList} - suggestionIndex={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) || 0} - isSecure={activeFrame && activeFrame.getIn(['security', 'isSecure'])} - locationValueSuffix={activeFrame && activeFrame.getIn(['navbar', 'urlbar', 'suggestions', 'urlSuffix']) || ''} - startLoadTime={activeFrame && activeFrame.get('startLoadTime') || undefined} - endLoadTime={activeFrame && activeFrame.get('endLoadTime') || undefined} - loading={activeFrame && activeFrame.get('loading')} - mouseInTitlebar={this.props.windowState.getIn(['ui', 'mouseInTitlebar'])} - searchDetail={this.props.windowState.get('searchDetail')} - enableNoScript={this.enableNoScript(activeSiteSettings)} - settings={this.props.appState.get('settings')} - noScriptIsVisible={noScriptIsVisible} - /> { siteInfoIsVisible ? + { this.props.appState.get('notifications') && this.props.appState.get('notifications').size && activeFrame diff --git a/less/contextMenu.less b/less/contextMenu.less index a5de7385e6e..1b2549d5e37 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -145,31 +145,3 @@ font-size: @bookmarksFolderIconSize; } } - -// Menubar (for use w/ slim titlebar) -.titlebar { - -webkit-user-select: none; - -webkit-app-region: drag; - height: 29px; - - .menubar { - display: inline-block; - cursor: default; - margin-top: 5px; - margin-left: 4px; - - .menubarItem { - color: black; - font: menu; - font-size: 12px; - -webkit-app-region: no-drag; - padding: 0 10px 1px; - border: 1px solid transparent; - - &:hover { - background-color: #e5f3ff; - border: 1px solid #cce8ff; - } - } - } -} diff --git a/less/navigationBar.less b/less/navigationBar.less index 43341b5762e..b8798bc7121 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -29,10 +29,28 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } +// Style adjustments needed for slim titlebar to work properly +.navbarCaptionButtonContainer { + display: flex; + border-bottom: 1px solid #aaaaaa; +} + +.navbarMenubarFlexContainer { + display: flex; + flex: 1; + box-sizing: border-box; + position: relative; + overflow: auto; + white-space: nowrap; +} +.navbarMenubarBlockContainer { + display: block; + width: 100%; +} + // Window Caption Buttons (for use w/ slim titlebar) .windowCaptionButtons { display: inline-block; - float: right; -webkit-user-select: none; -webkit-app-region: drag; height: 100%; @@ -382,6 +400,34 @@ } } +// Menubar (for use w/ slim titlebar) +.menubarContainer { + -webkit-user-select: none; + -webkit-app-region: drag; + height: 29px; + + .menubar { + display: inline-block; + cursor: default; + margin-top: 5px; + margin-left: 4px; + + .menubarItem { + color: black; + font: menu; + font-size: 12px; + -webkit-app-region: no-drag; + padding: 0 10px 1px; + border: 1px solid transparent; + + &:hover { + background-color: #e5f3ff; + border: 1px solid #cce8ff; + } + } + } +} + // Here I grouped the rules to keep the layout of the top bar consistent. // The tricky part is to keep the title in `#navigator` centered when the // box on its left has different size than the one on the right. @@ -474,7 +520,6 @@ justify-content: space-between; -webkit-app-region: drag; align-items: center; - border-bottom: 1px solid #aaaaaa; height: @navbarHeight; box-sizing: border-box; From 3199f4b14db4b7f80709b7cd0947e10fd513e776 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sun, 11 Sep 2016 16:44:40 -0700 Subject: [PATCH 06/17] Fixing known issues: - Fixes to properly position the modals and URL suggestions - Toggle method for menu can now include bool; default is that it'll toggle. - Remove unused style - Menubar items now auto-popup context menu when moused over (if one was initially clicked). This selection is not cleared yet but otherwise works great - Menus now close (and autohide themselves) after selection is cancelled - new selectedLabel state w/ action - created new resetMenu action which closes context menus, popup menus, and this new menubar control (if needed). - updated main.js to call resetMenu - Menu hides after selection made (when auto-hide is true) - Fix issue where pressing alt (even when auto-hide menu is disabled) hides context menus. - Keyboard accelerators now show on context menus. Mac version may not look right, needs to be tested. --- app/browser/menu.js | 2 +- app/renderer/components/menubar.js | 42 ++++++-- docs/state.md | 5 +- js/actions/windowActions.js | 37 ++++++- js/components/contextMenu.js | 74 ++++++++++++- js/components/main.js | 161 ++++++++++++++--------------- js/constants/windowConstants.js | 4 +- js/stores/appStore.js | 1 + js/stores/windowStore.js | 26 ++++- less/navigationBar.less | 10 +- 10 files changed, 255 insertions(+), 107 deletions(-) diff --git a/app/browser/menu.js b/app/browser/menu.js index 4951f88cb6a..1242da21db4 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -646,7 +646,7 @@ const doAction = (action) => { }) } break - case windowConstants.WINDOW_CLICK_MENUBAR_ITEM: + case windowConstants.WINDOW_CLICK_MENUBAR_SUBMENU: appDispatcher.waitFor([appStore.dispatchToken], () => { const clickedMenuItem = menuUtil.getMenuItem(appMenu, action.label) if (clickedMenuItem) { diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index 3ef73fcfaab..783697fa739 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -12,13 +12,22 @@ class MenubarItem extends ImmutableComponent { constructor () { super() this.onClick = this.onClick.bind(this) + this.onMouseOver = this.onMouseOver.bind(this) } onClick (e) { if (e && e.stopPropagation) { e.stopPropagation() } - + // If clicking on an already selected item, deselect it + const selected = this.props.menubar.props.selectedLabel + if (selected && selected === this.props.label) { + windowActions.setContextMenuDetail() + windowActions.setMenubarItemSelected() + return + } + // Otherwise, mark item as selected and show its context menu + windowActions.setMenubarItemSelected(this.props.label) const rect = e.target.getBoundingClientRect() windowActions.setContextMenuDetail(Immutable.fromJS({ left: rect.left, @@ -28,17 +37,25 @@ class MenubarItem extends ImmutableComponent { return submenuItem } submenuItem.click = function (e) { - windowActions.clickMenubarItem(submenuItem.label) + windowActions.clickMenubarSubmenu(submenuItem.label) } return submenuItem }) })) } + onMouseOver (e) { + const selected = this.props.menubar.props.selectedLabel + if (selected && selected !== this.props.label) { + this.onClick(e) + } + } + render () { return + className={'menubarItem' + (this.props.selected ? ' selected' : '')} + onClick={this.onClick} + onMouseOver={this.onMouseOver}> { this.props.label } } @@ -50,11 +67,24 @@ class MenubarItem extends ImmutableComponent { * NOTE: the system menu is still created and used in order to keep the accelerators working. */ class Menubar extends ImmutableComponent { + shouldComponentUpdate (nextProps, nextState) { + return this.props.selectedLabel !== nextProps.selectedLabel + } + render () { return
{ - this.props.template.map((menubarItem) => - ) + this.props.template.map((menubarItem) => { + let props = { + label: menubarItem.get('label'), + submenu: menubarItem.get('submenu').toJS(), + menubar: this + } + if (props.label === this.props.selectedLabel) { + props.selected = true + } + return + }) }
} diff --git a/docs/state.md b/docs/state.md index 672bd291236..39b88d15697 100644 --- a/docs/state.md +++ b/docs/state.md @@ -371,8 +371,9 @@ WindowStore releaseNotes: { isVisible: boolean, // Whether or not to show release notes }, - menubar: { - isVisible: boolean // windows only: true if Menubar control is visible + menubar: { // windows only + isVisible: boolean, // true if Menubar control is visible + selectedLabel: string // label of menu that is selected (or null for none selected) } }, searchDetail: { diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 0d114dc975a..a1e3274bcf6 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1010,10 +1010,12 @@ const windowActions = { /** * (Windows only) * Dispatches a message to indicate the custom rendered Menubar should be toggled (shown/hidden) + * @param {boolean} isVisible (optional) */ - toggleMenubarVisible: function () { + toggleMenubarVisible: function (isVisible) { dispatch({ - actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE + actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, + isVisible }) }, @@ -1021,15 +1023,40 @@ const windowActions = { * (Windows only) * Used to trigger the click() action for a menu * Called from the Menubar control, handled in menu.js - * @param {string} itemLabel - text of the label that was clicked + * @param {string} label - text of the label that was clicked */ - clickMenubarItem: function (label) { + clickMenubarSubmenu: function (label) { dispatch({ - actionType: WindowConstants.WINDOW_CLICK_MENUBAR_ITEM, + actionType: WindowConstants.WINDOW_CLICK_MENUBAR_SUBMENU, label }) }, + /** + * (Windows only) + * Used to track which menubar item is currently selected (or null for none selected) + * @param {string} label - text of the menubar item label that was clicked (file, edit, etc) + */ + setMenubarItemSelected: function (label) { + dispatch({ + actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED, + label + }) + }, + + /** + * Used by main.js when click happens on content area (not on a link or react control). + * - closes context menu + * - closes popup menu + * - nulls out menubar item selected (Windows only) + * - hides menubar if auto-hide preference is set (Windows only) + */ + resetMenuState: function () { + dispatch({ + actionType: WindowConstants.WINDOW_RESET_MENU_STATE + }) + }, + gotResponseDetails: function (tabId, details) { dispatch({ actionType: WindowConstants.WINDOW_GOT_RESPONSE_DETAILS, diff --git a/js/components/contextMenu.js b/js/components/contextMenu.js index 636f123ba1a..84281b17c63 100644 --- a/js/components/contextMenu.js +++ b/js/components/contextMenu.js @@ -27,11 +27,20 @@ class ContextMenuItem extends ImmutableComponent { get hasSubmenu () { return this.submenu && this.submenu.size > 0 } + get accelerator () { + const accelerator = this.props.contextMenuItem.get('accelerator') + return accelerator && typeof accelerator === 'string' + ? accelerator.trim() + : null + } + get hasAccelerator () { + return this.accelerator !== null + } onClick (clickAction, shouldHide, e) { e.stopPropagation() if (clickAction) { if (shouldHide) { - windowActions.setContextMenuDetail() + windowActions.resetMenuState() } clickAction(e) } @@ -104,6 +113,60 @@ class ContextMenuItem extends ImmutableComponent { } return '' } + + // BSCTODO: break this out to a utility class & cleanup + // see https://github.com/electron/electron/blob/master/docs/api/accelerator.md + formatAccelerator (accelerator) { + let result = accelerator + let splitResult = accelerator.split('+') + // sort in proper order, based on OS + // also, replace w/ name or symbol + if (process.platform === 'darwin') { + function orderLookup(value) { + switch(value) { + case 'Alt': + case 'Option': + case 'AltGr': + case 'Super': + return 0; + case 'Shift': + return 1; + case 'CmdOrCtrl': + case 'Control': + case 'Command': + return 2; + default: return 3; + } + splitResult.sort(function (left, right) { + if (left === right) return 0 + if (orderLookup(left) > orderLookup(right)) return 1 + return -1 + }) + result = splitResult.join('') + result = result.replace('CmdOrCtrl', '⌘') + result = result.replace('Alt', '⌥') + result = result.replace('Option', '⌥') + result = result.replace('Super', '⌘') + } + } else { + function orderLookup(value) { + switch(value) { + case 'CmdOrCtrl': return 0; + case 'Alt': return 1; + case 'Shift': return 2; + default: return 3; + } + } + splitResult.sort(function (left, right) { + if (left === right) return 0 + if (orderLookup(left) > orderLookup(right)) return 1 + return -1 + }) + result = splitResult.join('+') + result = result.replace('CmdOrCtrl', 'Ctrl') + } + return result + } render () { const iconSize = 16 let iconStyle = { @@ -180,7 +243,12 @@ class ContextMenuItem extends ImmutableComponent { - : null + : this.hasAccelerator + ? + + {this.formatAccelerator(this.accelerator)} + + : null }
} @@ -216,7 +284,7 @@ class ContextMenuSingle extends ImmutableComponent { */ class ContextMenu extends ImmutableComponent { onClick () { - windowActions.setContextMenuDetail() + windowActions.resetMenuState() } get openedSubmenuDetails () { return this.props.contextMenuDetail.get('openedSubmenuDetails') || new Immutable.List() diff --git a/js/components/main.js b/js/components/main.js index e57113d4c72..e4f4cdcfe12 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -597,24 +597,26 @@ class Main extends ImmutableComponent { } onMouseDown (e) { + // BSCTODO: update this to use eventUtil.eventElHasAncestorWithClasses let node = e.target while (node) { if (node.classList && (node.classList.contains('popupWindow') || node.classList.contains('contextMenu') || - node.classList.contains('extensionButton'))) { + node.classList.contains('extensionButton') || + node.classList.contains('menubarItem'))) { // Middle click (on context menu) needs to fire the click event. // We need to prevent the default "Auto-Scrolling" behavior. if (node.classList.contains('contextMenu') && e.button === 1) { e.preventDefault() } + // Click event is handled downstream return } node = node.parentNode } - // TODO(bridiver) combine context menu and popup window - windowActions.setContextMenuDetail() - windowActions.setPopupWindowDetail() + // Hide context menus, popup menus, and menu selections + windowActions.resetMenuState() } onClickWindow (e) { @@ -751,6 +753,7 @@ class Main extends ImmutableComponent { const menubarEnabled = isWindows const menubarVisible = menubarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null + const menubarSelectedLabel = this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']) const shouldAllowWindowDrag = !this.props.windowState.get('contextMenuDetail') && !this.props.windowState.get('bookmarkDetail') && @@ -790,7 +793,7 @@ class Main extends ImmutableComponent { { menubarVisible ?
- +
: null } @@ -838,6 +841,9 @@ class Main extends ImmutableComponent { noScriptIsVisible={noScriptIsVisible} />
+ { + this.extensionButtons + }
{ - siteInfoIsVisible - ? - : null - } - { - braveryPanelIsVisible - ? - : null - } - { - clearBrowsingDataPanelIsVisible - ? - : null - } - { - autofillAddressPanelIsVisible - ? - : null - } - { - autofillCreditCardPanelIsVisible - ? - : null - } - { - loginRequiredDetail - ? - : null - } - { - this.props.windowState.get('bookmarkDetail') - ? - : null - } - { - noScriptIsVisible - ? - : null - } - { - releaseNotesIsVisible - ? - : null - } -
- { - this.extensionButtons - } -
- { - menubarEnabled && !menubarVisible + menubarEnabled ? : null }
+ { + siteInfoIsVisible + ? + : null + } + { + braveryPanelIsVisible + ? + : null + } + { + clearBrowsingDataPanelIsVisible + ? + : null + } + { + autofillAddressPanelIsVisible + ? + : null + } + { + autofillCreditCardPanelIsVisible + ? + : null + } + { + loginRequiredDetail + ? + : null + } + { + this.props.windowState.get('bookmarkDetail') + ? + : null + } + { + noScriptIsVisible + ? + : null + } + { + releaseNotesIsVisible + ? + : null + } { diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index fade210b82f..9d68c4b9f55 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -70,7 +70,9 @@ const windowConstants = { WINDOW_SET_AUTOFILL_CREDIT_CARD_DETAIL: _, WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT: _, WINDOW_TOGGLE_MENUBAR_VISIBLE: _, - WINDOW_CLICK_MENUBAR_ITEM: _, + WINDOW_CLICK_MENUBAR_SUBMENU: _, + WINDOW_SET_MENUBAR_ITEM_SELECTED: _, + WINDOW_RESET_MENU_STATE: _, WINDOW_GOT_RESPONSE_DETAILS: _ } diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 4bc5eba9df6..66589db6244 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -678,6 +678,7 @@ const handleAppAction = (action) => { break case ExtensionConstants.EXTENSION_DISABLED: appState = extensionState.extensionDisabled(appState, action) + break case AppConstants.APP_SET_MENUBAR_TEMPLATE: appState = appState.setIn(['menu', 'template'], action.menubarTemplate) break diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index e849274e35a..2b270d0de47 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -781,8 +781,30 @@ const doAction = (action) => { break case WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE: if (getSetting(settings.AUTO_HIDE_MENU)) { - const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) - windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + // Close existing context menus + doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) + // Use value if provided; if not, toggle to opposite. + if (typeof action.isVisible === 'boolean') { + windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], action.isVisible) + } else { + const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) + windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + } + } + break + case WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED: + windowState = windowState.setIn(['ui', 'menubar', 'selectedLabel'], + action.label && typeof action.label === 'string' + ? action.label + : null) + break + case WindowConstants.WINDOW_RESET_MENU_STATE: + doAction({actionType: WindowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL}) + doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED}) + if (getSetting(settings.AUTO_HIDE_MENU)) { + doAction({actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, isVisible: false}) + } else { + doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) } break diff --git a/less/navigationBar.less b/less/navigationBar.less index b8798bc7121..298c87bf5c9 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -40,7 +40,7 @@ flex: 1; box-sizing: border-box; position: relative; - overflow: auto; + overflow: visible; white-space: nowrap; } .navbarMenubarBlockContainer { @@ -405,6 +405,7 @@ -webkit-user-select: none; -webkit-app-region: drag; height: 29px; + display: inline-block; .menubar { display: inline-block; @@ -425,6 +426,11 @@ border: 1px solid #cce8ff; } } + + .selected { + background-color: #cce8ff !important; + border: 1px solid #99d1ff !important; + } } } @@ -433,8 +439,6 @@ // box on its left has different size than the one on the right. // This is achieved by the local variable `@centerOffset`. .navigatorWrapper { - clear:both; - // Since we want to keep the navigator centered, we need to calculate the // difference between the width of the left box and the width of the right box. @centerOffset: 2 * (@navbarButtonWidth + @navbarButtonSpacing) // width area on the left From 05e8b85b3721903b320f2eb706cb843873bafd7d Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 12 Sep 2016 14:44:37 -0700 Subject: [PATCH 07/17] Fixing known issues: - Small fixes after testing context menu accelerators on Mac. Also includes lint cleanup and docs generation. - Fix navigation button spacing; original commit by @Sh1d0w bumped it from 4px to 14px. Besides fixing spacing, this also fixes the stretched out when hover issue. - Windows now has a 5px margin to the left of navigator buttons - Broke out accelerator formatting to a new file and added unit tests to ensure the logic is correct (glad I did- there were missing use-cases). - Updated accelerator style in context menu to be the same size as the item itself - Removed max-width of 300 for context menu (to prevent overflow) - Updated font style/size for context menu (windows only) - Fix the resize border problem - Fix menu font face (windows) --- app/common/lib/formatUtil.js | 80 +++++++++++++++++++++++++++++++++ docs/windowActions.md | 44 ++++++++++++++++++ js/components/contextMenu.js | 61 ++----------------------- less/contextMenu.less | 19 +++++++- less/navigationBar.less | 12 +++-- less/variables.less | 4 +- test/unit/lib/formatUtilTest.js | 47 +++++++++++++++++++ 7 files changed, 203 insertions(+), 64 deletions(-) create mode 100644 app/common/lib/formatUtil.js create mode 100644 test/unit/lib/formatUtilTest.js diff --git a/app/common/lib/formatUtil.js b/app/common/lib/formatUtil.js new file mode 100644 index 00000000000..a4efa70fd7a --- /dev/null +++ b/app/common/lib/formatUtil.js @@ -0,0 +1,80 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +'use strict' + +const macOrderLookup = (value) => { + switch (value) { + case 'Alt': + case 'Option': + case 'AltGr': + return 0 + case 'Shift': + return 1 + case 'Control': + case 'Ctrl': + return 2 + case 'Super': + case 'CmdOrCtrl': + case 'Command': + case 'Cmd': + return 3 + default: + return 4 + } +} +const defaultOrderLookup = (value) => { + switch (value) { + case 'CmdOrCtrl': + case 'Control': + case 'Ctrl': + return 0 + case 'Alt': + case 'AltGr': + return 1 + case 'Shift': + return 2 + default: + return 3 + } +} + +/* + * Format an electron accelerator in the order you'd expect in a menu + * Accelerator reference: https://github.com/electron/electron/blob/master/docs/api/accelerator.md + */ +module.exports.formatAccelerator = (accelerator) => { + let result = accelerator + let splitResult = accelerator.split('+') + // sort in proper order, based on OS + // also, replace w/ name or symbol + if (process.platform === 'darwin') { + splitResult.sort(function (left, right) { + if (macOrderLookup(left) === macOrderLookup(right)) return 0 + if (macOrderLookup(left) > macOrderLookup(right)) return 1 + return -1 + }) + // NOTE: these characters might only show properly on Mac + result = splitResult.join('') + result = result.replace('CmdOrCtrl', '⌘') + result = result.replace('Command', '⌘') + result = result.replace('Cmd', '⌘') + result = result.replace('Alt', '⌥') + result = result.replace('AltGr', '⌥') + result = result.replace('Super', '⌘') + result = result.replace('Option', '⌥') + result = result.replace('Shift', '⇧') + result = result.replace('Control', '^') + result = result.replace('Ctrl', '^') + } else { + splitResult.sort(function (left, right) { + if (defaultOrderLookup(left) === defaultOrderLookup(right)) return 0 + if (defaultOrderLookup(left) > defaultOrderLookup(right)) return 1 + return -1 + }) + result = splitResult.join('+') + result = result.replace('CmdOrCtrl', 'Ctrl') + } + return result +} diff --git a/docs/windowActions.md b/docs/windowActions.md index 8031d182346..58c0d8fa67d 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -788,6 +788,50 @@ blocked active mixed content on +### toggleMenubarVisible(isVisible) + +(Windows only) +Dispatches a message to indicate the custom rendered Menubar should be toggled (shown/hidden) + +**Parameters** + +**isVisible**: `boolean`, (optional) + + + +### clickMenubarSubmenu(label) + +(Windows only) +Used to trigger the click() action for a menu +Called from the Menubar control, handled in menu.js + +**Parameters** + +**label**: `string`, text of the label that was clicked + + + +### setMenubarItemSelected(label) + +(Windows only) +Used to track which menubar item is currently selected (or null for none selected) + +**Parameters** + +**label**: `string`, text of the menubar item label that was clicked (file, edit, etc) + + + +### resetMenuState() + +Used by main.js when click happens on content area (not on a link or react control). +- closes context menu +- closes popup menu +- nulls out menubar item selected (Windows only) +- hides menubar if auto-hide preference is set (Windows only) + + + * * * diff --git a/js/components/contextMenu.js b/js/components/contextMenu.js index 84281b17c63..c9ed1f73d5c 100644 --- a/js/components/contextMenu.js +++ b/js/components/contextMenu.js @@ -8,6 +8,7 @@ const ImmutableComponent = require('./immutableComponent') const windowActions = require('../actions/windowActions') const cx = require('../lib/classSet.js') const KeyCodes = require('../constants/keyCodes') +const { formatAccelerator } = require('../../app/common/lib/formatUtil') class ContextMenuItem extends ImmutableComponent { componentDidMount () { @@ -113,60 +114,6 @@ class ContextMenuItem extends ImmutableComponent { } return '' } - - // BSCTODO: break this out to a utility class & cleanup - // see https://github.com/electron/electron/blob/master/docs/api/accelerator.md - formatAccelerator (accelerator) { - let result = accelerator - let splitResult = accelerator.split('+') - // sort in proper order, based on OS - // also, replace w/ name or symbol - if (process.platform === 'darwin') { - function orderLookup(value) { - switch(value) { - case 'Alt': - case 'Option': - case 'AltGr': - case 'Super': - return 0; - case 'Shift': - return 1; - case 'CmdOrCtrl': - case 'Control': - case 'Command': - return 2; - default: return 3; - } - splitResult.sort(function (left, right) { - if (left === right) return 0 - if (orderLookup(left) > orderLookup(right)) return 1 - return -1 - }) - result = splitResult.join('') - result = result.replace('CmdOrCtrl', '⌘') - result = result.replace('Alt', '⌥') - result = result.replace('Option', '⌥') - result = result.replace('Super', '⌘') - } - } else { - function orderLookup(value) { - switch(value) { - case 'CmdOrCtrl': return 0; - case 'Alt': return 1; - case 'Shift': return 2; - default: return 3; - } - } - splitResult.sort(function (left, right) { - if (left === right) return 0 - if (orderLookup(left) > orderLookup(right)) return 1 - return -1 - }) - result = splitResult.join('+') - result = result.replace('CmdOrCtrl', 'Ctrl') - } - return result - } render () { const iconSize = 16 let iconStyle = { @@ -245,9 +192,9 @@ class ContextMenuItem extends ImmutableComponent { : this.hasAccelerator ? - - {this.formatAccelerator(this.accelerator)} - + + {formatAccelerator(this.accelerator)} + : null }
diff --git a/less/contextMenu.less b/less/contextMenu.less index 1b2549d5e37..2e7f47c5407 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -10,7 +10,7 @@ color: black; cursor: default; display: flex; - font-size: 14px; + font-size: @contextMenuFontSize; &.contextMenuScrollable { overflow-y: scroll; } @@ -68,7 +68,6 @@ } } box-sizing: border-box; - max-width: 300px; display: flex; align-items: center; text-overflow: ellipsis; @@ -139,9 +138,25 @@ float: right; margin: auto 0 auto 5px; } + .accelerator { + .submenuIndicator; + font-size: @contextMenuFontSize; + margin-left: 10px; + } } .fa { font-size: @bookmarksFolderIconSize; } } + + +// Make context menu style match menubar (Windows only- for use w/ slim titlebar) +.platform--win32 .contextMenuItem { + font: menu; + font-size: 12px; +} + +.platform--win32 .accelerator { + font: menu; +} diff --git a/less/navigationBar.less b/less/navigationBar.less index 298c87bf5c9..abc033ef4f6 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -29,6 +29,11 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } +// On Windows, we add extra padding to have spacing match the close button +.platform--win32 .navigatorWrapper { + margin-left: @navbarLeftMarginWindows; +} + // Style adjustments needed for slim titlebar to work properly .navbarCaptionButtonContainer { display: flex; @@ -51,7 +56,6 @@ // Window Caption Buttons (for use w/ slim titlebar) .windowCaptionButtons { display: inline-block; - -webkit-user-select: none; -webkit-app-region: drag; height: 100%; @@ -402,16 +406,16 @@ // Menubar (for use w/ slim titlebar) .menubarContainer { - -webkit-user-select: none; -webkit-app-region: drag; + width: 100%; height: 29px; display: inline-block; + margin-top: 5px; + margin-left: 4px; .menubar { display: inline-block; cursor: default; - margin-top: 5px; - margin-left: 4px; .menubarItem { color: black; diff --git a/less/variables.less b/less/variables.less index 1ac51bf2686..95462fe659e 100644 --- a/less/variables.less +++ b/less/variables.less @@ -35,6 +35,7 @@ @defaultSpacing: 12px; @progressBarColor: #3498DB; @defaultFontSize: 13px; +@contextMenuFontSize: 14px; @audioColor: @highlightBlue; @focusUrlbarOutline: @highlightBlue; @loadTimeColor: @highlightBlue; @@ -69,11 +70,12 @@ @bookmarksFileIconSize: 13px; @bookmarksFolderIconSize: 15px; -@navbarButtonSpacing: 14px; +@navbarButtonSpacing: 4px; @navbarButtonWidth: 35px; @navbarBraveButtonWidth: 23px; @navbarBraveButtonMarginLeft: 80px; @navbarLeftMarginDarwin: 70px; +@navbarLeftMarginWindows: 5px; @findbarBackground: #F7F7F7; diff --git a/test/unit/lib/formatUtilTest.js b/test/unit/lib/formatUtilTest.js new file mode 100644 index 00000000000..bb6765ca2c4 --- /dev/null +++ b/test/unit/lib/formatUtilTest.js @@ -0,0 +1,47 @@ +/* global describe, before, after, it */ +const formatUtil = require('../../../app/common/lib/formatUtil') +const assert = require('assert') + +require('../braveUnit') + +describe('formatUtil', function () { + describe('Windows and Linux', function () { + before(function () { + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { + value: 'win32' + }); + }) + + after(function () { + Object.defineProperty(process, 'platform', this.originalPlatform); + }) + + it('puts the modifiers in the correct order', function () { + const result = formatUtil.formatAccelerator('A+Shift+Alt+CmdOrCtrl') + assert.equal(result, 'Ctrl+Alt+Shift+A') + }) + it('leaves modifiers alone if order is correct', function () { + const result = formatUtil.formatAccelerator('Ctrl+Shift+O') + assert.equal(result, 'Ctrl+Shift+O') + }) + }) + + describe('Mac', function () { + before(function () { + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + Object.defineProperty(process, 'platform', { + value: 'darwin' + }); + }) + + after(function () { + Object.defineProperty(process, 'platform', this.originalPlatform); + }) + + it('replaces the key names with the correct symbols', function () { + const result = formatUtil.formatAccelerator('Alt+CmdOrCtrl+Ctrl+Shift+O') + assert.equal(result, '⌥⇧^⌘O') + }) + }) +}) From 05b8a2124179d822aabb7a931b4387dcefe6aec1 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 13 Sep 2016 16:34:47 -0700 Subject: [PATCH 08/17] Fixing known issues: - Added border around window (only affects Windows when frameless). - Fix font size on context menus (for accelerators) --- js/components/main.js | 3 ++- less/contextMenu.less | 1 + less/navigationBar.less | 5 ++++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/js/components/main.js b/js/components/main.js index e4f4cdcfe12..1842973993c 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -768,7 +768,8 @@ class Main extends ImmutableComponent { return
{ this.mainWindow = node }} onMouseDown={this.onMouseDown} diff --git a/less/contextMenu.less b/less/contextMenu.less index 2e7f47c5407..a13afa21e82 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -159,4 +159,5 @@ .platform--win32 .accelerator { font: menu; + font-size: 12px; } diff --git a/less/navigationBar.less b/less/navigationBar.less index abc033ef4f6..baf0089a56f 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -35,11 +35,14 @@ } // Style adjustments needed for slim titlebar to work properly +.platform--win32 div#window.frameless { + border: 1px solid #000; + box-sizing: border-box; +} .navbarCaptionButtonContainer { display: flex; border-bottom: 1px solid #aaaaaa; } - .navbarMenubarFlexContainer { display: flex; flex: 1; From 2e0c1ec212ed53f6b747b5d20d6fa3895d0038df Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Wed, 14 Sep 2016 17:49:13 -0700 Subject: [PATCH 09/17] Fixing known issues: - Removed border on top since that now works; starting to finalize the X with Brad - Windows 7 fixes - fix button size - new svg for close - fix margin - color updates - updated naming in main.js; menubar text no longer selectable --- img/windows/close.svg | 1 + js/components/main.js | 6 +-- less/navigationBar.less | 84 ++++++++++++----------------------------- 3 files changed, 29 insertions(+), 62 deletions(-) create mode 100644 img/windows/close.svg diff --git a/img/windows/close.svg b/img/windows/close.svg new file mode 100644 index 00000000000..fcca53e1a5f --- /dev/null +++ b/img/windows/close.svg @@ -0,0 +1 @@ +close_icon \ No newline at end of file diff --git a/js/components/main.js b/js/components/main.js index 1842973993c..977bcfb5558 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -750,8 +750,8 @@ class Main extends ImmutableComponent { const braverySettings = siteSettings.activeSettings(activeSiteSettings, this.props.appState, appConfig) const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null - const menubarEnabled = isWindows - const menubarVisible = menubarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) + const customTitlebarEnabled = isWindows + const menubarVisible = customTitlebarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null const menubarSelectedLabel = this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']) @@ -858,7 +858,7 @@ class Main extends ImmutableComponent {
{ - menubarEnabled + customTitlebarEnabled ? : null } diff --git a/less/navigationBar.less b/less/navigationBar.less index baf0089a56f..ca4ed1d2ed4 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -60,7 +60,6 @@ .windowCaptionButtons { display: inline-block; -webkit-app-region: drag; - height: 100%; .hidden { display: none; @@ -82,21 +81,21 @@ } .win7 { - margin-right: 5px; + margin-right: 6px; button.captionButton { outline: 0; height: 18px; color: #b1acac; - background-size: 12px 12px!important; - background-position: center!important; border: 1px solid #838383; + border-top: 0; display: inline-block; - background-color: #dfdfdf; + background-color: #e0e0e0; box-shadow: inset 1px 1px rgba(255, 255, 255, 0.9); width: 25px; &.minimize { + width: 28px; border-right: 0px; &:hover { background-color: #f5f5f5; @@ -110,7 +109,7 @@ width: 10px; height: 3px; border: 1px solid #838383; - background: #f0f0f0; + background: #fefefe; display: inline-block; border-radius: 1px; } @@ -118,6 +117,7 @@ &.maximize { border-right: 0px; + width: 27px; &:hover { background-color: #f5f5f5; .widget { @@ -145,14 +145,14 @@ .widget1 { width: 8px; top: 2px; - left: 8px; + left: 10px; } .widget2 { width: 8px; height: 8px; top: -5px; - left: 4px; - background: #f0f0f0; + left: 6px; + background: #fefefe; border-radius: 1px; } .widget3 { @@ -160,10 +160,10 @@ width: 2px; height: 2px; border: 1px solid #838383; - background: #f0f0f0; + background: #fefefe; position: relative; top: -20px; - left: -3px; + left: -2px; } } } @@ -173,20 +173,20 @@ width: 10px; height: 8px; border: 1px solid #838383; - background: #f0f0f0; + background: #fefefe; position: relative; top: 2px; - left: 6px; + left: 7px; border-radius: 1px; } .widget2 { width: 4px; height: 2px; border: 1px solid #838383; - background-color: #dfdfdf; + background-color: #e0e0e0; position: relative; top: -5px; - left: 9px; + left: 10px; border-radius: 0; } .widget3 { display: none; } @@ -196,61 +196,26 @@ } &.close { - width: 43px; + width: 48px; border-radius: 0 0 4px 0; &:hover { background-color: #d9504e; - .widget { - .widget1 { border: 1px solid black; } - .widget2 { border: 1px solid black; } - } } &:active { background-color: #d7393d; - .widget { - .widget1 { border: 1px solid black; } - .widget2 { border: 1px solid black; } - } } .widget { - width: 11px; - height: 11px; + background: url('../img/windows/close.svg') no-repeat; display: inline-block; - vertical-align: middle; + height: 12px; + width: 12px; + position: relative; + top: 3px; - .widget1 { - width: 10px; - height: 2px; - background: #f0f0f0; - display: inline-block; - transform: rotate(45deg); - position: relative; - top: -6px; - left: -2px; - border: 1px solid #838383; - } - .widget2 { - width: 10px; - height: 2px; - background: #f0f0f0; - display: inline-block; - transform: rotate(315deg); - position: relative; - top: -21px; - left: -2px; - border: 1px solid #838383; - } - .widget3 { - width: 10px; - height: 2px; - background: #f0f0f0; - display: inline-block; - transform: rotate(45deg); - position: relative; - top: -37px; - left: -2px; - } + .widget1 { display:none; } + .widget2 { display:none; } + .widget3 { display:none; } } } } @@ -419,6 +384,7 @@ .menubar { display: inline-block; cursor: default; + -webkit-user-select: none; .menubarItem { color: black; From c30a607b0b92d5417a10232bc1682bc2277681f9 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Thu, 15 Sep 2016 23:40:18 -0700 Subject: [PATCH 10/17] Fixing known issues: - Caption buttons and border are now hidden when a window goes fullscreen - Fix context menu accelerator size (windows only) --- js/components/main.js | 5 +++-- less/contextMenu.less | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/js/components/main.js b/js/components/main.js index 977bcfb5558..c6aa0b8bf99 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -751,6 +751,7 @@ class Main extends ImmutableComponent { const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null const customTitlebarEnabled = isWindows + const captionButtonsVisible = customTitlebarEnabled && !this.props.windowState.getIn(['ui', 'isFullScreen']) const menubarVisible = customTitlebarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null const menubarSelectedLabel = this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']) @@ -769,7 +770,7 @@ class Main extends ImmutableComponent { return
{ this.mainWindow = node }} onMouseDown={this.onMouseDown} @@ -858,7 +859,7 @@ class Main extends ImmutableComponent {
{ - customTitlebarEnabled + captionButtonsVisible ? : null } diff --git a/less/contextMenu.less b/less/contextMenu.less index a13afa21e82..611c51cf027 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -159,5 +159,5 @@ .platform--win32 .accelerator { font: menu; - font-size: 12px; + font-size: 12px !important; } From beeab7e4c52470efb603b99c99e39617a6b26d3e Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 16 Sep 2016 15:58:20 -0700 Subject: [PATCH 11/17] rev1 of keyboard shortcuts for menus - selection can be made left and right - menu can be popped up with up/down known issues (what's left) - storing the context menu's selected index in windowStore - properly handling up/down (changing selection) - handling submenus also includes: - small cleanup for custom titlebar in main.js - lint cleanup --- app/common/lib/formatUtil.js | 2 +- app/renderer/components/menubar.js | 113 +++++++++++++++++++++++++---- js/components/main.js | 44 +++++++---- js/stores/windowStore.js | 12 +-- test/unit/lib/formatUtilTest.js | 12 +-- 5 files changed, 145 insertions(+), 38 deletions(-) diff --git a/app/common/lib/formatUtil.js b/app/common/lib/formatUtil.js index a4efa70fd7a..c1ad832f411 100644 --- a/app/common/lib/formatUtil.js +++ b/app/common/lib/formatUtil.js @@ -44,7 +44,7 @@ const defaultOrderLookup = (value) => { * Format an electron accelerator in the order you'd expect in a menu * Accelerator reference: https://github.com/electron/electron/blob/master/docs/api/accelerator.md */ -module.exports.formatAccelerator = (accelerator) => { +module.exports.formatAccelerator = (accelerator) => { let result = accelerator let splitResult = accelerator.split('+') // sort in proper order, based on OS diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index 783697fa739..313ea3cf5c2 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -7,6 +7,23 @@ const Immutable = require('immutable') const ImmutableComponent = require('../../../js/components/immutableComponent') const windowActions = require('../../../js/actions/windowActions') const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem +const keyCodes = require('../../../js/constants/keyCodes') + +const showContextMenu = (rect, submenu) => { + windowActions.setContextMenuDetail(Immutable.fromJS({ + left: rect.left, + top: rect.bottom, + template: submenu.map((submenuItem) => { + if (submenuItem.type === separatorMenuItem.type) { + return submenuItem + } + submenuItem.click = function (e) { + windowActions.clickMenubarSubmenu(submenuItem.label) + } + return submenuItem + }) + })) +} class MenubarItem extends ImmutableComponent { constructor () { @@ -29,19 +46,7 @@ class MenubarItem extends ImmutableComponent { // Otherwise, mark item as selected and show its context menu windowActions.setMenubarItemSelected(this.props.label) const rect = e.target.getBoundingClientRect() - windowActions.setContextMenuDetail(Immutable.fromJS({ - left: rect.left, - top: rect.bottom, - template: this.props.submenu.map((submenuItem) => { - if (submenuItem.type === separatorMenuItem.type) { - return submenuItem - } - submenuItem.click = function (e) { - windowActions.clickMenubarSubmenu(submenuItem.label) - } - return submenuItem - }) - })) + showContextMenu(rect, this.props.submenu) } onMouseOver (e) { @@ -61,12 +66,94 @@ class MenubarItem extends ImmutableComponent { } } +// BSCTODO: where to put this? +const wrappingClamp = (value, min, max) => { + var range = (max - min) + 1 + return value - Math.floor((value - min) / range) * range +} + /** * Menubar that can be optionally be displayed at the top of a window (in favor of the system menu). * First intended use is with Windows to enable a slim titlebar. * NOTE: the system menu is still created and used in order to keep the accelerators working. */ class Menubar extends ImmutableComponent { + constructor () { + super() + this.onKeyDown = this.onKeyDown.bind(this) + document.addEventListener('keydown', this.onKeyDown) + } + + get selectedTemplate () { + const element = this.props.template.find((element) => { + return element.get('label') === this.props.selectedLabel + }) + return element ? element.get('submenu') : null + } + + get selectedRect () { + const selected = document.querySelectorAll('.menubar .selected') + if (selected.length === 1) { + return selected.item(0).getBoundingClientRect() + } + return null + } + + onKeyDown (e) { + // BSCTODO: how to handle if menu is ALWAYS showing? + // we can't just eat (preventDefault) the arrow keys :( + switch (e.which) { + case keyCodes.LEFT: + case keyCodes.RIGHT: + e.preventDefault() + + if (this.props.template.size > 0) { + const selectedIndex = this.props.template.findIndex((element) => { + return element.get('label') === this.props.selectedLabel + }) + const nextIndex = selectedIndex === -1 + ? 0 + : wrappingClamp( + selectedIndex + (e.which === keyCodes.LEFT ? -1 : 1), + 0, + this.props.template.size - 1) + + // BSCTODO: consider submenus + windowActions.setMenubarItemSelected(this.props.template.getIn([nextIndex, 'label'])) + + // Context menu already being displayed; auto-open the next one + if (this.props.contextMenuDetail && this.selectedTemplate && this.selectedRect) { + // BSCTODO: if already selected and in a menu, set the Y index to 0 + // BSCTODO: this logic should happen *after* render + showContextMenu(this.selectedRect, this.selectedTemplate) + } + } + break + case keyCodes.UP: + case keyCodes.DOWN: + if (this.props.selectedLabel) { + if (!this.selectedTemplate) break + + if (!this.props.contextMenuDetail && this.selectedRect) { + // First time hitting up/down; popup the context menu + showContextMenu(this.selectedRect, this.selectedTemplate) + } else { + // Context menu already visible; move selection up or down + // BSCTODO: need a new windowState to track active context menu selected index + } + } + break + } + } + + // BSCTODO: keyup handler needed: + // - for ESC, to set selection to null (context menu already handles ESC properly) + // - for ALT, unset selection and hide menu + + componentWillUnmount () { + document.removeEventListener('keydown', this.onKeyDown) + } + shouldComponentUpdate (nextProps, nextState) { return this.props.selectedLabel !== nextProps.selectedLabel } diff --git a/js/components/main.js b/js/components/main.js index c6aa0b8bf99..5be5e8410f6 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -119,8 +119,15 @@ class Main extends ImmutableComponent { ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_ZOOM_OUT) } break + } + }) + document.addEventListener('keyup', (e) => { + const customTitlebar = this.customTitlebar + switch (e.which) { case keyCodes.ALT: - if (isWindows) { + if (customTitlebar.enabled) { + // BSCTODO: if it's always visible, set a selected item instead + e.preventDefault() windowActions.toggleMenubarVisible() } break @@ -720,6 +727,20 @@ class Main extends ImmutableComponent { return buttons } + get customTitlebar () { + const customTitlebarEnabled = isWindows + const captionButtonsVisible = customTitlebarEnabled && !this.props.windowState.getIn(['ui', 'isFullScreen']) + const menubarVisible = customTitlebarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) + return { + enabled: customTitlebarEnabled, + captionButtonsVisible: captionButtonsVisible, + menubarVisible: menubarVisible, + menubarTemplate: menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null, + menubarSelectedLabel: this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']), + isMaximized: this.props.windowState.getIn(['ui', 'isMaximized']) + } + } + render () { const comparatorByKeyAsc = (a, b) => a.get('key') > b.get('key') ? 1 : b.get('key') > a.get('key') ? -1 : 0 @@ -749,13 +770,7 @@ class Main extends ImmutableComponent { const releaseNotesIsVisible = this.props.windowState.getIn(['ui', 'releaseNotes', 'isVisible']) const braverySettings = siteSettings.activeSettings(activeSiteSettings, this.props.appState, appConfig) const loginRequiredDetail = activeFrame ? basicAuthState.getLoginRequiredDetail(this.props.appState, activeFrame.get('tabId')) : null - - const customTitlebarEnabled = isWindows - const captionButtonsVisible = customTitlebarEnabled && !this.props.windowState.getIn(['ui', 'isFullScreen']) - const menubarVisible = customTitlebarEnabled && (!getSetting(settings.AUTO_HIDE_MENU) || this.props.windowState.getIn(['ui', 'menubar', 'isVisible'])) - const menubarTemplate = menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null - const menubarSelectedLabel = this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']) - + const customTitlebar = this.customTitlebar const shouldAllowWindowDrag = !this.props.windowState.get('contextMenuDetail') && !this.props.windowState.get('bookmarkDetail') && !siteInfoIsVisible && @@ -770,7 +785,7 @@ class Main extends ImmutableComponent { return
{ this.mainWindow = node }} onMouseDown={this.onMouseDown} @@ -793,9 +808,12 @@ class Main extends ImmutableComponent {
{ - menubarVisible + customTitlebar.menubarVisible ?
- +
: null } @@ -859,8 +877,8 @@ class Main extends ImmutableComponent {
{ - captionButtonsVisible - ? + customTitlebar.captionButtonsVisible + ? : null }
diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 2b270d0de47..6f7880255ac 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -784,12 +784,14 @@ const doAction = (action) => { // Close existing context menus doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) // Use value if provided; if not, toggle to opposite. - if (typeof action.isVisible === 'boolean') { - windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], action.isVisible) - } else { - const currentStatus = windowState.getIn(['ui', 'menubar', 'isVisible']) - windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], !currentStatus) + const newVisibleStatus = typeof action.isVisible === 'boolean' + ? action.isVisible + : !windowState.getIn(['ui', 'menubar', 'isVisible']) + // Clear selection when menu is shown + if (newVisibleStatus) { + doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED}) } + windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], newVisibleStatus) } break case WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED: diff --git a/test/unit/lib/formatUtilTest.js b/test/unit/lib/formatUtilTest.js index bb6765ca2c4..7ab00c6b29c 100644 --- a/test/unit/lib/formatUtilTest.js +++ b/test/unit/lib/formatUtilTest.js @@ -7,14 +7,14 @@ require('../braveUnit') describe('formatUtil', function () { describe('Windows and Linux', function () { before(function () { - this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') Object.defineProperty(process, 'platform', { value: 'win32' - }); + }) }) after(function () { - Object.defineProperty(process, 'platform', this.originalPlatform); + Object.defineProperty(process, 'platform', this.originalPlatform) }) it('puts the modifiers in the correct order', function () { @@ -29,14 +29,14 @@ describe('formatUtil', function () { describe('Mac', function () { before(function () { - this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform'); + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') Object.defineProperty(process, 'platform', { value: 'darwin' - }); + }) }) after(function () { - Object.defineProperty(process, 'platform', this.originalPlatform); + Object.defineProperty(process, 'platform', this.originalPlatform) }) it('replaces the key names with the correct symbols', function () { From 39fe326133339df0fafa38c15f087c1777617bad Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sat, 17 Sep 2016 15:56:06 -0700 Subject: [PATCH 12/17] Rev2 on the keyboard controlled menus Menu fixes - You can now hit enter to pick a selected menu item - Fixed bug where menu opened by keyboard was not clickable Cleanup - Added reducer property which retuns a menu minus the separator items - Moved wrappingClamp method into formatUtils --- app/common/lib/formatUtil.js | 13 +++- app/renderer/components/menubar.js | 105 ++++++++++++++++++----------- docs/state.md | 3 +- js/actions/windowActions.js | 11 ++- js/components/contextMenu.js | 33 ++++++--- js/components/main.js | 5 +- js/constants/windowConstants.js | 3 +- js/stores/windowStore.js | 14 +++- less/contextMenu.less | 5 ++ test/unit/lib/formatUtilTest.js | 70 +++++++++++-------- 10 files changed, 178 insertions(+), 84 deletions(-) diff --git a/app/common/lib/formatUtil.js b/app/common/lib/formatUtil.js index c1ad832f411..869217e623e 100644 --- a/app/common/lib/formatUtil.js +++ b/app/common/lib/formatUtil.js @@ -40,7 +40,7 @@ const defaultOrderLookup = (value) => { } } -/* +/** * Format an electron accelerator in the order you'd expect in a menu * Accelerator reference: https://github.com/electron/electron/blob/master/docs/api/accelerator.md */ @@ -78,3 +78,14 @@ module.exports.formatAccelerator = (accelerator) => { } return result } + +/** + * Clamp values down to a given range (min/max). + * Value is wrapped when out of bounds. ex: + * min-1 = max + * max+1 = min + */ +module.exports.wrappingClamp = (value, min, max) => { + const range = (max - min) + 1 + return value - Math.floor((value - min) / range) * range +} diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index 313ea3cf5c2..d92895d5c10 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -8,6 +8,7 @@ const ImmutableComponent = require('../../../js/components/immutableComponent') const windowActions = require('../../../js/actions/windowActions') const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem const keyCodes = require('../../../js/constants/keyCodes') +const { wrappingClamp } = require('../../common/lib/formatUtil') const showContextMenu = (rect, submenu) => { windowActions.setContextMenuDetail(Immutable.fromJS({ @@ -31,7 +32,6 @@ class MenubarItem extends ImmutableComponent { this.onClick = this.onClick.bind(this) this.onMouseOver = this.onMouseOver.bind(this) } - onClick (e) { if (e && e.stopPropagation) { e.stopPropagation() @@ -40,38 +40,31 @@ class MenubarItem extends ImmutableComponent { const selected = this.props.menubar.props.selectedLabel if (selected && selected === this.props.label) { windowActions.setContextMenuDetail() - windowActions.setMenubarItemSelected() + windowActions.setMenubarSelectedLabel() return } // Otherwise, mark item as selected and show its context menu - windowActions.setMenubarItemSelected(this.props.label) + windowActions.setMenubarSelectedLabel(this.props.label) const rect = e.target.getBoundingClientRect() showContextMenu(rect, this.props.submenu) } - onMouseOver (e) { const selected = this.props.menubar.props.selectedLabel if (selected && selected !== this.props.label) { this.onClick(e) } } - render () { return + onMouseOver={this.onMouseOver} + data-label={this.props.label}> { this.props.label } } } -// BSCTODO: where to put this? -const wrappingClamp = (value, min, max) => { - var range = (max - min) + 1 - return value - Math.floor((value - min) / range) * range -} - /** * Menubar that can be optionally be displayed at the top of a window (in favor of the system menu). * First intended use is with Windows to enable a slim titlebar. @@ -83,30 +76,65 @@ class Menubar extends ImmutableComponent { this.onKeyDown = this.onKeyDown.bind(this) document.addEventListener('keydown', this.onKeyDown) } - - get selectedTemplate () { + getTemplateByLabel (label) { const element = this.props.template.find((element) => { - return element.get('label') === this.props.selectedLabel + return element.get('label') === label }) return element ? element.get('submenu') : null } - - get selectedRect () { - const selected = document.querySelectorAll('.menubar .selected') + get selectedTemplate () { + return this.getTemplateByLabel(this.props.selectedLabel) + } + get selectedTemplateItemsOnly () { + // this exclude the separators + return this.selectedTemplate.reduce((previousValue, currentValue, currentIndex, array) => { + const result = currentIndex === 1 ? [] : previousValue + if (currentIndex === 1 && previousValue.get('type') !== separatorMenuItem.type) { + result.push(previousValue) + } + if (currentValue.get('type') !== separatorMenuItem.type) { + result.push(currentValue) + } + return result + }) + } + get selectedIndexMax () { + const result = this.selectedTemplateItemsOnly + if (result && Array.isArray(result)) { + return result.length; + } + return 0 + } + getRectByLabel (label) { + const selected = document.querySelectorAll('.menubar .menubarItem[data-label=\'' + label + '\']') if (selected.length === 1) { return selected.item(0).getBoundingClientRect() } return null } - + get selectedRect () { + return this.getRectByLabel(this.props.selectedLabel) + } onKeyDown (e) { // BSCTODO: how to handle if menu is ALWAYS showing? // we can't just eat (preventDefault) the arrow keys :( + // BSCTODO: keyup handler needed: + // - for ESC, to set selection to null (context menu already handles ESC properly) + // - for ALT, unset selection and hide menu + // switch (e.which) { + case keyCodes.ENTER: + e.preventDefault() + if (this.selectedTemplate) { + const selectedLabel = this.selectedTemplateItemsOnly[this.props.selectedIndex].get('label') + windowActions.clickMenubarSubmenu(selectedLabel) + windowActions.resetMenuState() + } + break + case keyCodes.LEFT: case keyCodes.RIGHT: e.preventDefault() - if (this.props.template.size > 0) { const selectedIndex = this.props.template.findIndex((element) => { return element.get('label') === this.props.selectedLabel @@ -118,46 +146,47 @@ class Menubar extends ImmutableComponent { 0, this.props.template.size - 1) - // BSCTODO: consider submenus - windowActions.setMenubarItemSelected(this.props.template.getIn([nextIndex, 'label'])) + // BSCTODO: consider submenus (ex: for bookmark folders) + + const nextLabel = this.props.template.getIn([nextIndex, 'label']) + const nextRect = this.getRectByLabel(nextLabel) + + windowActions.setMenubarSelectedLabel(nextLabel) // Context menu already being displayed; auto-open the next one - if (this.props.contextMenuDetail && this.selectedTemplate && this.selectedRect) { - // BSCTODO: if already selected and in a menu, set the Y index to 0 - // BSCTODO: this logic should happen *after* render - showContextMenu(this.selectedRect, this.selectedTemplate) + if (this.props.contextMenuDetail && this.selectedTemplate && nextRect) { + windowActions.setSubmenuSelectedIndex(0) + showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS()) } } break + case keyCodes.UP: case keyCodes.DOWN: - if (this.props.selectedLabel) { - if (!this.selectedTemplate) break - + e.preventDefault() + if (this.props.selectedLabel && this.selectedTemplate) { if (!this.props.contextMenuDetail && this.selectedRect) { // First time hitting up/down; popup the context menu - showContextMenu(this.selectedRect, this.selectedTemplate) + windowActions.setSubmenuSelectedIndex(0) + showContextMenu(this.selectedRect, this.selectedTemplate.toJS()) } else { // Context menu already visible; move selection up or down - // BSCTODO: need a new windowState to track active context menu selected index + const nextIndex = wrappingClamp( + this.props.selectedIndex + (e.which === keyCodes.UP ? -1 : 1), + 0, + this.selectedIndexMax - 1) + windowActions.setSubmenuSelectedIndex(nextIndex) } } break } } - - // BSCTODO: keyup handler needed: - // - for ESC, to set selection to null (context menu already handles ESC properly) - // - for ALT, unset selection and hide menu - componentWillUnmount () { document.removeEventListener('keydown', this.onKeyDown) } - shouldComponentUpdate (nextProps, nextState) { return this.props.selectedLabel !== nextProps.selectedLabel } - render () { return
{ diff --git a/docs/state.md b/docs/state.md index 39b88d15697..e8717d3ce25 100644 --- a/docs/state.md +++ b/docs/state.md @@ -373,7 +373,8 @@ WindowStore }, menubar: { // windows only isVisible: boolean, // true if Menubar control is visible - selectedLabel: string // label of menu that is selected (or null for none selected) + selectedLabel: string, // label of menu that is selected (or null for none selected) + selectedIndex: number // index of the selected context menu item } }, searchDetail: { diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index a1e3274bcf6..74ca229f8e9 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1037,9 +1037,9 @@ const windowActions = { * Used to track which menubar item is currently selected (or null for none selected) * @param {string} label - text of the menubar item label that was clicked (file, edit, etc) */ - setMenubarItemSelected: function (label) { + setMenubarSelectedLabel: function (label) { dispatch({ - actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED, + actionType: WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL, label }) }, @@ -1057,6 +1057,13 @@ const windowActions = { }) }, + setSubmenuSelectedIndex: function (index) { + dispatch({ + actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX, + index + }) + }, + gotResponseDetails: function (tabId, details) { dispatch({ actionType: WindowConstants.WINDOW_GOT_RESPONSE_DETAILS, diff --git a/js/components/contextMenu.js b/js/components/contextMenu.js index c9ed1f73d5c..28880375059 100644 --- a/js/components/contextMenu.js +++ b/js/components/contextMenu.js @@ -9,6 +9,7 @@ const windowActions = require('../actions/windowActions') const cx = require('../lib/classSet.js') const KeyCodes = require('../constants/keyCodes') const { formatAccelerator } = require('../../app/common/lib/formatUtil') +const separatorMenuItem = require('../../app/common/commonMenu').separatorMenuItem class ContextMenuItem extends ImmutableComponent { componentDidMount () { @@ -153,7 +154,8 @@ class ContextMenuItem extends ImmutableComponent { contextMenuItem: true, hasFaIcon: faIcon, checkedMenuItem: this.props.contextMenuItem.get('checked'), - hasIcon: icon || faIcon + hasIcon: icon || faIcon, + selectedByKeyboard: this.props.selected })} role='listitem' draggable={this.props.contextMenuItem.get('draggable')} @@ -210,17 +212,28 @@ class ContextMenuSingle extends ImmutableComponent { if (this.props.y) { styles.top = this.props.y } + let index = 0 + const selectedIndex = this.props.selectedIndex return
{ - this.props.template.map((contextMenuItem) => - ) + this.props.template.map((contextMenuItem) => { + let props = { + contextMenuItem: contextMenuItem, + submenuIndex: this.props.submenuIndex, + lastZoomPercentage: this.props.lastZoomPercentage, + contextMenuDetail: this.props.contextMenuDetail, + selected: false + } + // don't count separators when finding selectedIndex + if (contextMenuItem.get('type') !== separatorMenuItem.type) { + props.selected = index === this.props.selectedIndex + index++ + } + return + }) }
} @@ -267,14 +280,16 @@ class ContextMenu extends ImmutableComponent { + template={this.props.contextMenuDetail.get('template')} + selectedIndex={this.props.selectedIndex} /> { this.openedSubmenuDetails.map((openedSubmenuDetail, i) => ) + y={openedSubmenuDetail.get('y')} + selectedIndex={this.props.selectedIndex} />) }
} diff --git a/js/components/main.js b/js/components/main.js index 5be5e8410f6..b1bd73d94d1 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -737,6 +737,7 @@ class Main extends ImmutableComponent { menubarVisible: menubarVisible, menubarTemplate: menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null, menubarSelectedLabel: this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']), + menubarSelectedIndex: this.props.windowState.getIn(['ui', 'menubar', 'selectedIndex']), isMaximized: this.props.windowState.getIn(['ui', 'isMaximized']) } } @@ -794,7 +795,8 @@ class Main extends ImmutableComponent { this.props.windowState.get('contextMenuDetail') ? + contextMenuDetail={this.props.windowState.get('contextMenuDetail')} + selectedIndex={customTitlebar.menubarSelectedIndex} /> : null } { @@ -813,6 +815,7 @@ class Main extends ImmutableComponent {
: null diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 9d68c4b9f55..114ecb5f10c 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -71,8 +71,9 @@ const windowConstants = { WINDOW_SET_BLOCKED_RUN_INSECURE_CONTENT: _, WINDOW_TOGGLE_MENUBAR_VISIBLE: _, WINDOW_CLICK_MENUBAR_SUBMENU: _, - WINDOW_SET_MENUBAR_ITEM_SELECTED: _, + WINDOW_SET_MENUBAR_SELECTED_LABEL: _, WINDOW_RESET_MENU_STATE: _, + WINDOW_SET_SUBMENU_SELECTED_INDEX: _, WINDOW_GOT_RESPONSE_DETAILS: _ } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 6f7880255ac..b8c1ac5e397 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -789,12 +789,12 @@ const doAction = (action) => { : !windowState.getIn(['ui', 'menubar', 'isVisible']) // Clear selection when menu is shown if (newVisibleStatus) { - doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED}) + doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL}) } windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], newVisibleStatus) } break - case WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED: + case WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL: windowState = windowState.setIn(['ui', 'menubar', 'selectedLabel'], action.label && typeof action.label === 'string' ? action.label @@ -802,12 +802,20 @@ const doAction = (action) => { break case WindowConstants.WINDOW_RESET_MENU_STATE: doAction({actionType: WindowConstants.WINDOW_SET_POPUP_WINDOW_DETAIL}) - doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_ITEM_SELECTED}) + doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL}) if (getSetting(settings.AUTO_HIDE_MENU)) { doAction({actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, isVisible: false}) } else { doAction({actionType: WindowConstants.WINDOW_SET_CONTEXT_MENU_DETAIL}) } + doAction({actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX}) + break + case WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX: + const proposedIndex = Number(action.index) + windowState = windowState.setIn(['ui', 'menubar', 'selectedIndex'], + isNaN(proposedIndex) + ? null + : proposedIndex) break default: diff --git a/less/contextMenu.less b/less/contextMenu.less index 611c51cf027..ddb252a80d4 100644 --- a/less/contextMenu.less +++ b/less/contextMenu.less @@ -43,6 +43,11 @@ } } + .selectedByKeyboard { + background-color: lighten(@menuSelectionColor, 5%); + color: white; + } + .contextMenuIcon { font-size: 14px; margin-right: 8px; diff --git a/test/unit/lib/formatUtilTest.js b/test/unit/lib/formatUtilTest.js index 7ab00c6b29c..134f55efa4e 100644 --- a/test/unit/lib/formatUtilTest.js +++ b/test/unit/lib/formatUtilTest.js @@ -5,43 +5,57 @@ const assert = require('assert') require('../braveUnit') describe('formatUtil', function () { - describe('Windows and Linux', function () { - before(function () { - this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') - Object.defineProperty(process, 'platform', { - value: 'win32' + describe('formatAccelerator', function () { + describe('when platform is Windows', function () { + before(function () { + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { + value: 'win32' + }) }) - }) - after(function () { - Object.defineProperty(process, 'platform', this.originalPlatform) - }) + after(function () { + Object.defineProperty(process, 'platform', this.originalPlatform) + }) - it('puts the modifiers in the correct order', function () { - const result = formatUtil.formatAccelerator('A+Shift+Alt+CmdOrCtrl') - assert.equal(result, 'Ctrl+Alt+Shift+A') - }) - it('leaves modifiers alone if order is correct', function () { - const result = formatUtil.formatAccelerator('Ctrl+Shift+O') - assert.equal(result, 'Ctrl+Shift+O') + it('puts the modifiers in the correct order', function () { + const result = formatUtil.formatAccelerator('A+Shift+Alt+CmdOrCtrl') + assert.equal(result, 'Ctrl+Alt+Shift+A') + }) + it('leaves modifiers alone if order is correct', function () { + const result = formatUtil.formatAccelerator('Ctrl+Shift+O') + assert.equal(result, 'Ctrl+Shift+O') + }) }) - }) - describe('Mac', function () { - before(function () { - this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') - Object.defineProperty(process, 'platform', { - value: 'darwin' + describe('when platform is macOS', function () { + before(function () { + this.originalPlatform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { + value: 'darwin' + }) + }) + + after(function () { + Object.defineProperty(process, 'platform', this.originalPlatform) }) - }) - after(function () { - Object.defineProperty(process, 'platform', this.originalPlatform) + it('replaces the key names with the correct symbols', function () { + const result = formatUtil.formatAccelerator('Alt+CmdOrCtrl+Ctrl+Shift+O') + assert.equal(result, '⌥⇧^⌘O') + }) }) + }) - it('replaces the key names with the correct symbols', function () { - const result = formatUtil.formatAccelerator('Alt+CmdOrCtrl+Ctrl+Shift+O') - assert.equal(result, '⌥⇧^⌘O') + describe('wrappingClamp', function () { + it('does not change value if within bounds', function () { + assert.equal(formatUtil.wrappingClamp(5, 1, 10), 5) + }) + it('wraps negatively', function () { + assert.equal(formatUtil.wrappingClamp(-7, 1, 10), 3) + }) + it('wraps positively', function () { + assert.equal(formatUtil.wrappingClamp(18, 1, 10), 8) }) }) }) From 1daef9a1f576c7d5a68fa363216ca59c402c909b Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sun, 18 Sep 2016 10:43:47 -0700 Subject: [PATCH 13/17] Fixing known issues: - Menu now sets default selection when you press ALT to show it - Menu now behaves properly when always displayed (auto-hide=false) - ALT and ESC now are properly handled; providing windows-like behavior (deselect and/or closing menu, etc) - Fix click problem with extension button (it was tagged as draggable) - Windows specific fixes for drag. Tested each area that was changed, comparing this build to 0.12.1 to make sure it's consistent. - menu now returns focus properly to active web frame - the pseudo click now sends over webContents (required for cut/copy) - Bumped up minimum viewport width to 640px for Windows only. Media query strips margin left of the brave button unless you're at 720px or more (again- windows only) --- app/browser/menu.js | 3 ++- app/renderer/components/menubar.js | 37 ++++++++++++++++++------------ docs/appActions.md | 10 ++++++++ docs/windowActions.md | 2 +- js/actions/windowActions.js | 5 ++-- js/components/contextMenu.js | 1 - js/components/main.js | 34 ++++++++++++++++++++++++--- js/stores/appStore.js | 2 +- js/stores/windowStore.js | 6 ++++- less/bookmarksToolbar.less | 7 ++++++ less/navigationBar.less | 18 +++++++++++---- less/tabs.less | 7 ++++++ less/variables.less | 1 + 13 files changed, 104 insertions(+), 29 deletions(-) diff --git a/app/browser/menu.js b/app/browser/menu.js index 1242da21db4..f0e7a8934ad 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -650,7 +650,8 @@ const doAction = (action) => { appDispatcher.waitFor([appStore.dispatchToken], () => { const clickedMenuItem = menuUtil.getMenuItem(appMenu, action.label) if (clickedMenuItem) { - clickedMenuItem.click(clickedMenuItem, BrowserWindow.getFocusedWindow()) + const focusedWindow = BrowserWindow.getFocusedWindow() + clickedMenuItem.click(clickedMenuItem, focusedWindow, focusedWindow.webContents) } }) break diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index d92895d5c10..2a970dfeaec 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -10,7 +10,7 @@ const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem const keyCodes = require('../../../js/constants/keyCodes') const { wrappingClamp } = require('../../common/lib/formatUtil') -const showContextMenu = (rect, submenu) => { +const showContextMenu = (rect, submenu, activeFrame) => { windowActions.setContextMenuDetail(Immutable.fromJS({ left: rect.left, top: rect.bottom, @@ -19,6 +19,14 @@ const showContextMenu = (rect, submenu) => { return submenuItem } submenuItem.click = function (e) { + e.preventDefault() + if (activeFrame && activeFrame.has('key')) { + // Send focus back to the active web frame + const results = document.querySelectorAll('webview[data-frame-key="' + activeFrame.get('key') + '"]') + if (results.length === 1) { + results[0].focus() + } + } windowActions.clickMenubarSubmenu(submenuItem.label) } return submenuItem @@ -46,7 +54,7 @@ class MenubarItem extends ImmutableComponent { // Otherwise, mark item as selected and show its context menu windowActions.setMenubarSelectedLabel(this.props.label) const rect = e.target.getBoundingClientRect() - showContextMenu(rect, this.props.submenu) + showContextMenu(rect, this.props.submenu, this.props.activeFrame) } onMouseOver (e) { const selected = this.props.menubar.props.selectedLabel @@ -76,6 +84,9 @@ class Menubar extends ImmutableComponent { this.onKeyDown = this.onKeyDown.bind(this) document.addEventListener('keydown', this.onKeyDown) } + componentWillUnmount () { + document.removeEventListener('keydown', this.onKeyDown) + } getTemplateByLabel (label) { const element = this.props.template.find((element) => { return element.get('label') === label @@ -101,7 +112,7 @@ class Menubar extends ImmutableComponent { get selectedIndexMax () { const result = this.selectedTemplateItemsOnly if (result && Array.isArray(result)) { - return result.length; + return result.length } return 0 } @@ -116,12 +127,6 @@ class Menubar extends ImmutableComponent { return this.getRectByLabel(this.props.selectedLabel) } onKeyDown (e) { - // BSCTODO: how to handle if menu is ALWAYS showing? - // we can't just eat (preventDefault) the arrow keys :( - // BSCTODO: keyup handler needed: - // - for ESC, to set selection to null (context menu already handles ESC properly) - // - for ALT, unset selection and hide menu - // switch (e.which) { case keyCodes.ENTER: e.preventDefault() @@ -134,6 +139,8 @@ class Menubar extends ImmutableComponent { case keyCodes.LEFT: case keyCodes.RIGHT: + if (!this.props.autohide && !this.props.selectedLabel) break + e.preventDefault() if (this.props.template.size > 0) { const selectedIndex = this.props.template.findIndex((element) => { @@ -156,19 +163,21 @@ class Menubar extends ImmutableComponent { // Context menu already being displayed; auto-open the next one if (this.props.contextMenuDetail && this.selectedTemplate && nextRect) { windowActions.setSubmenuSelectedIndex(0) - showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS()) + showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.activeFrame) } } break case keyCodes.UP: case keyCodes.DOWN: + if (!this.props.autohide && !this.props.selectedLabel) break + e.preventDefault() if (this.props.selectedLabel && this.selectedTemplate) { if (!this.props.contextMenuDetail && this.selectedRect) { // First time hitting up/down; popup the context menu windowActions.setSubmenuSelectedIndex(0) - showContextMenu(this.selectedRect, this.selectedTemplate.toJS()) + showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.activeFrame) } else { // Context menu already visible; move selection up or down const nextIndex = wrappingClamp( @@ -181,9 +190,6 @@ class Menubar extends ImmutableComponent { break } } - componentWillUnmount () { - document.removeEventListener('keydown', this.onKeyDown) - } shouldComponentUpdate (nextProps, nextState) { return this.props.selectedLabel !== nextProps.selectedLabel } @@ -194,7 +200,8 @@ class Menubar extends ImmutableComponent { let props = { label: menubarItem.get('label'), submenu: menubarItem.get('submenu').toJS(), - menubar: this + menubar: this, + activeFrame: this.props.activeFrame } if (props.label === this.props.selectedLabel) { props.selected = true diff --git a/docs/appActions.md b/docs/appActions.md index 0cc3cbde426..0b8e110151a 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -403,6 +403,16 @@ Dispatches a message when appWindowId loses focus +### setMenubarTemplate(menubarTemplate) + +Saves current menubar template for use w/ Windows titlebar + +**Parameters** + +**menubarTemplate**: `Object`, JSON used to build the menu + + + ### networkConnected() Dispatches a message when the network is re-connected diff --git a/docs/windowActions.md b/docs/windowActions.md index 58c0d8fa67d..11c03490129 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -811,7 +811,7 @@ Called from the Menubar control, handled in menu.js -### setMenubarItemSelected(label) +### setMenubarSelectedLabel(label) (Windows only) Used to track which menubar item is currently selected (or null for none selected) diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 74ca229f8e9..e50c767f475 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1012,10 +1012,11 @@ const windowActions = { * Dispatches a message to indicate the custom rendered Menubar should be toggled (shown/hidden) * @param {boolean} isVisible (optional) */ - toggleMenubarVisible: function (isVisible) { + toggleMenubarVisible: function (isVisible, defaultLabel) { dispatch({ actionType: WindowConstants.WINDOW_TOGGLE_MENUBAR_VISIBLE, - isVisible + isVisible, + defaultLabel }) }, diff --git a/js/components/contextMenu.js b/js/components/contextMenu.js index 28880375059..f734240a2fe 100644 --- a/js/components/contextMenu.js +++ b/js/components/contextMenu.js @@ -213,7 +213,6 @@ class ContextMenuSingle extends ImmutableComponent { styles.top = this.props.y } let index = 0 - const selectedIndex = this.props.selectedIndex return
+ contextMenuDetail={this.props.windowState.get('contextMenuDetail')} + autohide={getSetting(settings.AUTO_HIDE_MENU)} + activeFrame={activeFrame} />
: null } diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 66589db6244..c69d8a73d4e 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -257,7 +257,7 @@ function windowDefaults () { show: false, width: appState.get('defaultWindowWidth'), height: appState.get('defaultWindowHeight'), - minWidth: 480, + minWidth: isWindows ? 640 : 480, // Windows has caption buttons (min/max/close) which take extra space minHeight: 300, minModalHeight: 100, minModalWidth: 100, diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index b8c1ac5e397..08fc9d04f53 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -789,7 +789,11 @@ const doAction = (action) => { : !windowState.getIn(['ui', 'menubar', 'isVisible']) // Clear selection when menu is shown if (newVisibleStatus) { - doAction({actionType: WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL}) + const actionProps = { actionType: WindowConstants.WINDOW_SET_MENUBAR_SELECTED_LABEL } + if (action.defaultLabel) { + actionProps.label = action.defaultLabel + } + doAction(actionProps) } windowState = windowState.setIn(['ui', 'menubar', 'isVisible'], newVisibleStatus) } diff --git a/less/bookmarksToolbar.less b/less/bookmarksToolbar.less index 8eedb824dd2..b39e4cb75a2 100644 --- a/less/bookmarksToolbar.less +++ b/less/bookmarksToolbar.less @@ -13,6 +13,13 @@ --bookmarks-toolbar-padding: 10px; } +// (Windows) Bookmarks toolbar not right-clickable unless it has no-drag +.platform--win32 .bookmarksToolbar { + &.allowDragging { + -webkit-app-region: no-drag !important; + } +} + .bookmarksToolbar { background: @toolbarBackground; border-bottom: 1px solid #aaaaaa; diff --git a/less/navigationBar.less b/less/navigationBar.less index ca4ed1d2ed4..3d14dd6204d 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -18,7 +18,7 @@ } } -// On MacOS we need to keep a padding left to avoid overlapping +// (macOS) We need to keep a padding left to avoid overlapping // with the window buttons to close/maximize/minimize the window. .platform--darwin .navigatorWrapper .backforward { margin-left: @navbarLeftMarginDarwin; @@ -29,16 +29,26 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } -// On Windows, we add extra padding to have spacing match the close button +// (Windows) Add extra padding to have spacing match the close button .platform--win32 .navigatorWrapper { margin-left: @navbarLeftMarginWindows; } - -// Style adjustments needed for slim titlebar to work properly +// (Windows) Style adjustments needed for slim titlebar to work properly .platform--win32 div#window.frameless { border: 1px solid #000; box-sizing: border-box; } +// (Windows) Extension button not clickable unless it has no-drag +.platform--win32 .extensionButton { + -webkit-app-region: no-drag !important; +} +// (Windows) remove margin next to brave button when lower than 720px +.platform--win32 .navigatorWrapper .topLevelEndButtons { + @media (max-width: @windowsMinimumViewport) { + margin-left: 0; + } +} + .navbarCaptionButtonContainer { display: flex; border-bottom: 1px solid #aaaaaa; diff --git a/less/tabs.less b/less/tabs.less index c83aaa02c66..59b3e645b34 100644 --- a/less/tabs.less +++ b/less/tabs.less @@ -4,6 +4,13 @@ @import "variables.less"; +// (Windows) Tabs row not right-clickable unless it has no-drag +.platform--win32 .tabStripContainer { + &.allowDragging { + -webkit-app-region: no-drag !important; + } +} + .tabs { background: @tabsBackground; box-sizing: border-box; diff --git a/less/variables.less b/less/variables.less index 95462fe659e..73c5411dfc9 100644 --- a/less/variables.less +++ b/less/variables.less @@ -128,6 +128,7 @@ @zindexWindowFullScreenBanner: 4100; @breakpointNarrowViewport: 600px; +@windowsMinimumViewport: 720px; @transitionDuration: 100ms; @transition: all 600ms linear; From 02564f1b9095b5e22895888db6572d36413043eb Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 19 Sep 2016 14:00:22 -0700 Subject: [PATCH 14/17] Fixes: - Properly handle all accelerator modifiers (was missing CommandOrControl and some replacement cases) - Updated code to skip rendering / processing for hidden context menu items --- app/common/lib/formatUtil.js | 6 ++++++ app/renderer/components/menubar.js | 21 +++++++++------------ js/components/contextMenu.js | 8 +++++++- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/app/common/lib/formatUtil.js b/app/common/lib/formatUtil.js index 869217e623e..1c7d314db5a 100644 --- a/app/common/lib/formatUtil.js +++ b/app/common/lib/formatUtil.js @@ -17,6 +17,7 @@ const macOrderLookup = (value) => { return 2 case 'Super': case 'CmdOrCtrl': + case 'CommandOrControl': case 'Command': case 'Cmd': return 3 @@ -27,6 +28,7 @@ const macOrderLookup = (value) => { const defaultOrderLookup = (value) => { switch (value) { case 'CmdOrCtrl': + case 'CommandOrControl': case 'Control': case 'Ctrl': return 0 @@ -57,6 +59,7 @@ module.exports.formatAccelerator = (accelerator) => { }) // NOTE: these characters might only show properly on Mac result = splitResult.join('') + result = result.replace('CommandOrControl', '⌘') result = result.replace('CmdOrCtrl', '⌘') result = result.replace('Command', '⌘') result = result.replace('Cmd', '⌘') @@ -74,11 +77,14 @@ module.exports.formatAccelerator = (accelerator) => { return -1 }) result = splitResult.join('+') + result = result.replace('CommandOrControl', 'Ctrl') result = result.replace('CmdOrCtrl', 'Ctrl') + result = result.replace('Control', 'Ctrl') } return result } + /** * Clamp values down to a given range (min/max). * Value is wrapped when out of bounds. ex: diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index 2a970dfeaec..adca0bf8d53 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -82,6 +82,8 @@ class Menubar extends ImmutableComponent { constructor () { super() this.onKeyDown = this.onKeyDown.bind(this) + } + componentWillMount () { document.addEventListener('keydown', this.onKeyDown) } componentWillUnmount () { @@ -97,22 +99,17 @@ class Menubar extends ImmutableComponent { return this.getTemplateByLabel(this.props.selectedLabel) } get selectedTemplateItemsOnly () { - // this exclude the separators - return this.selectedTemplate.reduce((previousValue, currentValue, currentIndex, array) => { - const result = currentIndex === 1 ? [] : previousValue - if (currentIndex === 1 && previousValue.get('type') !== separatorMenuItem.type) { - result.push(previousValue) - } - if (currentValue.get('type') !== separatorMenuItem.type) { - result.push(currentValue) - } - return result + // exclude the separators AND items that are not visible + return this.selectedTemplate.filter((element) => { + if (element.get('type') === separatorMenuItem.type) return false + if (element.has('visible')) return element.get('visible') + return true }) } get selectedIndexMax () { const result = this.selectedTemplateItemsOnly - if (result && Array.isArray(result)) { - return result.length + if (result && result.size && result.size > 0) { + return result.size } return 0 } diff --git a/js/components/contextMenu.js b/js/components/contextMenu.js index f734240a2fe..95045cdfdd0 100644 --- a/js/components/contextMenu.js +++ b/js/components/contextMenu.js @@ -212,13 +212,19 @@ class ContextMenuSingle extends ImmutableComponent { if (this.props.y) { styles.top = this.props.y } + const visibleMenuItems = this.props.template.filter((element) => { + return element.has('visible') + ? element.get('visible') + : true + }) + let index = 0 return
{ - this.props.template.map((contextMenuItem) => { + visibleMenuItems.map((contextMenuItem) => { let props = { contextMenuItem: contextMenuItem, submenuIndex: this.props.submenuIndex, From c0240a5d34a1e652df3c0d4f982e60d79da0589c Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 19 Sep 2016 15:58:15 -0700 Subject: [PATCH 15/17] Fix issue w/ last commit, added cleanup after evaluating moving menu from appState to windowState (not going to move it after talking w/ @bridiver). --- app/common/lib/formatUtil.js | 1 - app/renderer/components/menubar.js | 2 +- js/components/main.js | 5 +++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/common/lib/formatUtil.js b/app/common/lib/formatUtil.js index 1c7d314db5a..9d917ad4ee4 100644 --- a/app/common/lib/formatUtil.js +++ b/app/common/lib/formatUtil.js @@ -84,7 +84,6 @@ module.exports.formatAccelerator = (accelerator) => { return result } - /** * Clamp values down to a given range (min/max). * Value is wrapped when out of bounds. ex: diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index adca0bf8d53..6225c6be9fb 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -128,7 +128,7 @@ class Menubar extends ImmutableComponent { case keyCodes.ENTER: e.preventDefault() if (this.selectedTemplate) { - const selectedLabel = this.selectedTemplateItemsOnly[this.props.selectedIndex].get('label') + const selectedLabel = this.selectedTemplateItemsOnly.getIn([this.props.selectedIndex, 'label']) windowActions.clickMenubarSubmenu(selectedLabel) windowActions.resetMenuState() } diff --git a/js/components/main.js b/js/components/main.js index 95ce9b1c55f..567e1d43dea 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -128,8 +128,9 @@ class Main extends ImmutableComponent { if (customTitlebar.enabled) { e.preventDefault() - const menubarTemplate = this.props.appState.getIn(['menu', 'template']) - const defaultLabel = menubarTemplate.getIn([0, 'label']) + const defaultLabel = customTitlebar.menubarTemplate + ? customTitlebar.menubarTemplate.getIn([0, 'label']) + : null if (getSetting(settings.AUTO_HIDE_MENU)) { windowActions.toggleMenubarVisible(null, defaultLabel) From 75365f0bc5f6009ac4a11dfcfd6ca59f7b9f8a64 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Mon, 19 Sep 2016 17:22:40 -0700 Subject: [PATCH 16/17] Window can be as skinny as 480px wide on Windows now without issues. Works well with URL bar always showing and also with title mode. --- js/stores/appStore.js | 2 +- less/navigationBar.less | 48 ++++++++++++++++++++++++++--------------- less/variables.less | 4 +++- 3 files changed, 35 insertions(+), 19 deletions(-) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index c69d8a73d4e..66589db6244 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -257,7 +257,7 @@ function windowDefaults () { show: false, width: appState.get('defaultWindowWidth'), height: appState.get('defaultWindowHeight'), - minWidth: isWindows ? 640 : 480, // Windows has caption buttons (min/max/close) which take extra space + minWidth: 480, minHeight: 300, minModalHeight: 100, minModalWidth: 100, diff --git a/less/navigationBar.less b/less/navigationBar.less index 3d14dd6204d..8620651885f 100644 --- a/less/navigationBar.less +++ b/less/navigationBar.less @@ -29,26 +29,40 @@ - (@navbarBraveButtonWidth + 2 * @navbarButtonSpacing); } -// (Windows) Add extra padding to have spacing match the close button -.platform--win32 .navigatorWrapper { - margin-left: @navbarLeftMarginWindows; -} -// (Windows) Style adjustments needed for slim titlebar to work properly -.platform--win32 div#window.frameless { - border: 1px solid #000; - box-sizing: border-box; -} -// (Windows) Extension button not clickable unless it has no-drag -.platform--win32 .extensionButton { - -webkit-app-region: no-drag !important; -} -// (Windows) remove margin next to brave button when lower than 720px -.platform--win32 .navigatorWrapper .topLevelEndButtons { - @media (max-width: @windowsMinimumViewport) { - margin-left: 0; +// Windows specific fixes +.platform--win32 { + .navigatorWrapper { + margin-left: @navbarLeftMarginWindows; + } + + div#window.frameless { + border: 1px solid #000; + box-sizing: border-box; + } + + // Extension button not clickable unless it has no-drag + .extensionButton { + -webkit-app-region: no-drag !important; + } + + // changes to ensure window can be as small as 480px wide + // and still be useable and have the caption buttons intact + @media (max-width: @breakpointExtensionButtonPadding) { + .navigatorWrapper .topLevelEndButtons { + margin-left: 0; + } + } + @media (max-width: @breakpointSmallWin32) { + .loadTime { display: none; } + #urlInput { max-width: 75px; } + #titleBar { width: 100px; } + } + @media (max-width: @breakpointTinyWin32) { + #urlInput { max-width: 50px; } } } +// Styles had to be reworked to properly position the new caption buttons for Windows .navbarCaptionButtonContainer { display: flex; border-bottom: 1px solid #aaaaaa; diff --git a/less/variables.less b/less/variables.less index 73c5411dfc9..3ce797d11ea 100644 --- a/less/variables.less +++ b/less/variables.less @@ -128,7 +128,9 @@ @zindexWindowFullScreenBanner: 4100; @breakpointNarrowViewport: 600px; -@windowsMinimumViewport: 720px; +@breakpointExtensionButtonPadding: 720px; +@breakpointSmallWin32: 650px; +@breakpointTinyWin32: 500px; @transitionDuration: 100ms; @transition: all 600ms linear; From 39d2b0c35ce02c0e88c1ab9cad372d5cbdd291a6 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Tue, 20 Sep 2016 12:05:37 -0700 Subject: [PATCH 17/17] Fixes focus issues by: - capturing the browser's selection changes - converting the activeElement object to a selector - returning focus after menu is interacted with by: - using document.querySelectAll() to return focus to original element - executing window action which actually handles the menu action This commit also cleans up `main.js` by properly encapsulating the handlers required into a new method, `registerCustomTitlebarHandlers` cc: @bbondy --- app/renderer/components/menubar.js | 14 ++++----- docs/state.md | 3 +- docs/windowActions.md | 42 ++++++++++++++++++++++++- js/actions/windowActions.js | 30 +++++++++++++++++- js/components/main.js | 49 ++++++++++++++++++++---------- js/constants/windowConstants.js | 1 + js/stores/windowStore.js | 3 ++ 7 files changed, 116 insertions(+), 26 deletions(-) diff --git a/app/renderer/components/menubar.js b/app/renderer/components/menubar.js index 6225c6be9fb..a0b62a21c03 100644 --- a/app/renderer/components/menubar.js +++ b/app/renderer/components/menubar.js @@ -10,7 +10,7 @@ const separatorMenuItem = require('../../common/commonMenu').separatorMenuItem const keyCodes = require('../../../js/constants/keyCodes') const { wrappingClamp } = require('../../common/lib/formatUtil') -const showContextMenu = (rect, submenu, activeFrame) => { +const showContextMenu = (rect, submenu, lastFocusedSelector) => { windowActions.setContextMenuDetail(Immutable.fromJS({ left: rect.left, top: rect.bottom, @@ -20,9 +20,9 @@ const showContextMenu = (rect, submenu, activeFrame) => { } submenuItem.click = function (e) { e.preventDefault() - if (activeFrame && activeFrame.has('key')) { + if (lastFocusedSelector) { // Send focus back to the active web frame - const results = document.querySelectorAll('webview[data-frame-key="' + activeFrame.get('key') + '"]') + const results = document.querySelectorAll(lastFocusedSelector) if (results.length === 1) { results[0].focus() } @@ -54,7 +54,7 @@ class MenubarItem extends ImmutableComponent { // Otherwise, mark item as selected and show its context menu windowActions.setMenubarSelectedLabel(this.props.label) const rect = e.target.getBoundingClientRect() - showContextMenu(rect, this.props.submenu, this.props.activeFrame) + showContextMenu(rect, this.props.submenu, this.props.lastFocusedSelector) } onMouseOver (e) { const selected = this.props.menubar.props.selectedLabel @@ -160,7 +160,7 @@ class Menubar extends ImmutableComponent { // Context menu already being displayed; auto-open the next one if (this.props.contextMenuDetail && this.selectedTemplate && nextRect) { windowActions.setSubmenuSelectedIndex(0) - showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.activeFrame) + showContextMenu(nextRect, this.getTemplateByLabel(nextLabel).toJS(), this.props.lastFocusedSelector) } } break @@ -174,7 +174,7 @@ class Menubar extends ImmutableComponent { if (!this.props.contextMenuDetail && this.selectedRect) { // First time hitting up/down; popup the context menu windowActions.setSubmenuSelectedIndex(0) - showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.activeFrame) + showContextMenu(this.selectedRect, this.selectedTemplate.toJS(), this.props.lastFocusedSelector) } else { // Context menu already visible; move selection up or down const nextIndex = wrappingClamp( @@ -198,7 +198,7 @@ class Menubar extends ImmutableComponent { label: menubarItem.get('label'), submenu: menubarItem.get('submenu').toJS(), menubar: this, - activeFrame: this.props.activeFrame + lastFocusedSelector: this.props.lastFocusedSelector } if (props.label === this.props.selectedLabel) { props.selected = true diff --git a/docs/state.md b/docs/state.md index e8717d3ce25..9ddfe464ba0 100644 --- a/docs/state.md +++ b/docs/state.md @@ -374,7 +374,8 @@ WindowStore menubar: { // windows only isVisible: boolean, // true if Menubar control is visible selectedLabel: string, // label of menu that is selected (or null for none selected) - selectedIndex: number // index of the selected context menu item + selectedIndex: number, // index of the selected context menu item + lastFocusedSelector: string // selector for the last selected element (browser ui, not frame content) } }, searchDetail: { diff --git a/docs/windowActions.md b/docs/windowActions.md index 11c03490129..4a6c6c7fcb1 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -824,7 +824,7 @@ Used to track which menubar item is currently selected (or null for none selecte ### resetMenuState() -Used by main.js when click happens on content area (not on a link or react control). +Used by `main.js` when click happens on content area (not on a link or react control). - closes context menu - closes popup menu - nulls out menubar item selected (Windows only) @@ -832,6 +832,46 @@ Used by main.js when click happens on content area (not on a link or react contr +### setSubmenuSelectedIndex(index) + +(Windows only) +Used to track selected index of a context menu +Needed because arrow keys can be used to navigate the custom menu + +**Parameters** + +**index**: `number`, zero based index of the item. + Index excludes menu separators and hidden items. + + + +### setLastFocusedSelector(selector) + +(Windows only at the moment) +Used to track last selected element (typically the URL bar or the frame) +Important because focus is lost when using the custom menu and needs +to be returned in order for the cut/copy operation to work + +**Parameters** + +**selector**: `string`, selector used w/ querySelectorAll to return focus + after a menu item is selected (via the custom titlebar / menubar) + + + +### gotResponseDetails(tabId, details) + +Used to get response details (such as the HTTP response code) from a response +See `eventStore.js` for an example use-case + +**Parameters** + +**tabId**: `number`, the tab id to set + +**details**: `Object`, object containing response details + + + * * * diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index e50c767f475..60f62d7cf3b 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1046,7 +1046,7 @@ const windowActions = { }, /** - * Used by main.js when click happens on content area (not on a link or react control). + * Used by `main.js` when click happens on content area (not on a link or react control). * - closes context menu * - closes popup menu * - nulls out menubar item selected (Windows only) @@ -1058,6 +1058,13 @@ const windowActions = { }) }, + /** + * (Windows only) + * Used to track selected index of a context menu + * Needed because arrow keys can be used to navigate the custom menu + * @param {number} index - zero based index of the item. + * Index excludes menu separators and hidden items. + */ setSubmenuSelectedIndex: function (index) { dispatch({ actionType: WindowConstants.WINDOW_SET_SUBMENU_SELECTED_INDEX, @@ -1065,6 +1072,27 @@ const windowActions = { }) }, + /** + * (Windows only at the moment) + * Used to track last selected element (typically the URL bar or the frame) + * Important because focus is lost when using the custom menu and needs + * to be returned in order for the cut/copy operation to work + * @param {string} selector - selector used w/ querySelectorAll to return focus + * after a menu item is selected (via the custom titlebar / menubar) + */ + setLastFocusedSelector: function (selector) { + dispatch({ + actionType: WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR, + selector + }) + }, + + /** + * Used to get response details (such as the HTTP response code) from a response + * See `eventStore.js` for an example use-case + * @param {number} tabId - the tab id to set + * @param {Object} details - object containing response details + */ gotResponseDetails: function (tabId, details) { dispatch({ actionType: WindowConstants.WINDOW_GOT_RESPONSE_DETAILS, diff --git a/js/components/main.js b/js/components/main.js index 567e1d43dea..2ab67795d72 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -121,16 +121,18 @@ class Main extends ImmutableComponent { break } }) - document.addEventListener('keyup', (e) => { - const customTitlebar = this.customTitlebar - switch (e.which) { - case keyCodes.ALT: - if (customTitlebar.enabled) { + } + + registerCustomTitlebarHandlers () { + if (this.customTitlebar.enabled) { + document.addEventListener('keyup', (e) => { + const customTitlebar = this.customTitlebar + switch (e.which) { + case keyCodes.ALT: e.preventDefault() - const defaultLabel = customTitlebar.menubarTemplate - ? customTitlebar.menubarTemplate.getIn([0, 'label']) - : null + const menubarTemplate = this.props.appState.getIn(['menu', 'template']) + const defaultLabel = menubarTemplate.getIn([0, 'label']) if (getSetting(settings.AUTO_HIDE_MENU)) { windowActions.toggleMenubarVisible(null, defaultLabel) @@ -142,10 +144,8 @@ class Main extends ImmutableComponent { windowActions.setMenubarSelectedLabel(defaultLabel) } } - } - break - case keyCodes.ESC: - if (customTitlebar.enabled) { + break + case keyCodes.ESC: if (getSetting(settings.AUTO_HIDE_MENU) && customTitlebar.menubarVisible && !customTitlebar.menubarSelectedLabel) { e.preventDefault() windowActions.toggleMenubarVisible(false) @@ -156,10 +156,25 @@ class Main extends ImmutableComponent { windowActions.setMenubarSelectedLabel() windowActions.setContextMenuDetail() } + break + } + }) + + document.addEventListener('focus', (e) => { + let selector = document.activeElement.id + ? '#' + document.activeElement.id + : null + + if (!selector && document.activeElement.tagName === 'WEBVIEW') { + const frameKeyAttribute = document.activeElement.getAttribute('data-frame-key') + if (frameKeyAttribute) { + selector = 'webview[data-frame-key="' + frameKeyAttribute + '"]' } - break - } - }) + } + + windowActions.setLastFocusedSelector(selector) + }, true) + } } exitFullScreen () { @@ -298,6 +313,7 @@ class Main extends ImmutableComponent { componentDidMount () { this.registerSwipeListener() this.registerWindowLevelShortcuts() + this.registerCustomTitlebarHandlers() ipc.on(messages.SHORTCUT_NEW_FRAME, (event, url, options = {}) => { if (options.singleFrame) { @@ -765,6 +781,7 @@ class Main extends ImmutableComponent { menubarTemplate: menubarVisible ? this.props.appState.getIn(['menu', 'template']) : null, menubarSelectedLabel: this.props.windowState.getIn(['ui', 'menubar', 'selectedLabel']), menubarSelectedIndex: this.props.windowState.getIn(['ui', 'menubar', 'selectedIndex']), + lastFocusedSelector: this.props.windowState.getIn(['ui', 'menubar', 'lastFocusedSelector']), isMaximized: this.props.windowState.getIn(['ui', 'isMaximized']) } } @@ -845,7 +862,7 @@ class Main extends ImmutableComponent { selectedIndex={customTitlebar.menubarSelectedIndex} contextMenuDetail={this.props.windowState.get('contextMenuDetail')} autohide={getSetting(settings.AUTO_HIDE_MENU)} - activeFrame={activeFrame} /> + lastFocusedSelector={customTitlebar.lastFocusedSelector} />
: null } diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 114ecb5f10c..b915f71c92a 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -74,6 +74,7 @@ const windowConstants = { WINDOW_SET_MENUBAR_SELECTED_LABEL: _, WINDOW_RESET_MENU_STATE: _, WINDOW_SET_SUBMENU_SELECTED_INDEX: _, + WINDOW_SET_LAST_FOCUSED_SELECTOR: _, WINDOW_GOT_RESPONSE_DETAILS: _ } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 08fc9d04f53..049230f72c6 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -821,6 +821,9 @@ const doAction = (action) => { ? null : proposedIndex) break + case WindowConstants.WINDOW_SET_LAST_FOCUSED_SELECTOR: + windowState = windowState.setIn(['ui', 'menubar', 'lastFocusedSelector'], action.selector) + break default: }