Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

Commit 67ee2c7

Browse files
committed
Tweaked code from @lucidNTR and added right click to open to LongPressButton
Fixes #1594 Auditors: @lucidNTR, @cezaraugusto, @jkup Test Plan: 1. Launch Brave and right click the new tab button "+" 2. Verify that it shows New Private / New Session / New Window 3. Cancel the menu and then click and hold the new tab button "+" 4. After ~300ms, the menu should show up 5. Cancel the menu again and navigate to Brave.com. 6. Follow a few links so that you have a history, then hit back button. 7. Confirm you can right click both the back/forward buttons to show the menu 8. Cancel the menu and then click and hold each button to confirm the existing behavior works
1 parent 367898b commit 67ee2c7

File tree

6 files changed

+66
-24
lines changed

6 files changed

+66
-24
lines changed

js/components/longPressButton.js

+16-1
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,24 @@ class LongPressButton extends ImmutableComponent {
1919
if (e.target.attributes.getNamedItem('disabled')) {
2020
return
2121
}
22+
if (e && e.preventDefault) {
23+
e.preventDefault()
24+
}
25+
if (e && e.stopPropagation) {
26+
e.stopPropagation()
27+
}
2228

2329
const self = this
2430
const target = e.target
2531
const LONG_PRESS_MILLISECONDS = 300
2632

33+
// Right click should immediately trigger the action
34+
if (e.button === 2) {
35+
self.props.onLongPress(target)
36+
return
37+
}
38+
39+
// Otherwise, it should wait before triggering
2740
this.longPressTimer = setTimeout(function () {
2841
self.isLocked = true
2942
self.props.onLongPress(target)
@@ -49,7 +62,9 @@ class LongPressButton extends ImmutableComponent {
4962
this.isLocked = false
5063
return
5164
}
52-
this.props.onClick(e)
65+
if (this.props.onClick) {
66+
this.props.onClick(e)
67+
}
5368
}
5469

5570
render () {

js/components/tabs.js

+8-5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ class Tabs extends ImmutableComponent {
2525
this.onDrop = this.onDrop.bind(this)
2626
this.onPrevPage = this.onPrevPage.bind(this)
2727
this.onNextPage = this.onNextPage.bind(this)
28+
this.onNewTabLongPress = this.onNewTabLongPress.bind(this)
29+
this.wasNewTabClicked = this.wasNewTabClicked.bind(this)
2830
}
2931

3032
onPrevPage () {
@@ -83,15 +85,15 @@ class Tabs extends ImmutableComponent {
8385
e.preventDefault()
8486
}
8587
}
86-
88+
wasNewTabClicked (target) {
89+
return target.className === this.refs.newTabButton.props.className
90+
}
8791
newTab () {
8892
windowActions.newFrame()
8993
}
90-
91-
onNewTabLongPress (rect) {
92-
contextMenus.onNewTabContextMenu(rect)
94+
onNewTabLongPress (target) {
95+
contextMenus.onNewTabContextMenu(target)
9396
}
94-
9597
render () {
9698
this.tabRefs = []
9799
const index = this.props.previewTabPageIndex !== undefined
@@ -132,6 +134,7 @@ class Tabs extends ImmutableComponent {
132134
})()}
133135

134136
<LongPressButton
137+
ref='newTabButton'
135138
label='+'
136139
l10nId='newTabButton'
137140
className='browserButton navbutton newFrameButton'

js/components/tabsToolbar.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ class TabsToolbar extends ImmutableComponent {
2626
}
2727

2828
onContextMenu (e) {
29+
if (this.refs.tabs.wasNewTabClicked(e.target)) {
30+
// Don't show the tabs menu if the new tab "+"" was clicked
31+
return
32+
}
2933
contextMenus.onTabsToolbarContextMenu(windowStore.getFrame(this.props.activeFrameKey), undefined, undefined, e)
3034
}
3135

@@ -51,7 +55,9 @@ class TabsToolbar extends ImmutableComponent {
5155
/>
5256
: null
5357
}
54-
<Tabs tabs={unpinnedTabs}
58+
<Tabs
59+
ref='tabs'
60+
tabs={unpinnedTabs}
5561
shouldAllowWindowDrag={this.props.shouldAllowWindowDrag}
5662
draggingOverData={this.props.draggingOverData}
5763
paintTabs={this.props.paintTabs}

js/contextMenus.js

+7-11
Original file line numberDiff line numberDiff line change
@@ -611,15 +611,6 @@ function getMisspelledSuggestions (selection, isMisspelled, suggestions) {
611611
return items
612612
}
613613

614-
function newTabMenuTemplateInit (location, e) {
615-
const template = [
616-
CommonMenu.newPrivateTabMenuItem(),
617-
CommonMenu.newPartitionedTabMenuItem(),
618-
CommonMenu.newWindowMenuItem()
619-
]
620-
return template
621-
}
622-
623614
function getEditableItems (selection, editFlags) {
624615
const hasSelection = selection.length > 0
625616
const hasClipboard = clipboard.readText().length > 0
@@ -1257,8 +1248,13 @@ function onTabContextMenu (frameProps, e) {
12571248
tabMenu.destroy()
12581249
}
12591250

1260-
function onNewTabContextMenu (rect) {
1261-
const menuTemplate = newTabMenuTemplateInit(rect)
1251+
function onNewTabContextMenu (target) {
1252+
const rect = target.getBoundingClientRect()
1253+
const menuTemplate = [
1254+
CommonMenu.newPrivateTabMenuItem(),
1255+
CommonMenu.newPartitionedTabMenuItem(),
1256+
CommonMenu.newWindowMenuItem()
1257+
]
12621258

12631259
windowActions.setContextMenuDetail(Immutable.fromJS({
12641260
left: rect.left,

test/about/braveTest.js

-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ describe('about:brave tests', function () {
2323
.waitUntil(function () {
2424
return this.getText('table.sortableTable td[data-sort="Brave"]')
2525
.then((textValue) => {
26-
console.log(textValue)
2726
return textValue && String(textValue).length > 0 && String(textValue) !== 'null'
2827
})
2928
})
@@ -33,7 +32,6 @@ describe('about:brave tests', function () {
3332
.waitUntil(function () {
3433
return this.getText('table.sortableTable td[data-sort="Muon"]')
3534
.then((textValue) => {
36-
console.log(textValue)
3735
return textValue && String(textValue).length > 0 && String(textValue) !== 'null'
3836
})
3937
})
@@ -43,7 +41,6 @@ describe('about:brave tests', function () {
4341
.waitUntil(function () {
4442
return this.getText('table.sortableTable td[data-sort="Update Channel"]')
4543
.then((textValue) => {
46-
console.log(textValue)
4744
return textValue && String(textValue).length > 0 && String(textValue) !== 'null'
4845
})
4946
})

test/components/tabTest.js

+28-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
/* global describe, it, before */
1+
/* global describe, it, before, beforeEach */
22

33
const Brave = require('../lib/brave')
44
const messages = require('../../js/constants/messages')
55
const settings = require('../../js/constants/settings')
6-
const {urlInput, backButton, forwardButton, activeTabTitle} = require('../lib/selectors')
6+
const {urlInput, backButton, forwardButton, activeTabTitle, newFrameButton} = require('../lib/selectors')
77

8-
describe('tabs', function () {
8+
describe('tab tests', function () {
99
function * setup (client) {
1010
yield client
1111
.waitForUrl(Brave.newTabUrl)
@@ -73,6 +73,31 @@ describe('tabs', function () {
7373
})
7474
})
7575

76+
describe('new tab button', function () {
77+
Brave.beforeEach(this)
78+
beforeEach(function * () {
79+
yield setup(this.app.client)
80+
})
81+
82+
it('creates a new tab when clicked', function * () {
83+
yield this.app.client
84+
.click(newFrameButton)
85+
.waitForExist('.tab[data-frame-key="2"]')
86+
})
87+
it('shows a context menu when long pressed (click and hold)', function * () {
88+
yield this.app.client
89+
.moveToObject(newFrameButton)
90+
.buttonDown(0)
91+
.waitForExist('.contextMenu .contextMenuItem .contextMenuItemText')
92+
.buttonUp(0)
93+
})
94+
it('shows a context menu when right clicked', function * () {
95+
yield this.app.client
96+
.rightClick(newFrameButton)
97+
.waitForExist('.contextMenu .contextMenuItem .contextMenuItemText')
98+
})
99+
})
100+
76101
describe('tab order', function () {
77102
describe('sequentially by default', function () {
78103
Brave.beforeAll(this)

0 commit comments

Comments
 (0)