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

Commit

Permalink
make rule for secondary icons consistent
Browse files Browse the repository at this point in the history
- Auditors: @bsclifton
- Close #8158

Test Plan covered by automated tests:

- tabContent components NewSessionIcon
- tabContent components PrivateIcon
  • Loading branch information
cezaraugusto committed Apr 10, 2017
1 parent 86eacb6 commit 1d1c99e
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 16 deletions.
18 changes: 4 additions & 14 deletions app/renderer/components/tabContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ const globalStyles = require('./styles/global')
const {isWindows} = require('../../common/lib/platformUtil')
const {getTextColorForBackground} = require('../../../js/lib/color')
const {tabs} = require('../../../js/constants/config')
const {hasBreakpoint, hasRelativeCloseIcon, hasFixedCloseIcon} = require('../lib/tabUtil')

const {hasBreakpoint, hasRelativeCloseIcon,
hasFixedCloseIcon, hasVisibleSecondaryIcon} = require('../lib/tabUtil')
const newSessionSvg = require('../../extensions/brave/img/tabs/new_session.svg')
const privateSvg = require('../../extensions/brave/img/tabs/private.svg')
const closeTabSvg = require('../../extensions/brave/img/tabs/close_btn_normal.svg')
Expand Down Expand Up @@ -133,31 +133,21 @@ class AudioTabIcon extends ImmutableComponent {
}

class PrivateIcon extends ImmutableComponent {
get narrowView () {
const sizes = ['small', 'extraSmall', 'smallest']
return sizes.includes(this.props.tab.get('breakpoint'))
}

render () {
const privateStyles = StyleSheet.create({
icon: {
WebkitMaskImage: `url(${privateSvg})`,
backgroundColor: this.props.isActive ? globalStyles.color.white100 : globalStyles.color.black100
}
})
return this.props.tab.get('isPrivate') && !this.props.tab.get('hoverState') && !this.narrowView
return this.props.tab.get('isPrivate') && hasVisibleSecondaryIcon(this.props)
? <TabIcon data-test-id='privateIcon'
className={css(styles.icon, styles.secondaryIcon, privateStyles.icon)} />
: null
}
}

class NewSessionIcon extends ImmutableComponent {
get narrowView () {
const sizes = ['small', 'extraSmall', 'smallest']
return sizes.includes(this.props.tab.get('breakpoint'))
}

get partitionNumber () {
let partition = this.props.tab.get('partitionNumber')
// Persistent partitions opened by `target="_blank"` will have
Expand Down Expand Up @@ -190,7 +180,7 @@ class NewSessionIcon extends ImmutableComponent {
}
})

return this.partitionNumber && !this.props.tab.get('hoverState') && !this.narrowView
return this.partitionNumber && hasVisibleSecondaryIcon(this.props)
? <TabIcon symbol
data-test-id='newSessionIcon'
className={css(styles.icon, styles.newSession, newSession.indicator)}
Expand Down
10 changes: 10 additions & 0 deletions app/renderer/lib/tabUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ module.exports.hasRelativeCloseIcon = (props) => {
!module.exports.hasBreakpoint(props, ['small', 'extraSmall', 'smallest'])
}

/**
* Check whether or not private or newSession icon should be visible
* @param {Object} props - Object that hosts the tab props
* @returns {Boolean} Whether or not private or newSession icon should be visible
*/
module.exports.hasVisibleSecondaryIcon = (props) => {
return !props.tab.get('hoverState') &&
!module.exports.hasBreakpoint(props, ['small', 'extraSmall', 'smallest'])
}

/**
* Check whether or not closeTab icon is always visible (fixed) in tab
* @param {Object} props - Object that hosts the tab props
Expand Down
56 changes: 54 additions & 2 deletions test/unit/app/renderer/tabContentTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('tabContent components', function () {
)
assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon')
})
it('should not show private icon if tab size is too small', function () {
it('should not show private icon if tab size is small', function () {
const wrapper = shallow(
<PrivateIcon
tab={
Expand All @@ -206,6 +206,32 @@ describe('tabContent components', function () {
)
assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon')
})
it('should not show private icon if tab size is extraSmall', function () {
const wrapper = shallow(
<PrivateIcon
tab={
Immutable.Map({
isPrivate: true,
hoverState: false,
breakpoint: 'extraSmall'
})}
/>
)
assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon')
})
it('should not show private icon if tab size is the smallest', function () {
const wrapper = shallow(
<PrivateIcon
tab={
Immutable.Map({
isPrivate: true,
hoverState: false,
breakpoint: 'smallest'
})}
/>
)
assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon')
})
})

describe('NewSessionIcon', function () {
Expand Down Expand Up @@ -243,7 +269,7 @@ describe('tabContent components', function () {
)
assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
})
it('should not show new session icon if tab size is too small', function () {
it('should not show new session icon if tab size is small', function () {
const wrapper = shallow(
<NewSessionIcon
tab={
Expand All @@ -256,6 +282,32 @@ describe('tabContent components', function () {
)
assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
})
it('should not show new session icon if tab size is extraSmall', function () {
const wrapper = shallow(
<NewSessionIcon
tab={
Immutable.Map({
partitionNumber: 1,
hoverState: true,
breakpoint: 'extraSmall'
})}
/>
)
assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
})
it('should not show new session icon if tab size is the smallest', function () {
const wrapper = shallow(
<NewSessionIcon
tab={
Immutable.Map({
partitionNumber: 1,
hoverState: true,
breakpoint: 'smallest'
})}
/>
)
assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
})
it('should show partition number for new sessions', function () {
const wrapper = shallow(
<NewSessionIcon
Expand Down

1 comment on commit 1d1c99e

@bsclifton
Copy link
Member

Choose a reason for hiding this comment

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

++

Please sign in to comment.