From e6b76bb21c050b443d958475f122d2303fd0750a Mon Sep 17 00:00:00 2001 From: Cezar Augusto Date: Thu, 20 Apr 2017 07:59:18 -0300 Subject: [PATCH 1/2] Update min-threshold for closing a tab - Auditors: @bsclifton, @nejczdovc - Close #8130 - Test Plan: covered by automated tests --- .../components/tabs/content/tabTitle.js | 13 ++++++------ app/renderer/lib/tabUtil.js | 20 +++++++++++++++---- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/app/renderer/components/tabs/content/tabTitle.js b/app/renderer/components/tabs/content/tabTitle.js index 7239170abe5..4af91b649bb 100644 --- a/app/renderer/components/tabs/content/tabTitle.js +++ b/app/renderer/components/tabs/content/tabTitle.js @@ -9,15 +9,16 @@ const {StyleSheet, css} = require('aphrodite/no-important') const ImmutableComponent = require('../../immutableComponent') // Utils -const {hasBreakpoint, hasFixedCloseIcon, getTabIconColor} = require('../../../lib/tabUtil') +const {hasBreakpoint, getTabIconColor} = require('../../../lib/tabUtil') const {isWindows, isDarwin} = require('../../../../common/lib/platformUtil') // Styles const globalStyles = require('../../styles/global') class TabTitle extends ImmutableComponent { - get locationHasSecondaryIcon () { - return !!this.props.tab.get('isPrivate') || !!this.props.tab.get('partitionNumber') + get isActiveOrHasSecondaryIcon () { + return this.props.isActive || + (!!this.props.tab.get('isPrivate') || !!this.props.tab.get('partitionNumber')) } get isPinned () { @@ -25,10 +26,8 @@ class TabTitle extends ImmutableComponent { } get shouldHideTitle () { - return (this.props.tab.get('breakpoint') === 'mediumSmall' && this.locationHasSecondaryIcon) || - (hasBreakpoint(this.props, 'mediumSmall') && this.props.tab.get('hoverState')) || - hasBreakpoint(this.props, ['extraSmall', 'smallest']) || - hasFixedCloseIcon(this.props) + return (hasBreakpoint(this.props, 'small') && this.props.isActive) || + hasBreakpoint(this.props, ['extraSmall', 'smallest']) } render () { diff --git a/app/renderer/lib/tabUtil.js b/app/renderer/lib/tabUtil.js index 47d3e3cfbf8..e8198301e47 100644 --- a/app/renderer/lib/tabUtil.js +++ b/app/renderer/lib/tabUtil.js @@ -48,7 +48,7 @@ module.exports.hasBreakpoint = (props, arr) => { */ module.exports.hasRelativeCloseIcon = (props) => { return props.tab.get('hoverState') && - !module.exports.hasBreakpoint(props, ['small', 'extraSmall', 'smallest']) + module.exports.hasBreakpoint(props, ['default', 'large']) } /** @@ -57,8 +57,14 @@ module.exports.hasRelativeCloseIcon = (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']) + return ( + // Hide icon on hover + !module.exports.hasRelativeCloseIcon(props) && + // If closeIcon is fixed then there's no room for another icon + !module.exports.hasFixedCloseIcon(props) && + // completely hide it for small sizes + !module.exports.hasBreakpoint(props, ['mediumSmall', 'small', 'extraSmall', 'smallest']) + ) } /** @@ -67,7 +73,13 @@ module.exports.hasVisibleSecondaryIcon = (props) => { * @returns {Boolean} Whether or not the close icon is always visible (fixed) */ module.exports.hasFixedCloseIcon = (props) => { - return props.isActive && module.exports.hasBreakpoint(props, ['small', 'extraSmall']) + return ( + props.isActive && + // larger sizes still have a relative closeIcon + !module.exports.hasBreakpoint(props, ['default', 'large']) && + // We don't resize closeIcon as we do with favicon so don't show it + !module.exports.hasBreakpoint(props, 'smallest') + ) } /** From 94790ed96848319bffb8f2d5c15e310e02fbe24b Mon Sep 17 00:00:00 2001 From: Cezar Augusto Date: Thu, 20 Apr 2017 08:04:24 -0300 Subject: [PATCH 2/2] Add missing breakpoint tests for tab content --- .../tabs/content/closeTabIconTest.js | 86 +++++++++++- .../tabs/content/newSessionIconTest.js | 117 ++++++++++++++++- .../tabs/content/privateIconTest.js | 123 ++++++++++++++++-- .../components/tabs/content/tabTitleTest.js | 92 ++++++++++++- 4 files changed, 393 insertions(+), 25 deletions(-) diff --git a/test/unit/app/renderer/components/tabs/content/closeTabIconTest.js b/test/unit/app/renderer/components/tabs/content/closeTabIconTest.js index e9d77d0043e..4a8c21faee8 100644 --- a/test/unit/app/renderer/components/tabs/content/closeTabIconTest.js +++ b/test/unit/app/renderer/components/tabs/content/closeTabIconTest.js @@ -26,27 +26,29 @@ describe('Tabs content - CloseTabIcon', function () { mockery.disable() }) - it('should show closeTab icon if mouse is over tab', function () { + it('should show closeTab icon if mouse is over tab and breakpoint is default', function () { const wrapper = shallow( ) assert.equal(wrapper.props()['data-test-id'], 'closeTabIcon') }) - it('should not show closeTab icon if mouse is not over a tab', function () { + it('should show closeTab icon if mouse is over tab and breakpoint is large', function () { const wrapper = shallow( ) - assert.notEqual(wrapper.props()['data-test-id'], 'closeTabIcon') + assert.equal(wrapper.props()['data-test-id'], 'closeTabIcon') }) it('should not show closeTab icon if tab is pinned', function () { const wrapper = shallow( @@ -60,6 +62,80 @@ describe('Tabs content - CloseTabIcon', function () { ) assert.notEqual(wrapper.props()['data-test-id'], 'closeTabIcon') }) + it('should show closeTab icon if tab size is largeMedium and tab is active', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'closeTabIcon') + }) + it('should not show closeTab icon if tab size is largeMedium and tab is not active', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'closeTabIcon') + }) + + it('should show closeTab icon if tab size is medium and tab is active', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'closeTabIcon') + }) + it('should not show closeTab icon if tab size is medium and tab is not active', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'closeTabIcon') + }) + + it('should show closeTab icon if tab size is mediumSmall and tab is active', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'closeTabIcon') + }) + it('should not show closeTab icon if tab size is mediumSmall and tab is not active', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'closeTabIcon') + }) it('should show closeTab icon if tab size is small and tab is active', function () { const wrapper = shallow( ) assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') }) - it('should not show new session icon if tab size is small', function () { + it('should show new session icon if mouse is not over tab and breakpoint is default', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if mouse is over tab and breakpoint is large', function () { const wrapper = shallow( ) assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') }) - it('should not show new session icon if tab size is extraSmall', function () { + it('should show new session icon if mouse is not over tab and breakpoint is large', function () { const wrapper = shallow( + ) + assert.equal(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if tab is active and breakpoint is largeMedium', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should show new session icon if tab is not active and breakpoint is largeMedium', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if tab is active and breakpoint is medium', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should show new session icon if tab is not active and breakpoint is medium', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if breakpoint is mediumSmall', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if breakpoint is small', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon') + }) + it('should not show new session icon if breakpoint is extraSmall', function () { + const wrapper = shallow( + ) assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') }) - it('should not show private icon if tab size is small', function () { + it('should show private icon if mouse is not over tab and breakpoint is default', function () { const wrapper = shallow( + ) + assert.equal(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if mouse is over tab and breakpoint is large', function () { + const wrapper = shallow( + ) assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') }) - it('should not show private icon if tab size is extraSmall', function () { + it('should show private icon if mouse is not over tab and breakpoint is large', function () { const wrapper = shallow( + ) + assert.equal(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if tab is active and breakpoint is largeMedium', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should show private icon if tab is not active and breakpoint is largeMedium', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if tab is active and breakpoint is medium', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should show private icon if tab is not active and breakpoint is medium', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if breakpoint is mediumSmall', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if breakpoint is small', function () { + const wrapper = shallow( + + ) + assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') + }) + it('should not show private icon if breakpoint is extraSmall', function () { + const wrapper = shallow( + ) assert.notEqual(wrapper.props()['data-test-id'], 'privateIcon') }) - it('should not show private icon if tab size is the smallest', function () { + it('should not show private icon if breakpoint is the smallest', function () { const wrapper = shallow( diff --git a/test/unit/app/renderer/components/tabs/content/tabTitleTest.js b/test/unit/app/renderer/components/tabs/content/tabTitleTest.js index 915a17b2bc6..b217bb8d4b5 100644 --- a/test/unit/app/renderer/components/tabs/content/tabTitleTest.js +++ b/test/unit/app/renderer/components/tabs/content/tabTitleTest.js @@ -44,23 +44,91 @@ describe('Tabs content - Title', function () { ) assert.notEqual(wrapper.text(), pageTitle1) }) - it('should not show text if size is mediumSmall and location has a secondary icon', function () { + it('should show text if breakpoint is default', function () { const wrapper = shallow( + ) + assert.equal(wrapper.text(), pageTitle1) + }) + it('should show text if breakpoint is large', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.text(), pageTitle1) + }) + it('should show text if breakpoint is medium', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.text(), pageTitle1) + }) + it('should show text if breakpoint is mediumSmall', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.text(), pageTitle1) + }) + it('should show text if breakpoint is small and tab is not active', function () { + const wrapper = shallow( + + ) + assert.equal(wrapper.text(), pageTitle1) + }) + it('should not show text if breakpoint is small and tab is active', function () { + const wrapper = shallow( + ) assert.notEqual(wrapper.text(), pageTitle1) }) - it('should not show text if size is too small', function () { + it('should not show text if breakpoint is extraSmall', function () { const wrapper = shallow( + ) + assert.notEqual(wrapper.text(), pageTitle1) + }) })