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

Commit

Permalink
Rename tab close by mouse action to be more explicit
Browse files Browse the repository at this point in the history
The old name wouldn't be invoked for all types of tab closes, so it was a misleading name for future
people that write code. They might have used that event thinking they'd get all tab closes.

Auditors: @bsclifton

Related to #6088
  • Loading branch information
bbondy committed Jan 28, 2017
1 parent 897cebb commit 4a907a8
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 13 deletions.
4 changes: 2 additions & 2 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1201,9 +1201,9 @@ const windowActions = {
})
},

onTabClose: function (data) {
onTabClosedWithMouse: function (data) {
dispatch({
actionType: windowConstants.WINDOW_TAB_CLOSE,
actionType: windowConstants.WINDOW_TAB_CLOSED_WITH_MOUSE,
data
})
},
Expand Down
8 changes: 4 additions & 4 deletions js/components/tab.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ class Tab extends ImmutableComponent {
windowActions.setActiveFrame(this.frame)
}

onCloseFrame (event) {
onTabClosedWithMouse (event) {
event.stopPropagation()
this.props.onFrameClose(this.tabNode.parentNode.getBoundingClientRect())
this.props.onTabClosedWithMouse(this.tabNode.parentNode.getBoundingClientRect())
windowActions.closeFrame(windowStore.getFrames(), this.frame)
}

Expand Down Expand Up @@ -142,7 +142,7 @@ class Tab extends ImmutableComponent {
onClickTab (e) {
// Middle click should close tab
if (e.button === 1) {
this.onCloseFrame(e)
this.onTabClosedWithMouse(e)
} else {
this.setActiveFrame(e)
}
Expand Down Expand Up @@ -256,7 +256,7 @@ class Tab extends ImmutableComponent {
}
{
!this.isPinned
? <span onClick={this.onCloseFrame.bind(this)}
? <span onClick={this.onTabClosedWithMouse.bind(this)}
data-l10n-id='closeTabButton'
className='closeTab fa fa-times-circle' />
: null
Expand Down
8 changes: 4 additions & 4 deletions js/components/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Tabs extends ImmutableComponent {
this.onNewTabLongPress = this.onNewTabLongPress.bind(this)
this.wasNewTabClicked = this.wasNewTabClicked.bind(this)
this.onMouseLeave = this.onMouseLeave.bind(this)
this.onFrameClose = this.onFrameClose.bind(this)
this.onTabClosedWithMouse = this.onTabClosedWithMouse.bind(this)
}

onMouseLeave () {
Expand All @@ -37,8 +37,8 @@ class Tabs extends ImmutableComponent {
})
}

onFrameClose (rect) {
windowActions.onTabClose({
onTabClosedWithMouse (rect) {
windowActions.onTabClosedWithMouse({
fixTabWidth: rect.width
})
}
Expand Down Expand Up @@ -140,7 +140,7 @@ class Tabs extends ImmutableComponent {
paintTabs={this.props.paintTabs}
previewTabs={this.props.previewTabs}
isActive={this.props.activeFrameKey === tab.get('frameKey')}
onFrameClose={this.onFrameClose}
onTabClosedWithMouse={this.onTabClosedWithMouse}
tabWidth={this.props.fixTabWidth}
partOfFullPageSet={this.props.partOfFullPageSet} />)
}
Expand Down
2 changes: 1 addition & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const windowConstants = {
WINDOW_WIDEVINE_PANEL_DETAIL_CHANGED: _,
WINDOW_AUTOFILL_SELECTION_CLICKED: _,
WINDOW_AUTOFILL_POPUP_HIDDEN: _,
WINDOW_TAB_CLOSE: _,
WINDOW_TAB_CLOSED_WITH_MOUSE: _,
WINDOW_TAB_MOUSE_LEAVE: _
}

Expand Down
2 changes: 1 addition & 1 deletion js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ const doAction = (action) => {
case appConstants.APP_NEW_TAB:
newFrame(action.frameProps, action.frameProps.get('disposition') === 'foreground-tab')
break
case windowConstants.WINDOW_TAB_CLOSE:
case windowConstants.WINDOW_TAB_CLOSED_WITH_MOUSE:
if (frameStateUtil.getNonPinnedFrameCount(windowState) % getSetting(settings.TABS_PER_PAGE) === 0) {
windowState = windowState.deleteIn(['ui', 'tabs', 'fixTabWidth'])
} else {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/state/frameStateUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ describe('frameStateUtil', function () {
})
it('returns 0 with only pinned frames', function () {
this.windowState = Immutable.fromJS(Object.assign({}, defaultWindowStore.toJS()))
assert.equal(frameStateUtil.getNonPinnedFrameCount(Immutable.fromJS({frames: [{title:'153,409th prime', pinnedLocation: 'http://www.brave.com/2064737'}]})), 0)
assert.equal(frameStateUtil.getNonPinnedFrameCount(Immutable.fromJS({frames: [{title: '153,409th prime', pinnedLocation: 'http://www.brave.com/2064737'}]})), 0)
})
it('returns 1 with a frame and a pinned frames', function () {
this.windowState = Immutable.fromJS(Object.assign({}, defaultWindowStore.toJS()))
Expand Down

1 comment on commit 4a907a8

@bsclifton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix! I didn't even consider closing via keyboard shortcuts 😄
cc: @gyandeeps

Please sign in to comment.