Update min-threshold for closing a tab

app/renderer/components/tabs/content/tabTitle.js

@@ -9,26 +9,25 @@ 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 () {
     return !!this.props.tab.get('pinnedLocation')
   }
 
   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 () {

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')
+  )
 }
 
 /**

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(
       <CloseTabIcon
         tab={
           Immutable.Map({
-            hoverState: true
+            hoverState: true,
+            breakpoint: 'default'
           })}
       />
     )
     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(
       <CloseTabIcon
         tab={
           Immutable.Map({
-            hoverState: false
+            hoverState: true,
+            breakpoint: 'large'
           })}
       />
     )
-    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(
+      <CloseTabIcon isActive
+        tab={
+          Immutable.Map({
+            hoverState: false,
+            breakpoint: 'largeMedium'
+          })}
+      />
+    )
+    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(
+      <CloseTabIcon isActive={false}
+        tab={
+          Immutable.Map({
+            hoverState: true,
+            breakpoint: 'largeMedium'
+          })}
+      />
+    )
+    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(
+      <CloseTabIcon isActive
+        tab={
+          Immutable.Map({
+            hoverState: false,
+            breakpoint: 'medium'
+          })}
+      />
+    )
+    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(
+      <CloseTabIcon isActive={false}
+        tab={
+          Immutable.Map({
+            hoverState: true,
+            breakpoint: 'medium'
+          })}
+      />
+    )
+    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(
+      <CloseTabIcon isActive
+        tab={
+          Immutable.Map({
+            hoverState: false,
+            breakpoint: 'mediumSmall'
+          })}
+      />
+    )
+    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(
+      <CloseTabIcon isActive={false}
+        tab={
+          Immutable.Map({
+            hoverState: true,
+            breakpoint: 'mediumSmall'
+          })}
+      />
+    )
+    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(
       <CloseTabIcon isActive

test/unit/app/renderer/components/tabs/content/newSessionIconTest.js

@@ -48,34 +48,139 @@ describe('Tabs content - NewSessionIcon', function () {
     )
     assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
   })
-  it('should not show new session icon if mouse is over tab (avoid icon overflow)', function () {
+  it('should not show new session icon if mouse is over tab and breakpoint is default', function () {
     const wrapper = shallow(
       <NewSessionIcon
         tab={
           Immutable.Map({
             partitionNumber: 1,
-            hoverState: true
+            hoverState: true,
+            breakpoint: 'default'
           })}
       />
     )
     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(
+      <NewSessionIcon
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: false,
+            breakpoint: 'default'
+          })}
+      />
+    )
+    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(
       <NewSessionIcon
         tab={
           Immutable.Map({
             partitionNumber: 1,
             hoverState: true,
-            breakpoint: 'small'
+            breakpoint: 'large'
           })}
       />
     )
     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(
       <NewSessionIcon
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: false,
+            breakpoint: 'large'
+          })}
+      />
+    )
+    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(
+      <NewSessionIcon isActive
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: true,
+            breakpoint: 'largeMedium'
+          })}
+      />
+    )
+    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(
+      <NewSessionIcon isActive={false}
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: false,
+            breakpoint: 'largeMedium'
+          })}
+      />
+    )
+    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(
+      <NewSessionIcon isActive
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: true,
+            breakpoint: 'medium'
+          })}
+      />
+    )
+    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(
+      <NewSessionIcon isActive={false}
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: false,
+            breakpoint: 'medium'
+          })}
+      />
+    )
+    assert.equal(wrapper.props()['data-test-id'], 'newSessionIcon')
+  })
+  it('should not show new session icon if breakpoint is mediumSmall', function () {
+    const wrapper = shallow(
+      <NewSessionIcon isActive
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: false,
+            breakpoint: 'mediumSmall'
+          })}
+      />
+    )
+    assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
+  })
+  it('should not show new session icon if breakpoint is small', function () {
+    const wrapper = shallow(
+      <NewSessionIcon isActive
+        tab={
+          Immutable.Map({
+            partitionNumber: 1,
+            hoverState: true,
+            breakpoint: 'small'
+          })}
+      />
+    )
+    assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
+  })
+  it('should not show new session icon if breakpoint is extraSmall', function () {
+    const wrapper = shallow(
+      <NewSessionIcon isActive
         tab={
           Immutable.Map({
             partitionNumber: 1,
@@ -86,7 +191,7 @@ describe('Tabs content - NewSessionIcon', function () {
     )
     assert.notEqual(wrapper.props()['data-test-id'], 'newSessionIcon')
   })
-  it('should not show new session icon if tab size is the smallest', function () {
+  it('should not show new session icon if breakpoint is the smallest', function () {
     const wrapper = shallow(
       <NewSessionIcon
         tab={