fix(plugins/plugin-client-common): top tab buttons should be owned by the top tab stripe

they had been owned and rendered (with position: absolute) by TabContent. this was ugly. worse, any clients that had their own top stripe would find kui elements rudely interjected.

Fixes #4690

Author: starpit

Diff for: plugins/plugin-client-common/src/components/Client/TabContainer.tsx

@@ -17,7 +17,7 @@
 import * as React from 'react'
 import { inElectron, Tab, eventBus } from '@kui-shell/core'
 
-import TabModel from './TabModel'
+import TabModel, { TopTabButton } from './TabModel'
 import TabContent, { TabContentOptions } from './TabContent'
 import TopTabStripe, { TopTabStripeConfiguration } from './TopTabStripe'
 
@@ -202,6 +202,20 @@ export default class TabContainer extends React.PureComponent<Props, State> {
     }
   }
 
+  private willUpdateTopTabButtons(uuid: string, buttons: TopTabButton[]) {
+    this.setState(curState => {
+      const idx = curState.tabs.findIndex(_ => _.uuid === uuid)
+      if (idx >= 0) {
+        return {
+          tabs: curState.tabs
+            .slice(0, idx)
+            .concat([curState.tabs[idx].update(buttons)])
+            .concat(curState.tabs.slice(idx + 1))
+        }
+      }
+    })
+  }
+
   public render() {
     return (
       <div className="kui--full-height">
@@ -216,7 +230,14 @@ export default class TabContainer extends React.PureComponent<Props, State> {
         {this.search()}
         <div className="tab-container">
           {this.state.tabs.map((_, idx) => (
-            <TabContent key={idx} uuid={_.uuid} active={idx === this.state.activeIdx} state={_.state} {...this.props}>
+            <TabContent
+              key={idx}
+              uuid={_.uuid}
+              active={idx === this.state.activeIdx}
+              willUpdateTopTabButtons={this.willUpdateTopTabButtons.bind(this, _.uuid)}
+              state={_.state}
+              {...this.props}
+            >
               {this.children(_.uuid)}
             </TabContent>
           ))}

Diff for: plugins/plugin-client-common/src/components/Client/TabContent.tsx

@@ -22,6 +22,7 @@ import Icons from '../spi/Icons'
 import KuiContext from './context'
 import Confirm from '../Views/Confirm'
 import Loading from '../spi/Loading'
+import { TopTabButton } from './TabModel'
 import Width from '../Views/Sidecar/width'
 import WatchPane, { Height } from '../Views/WatchPane'
 
@@ -53,6 +54,7 @@ type Props = TabContentOptions &
     active: boolean
     state: TabState
     onTabReady?: (tab: KuiTab) => void
+    willUpdateTopTabButtons?: (buttons: TopTabButton[]) => void
   }
 
 type CurrentlyShowing = 'TerminalOnly' | 'TerminalPlusSidecar' | 'TerminalPlusWatcher' | 'TerminalSidecarWatcher'
@@ -274,7 +276,7 @@ export default class TabContent extends React.PureComponent<Props, State> {
       const sidecarWidth = desiredWidth
       const watchPaneOpen = curState.primaryHeight === Height.Split
 
-      const activeView =
+      const activeView: CurrentlyShowing =
         sidecarWidth === Width.Closed
           ? watchPaneOpen
             ? 'TerminalPlusWatcher'
@@ -283,12 +285,15 @@ export default class TabContent extends React.PureComponent<Props, State> {
           ? 'TerminalSidecarWatcher'
           : 'TerminalPlusSidecar'
 
-      return {
+      const newState = {
         sidecarHasContent: true,
         sidecarWidth,
         priorSidecarWidth: curState.sidecarWidth,
         activeView
       }
+
+      this.updateTopTabButtons(newState)
+      return newState
     })
   }
 
@@ -300,10 +305,26 @@ export default class TabContent extends React.PureComponent<Props, State> {
       const sidecarWidth = showSidecar ? Width.Split60 : Width.Closed
       const primaryHeight = showWatchPane ? Height.Split : Height.NotSplit
 
-      return { sidecarWidth, activeView, priorSidecarWidth: curState.sidecarWidth, primaryHeight }
+      const newState = {
+        sidecarWidth,
+        activeView,
+        priorSidecarWidth: curState.sidecarWidth,
+        primaryHeight,
+        sidecarHasContent: curState.sidecarHasContent
+      }
+
+      this.updateTopTabButtons(newState)
+      return newState
     })
   }
 
+  /** Switch to the given view, if we aren't already there */
+  private showIfNot(desiredView: CurrentlyShowing) {
+    if (this.state.activeView !== desiredView) {
+      this.show(desiredView)
+    }
+  }
+
   private openWatchPane() {
     const notWatching = this.state.activeView === 'TerminalOnly' || this.state.activeView === 'TerminalPlusSidecar'
 
@@ -444,8 +465,6 @@ export default class TabContent extends React.PureComponent<Props, State> {
           </div>
           {this.state.tab && <Confirm tab={this.state.tab} uuid={this.props.uuid} />}
         </div>
-
-        {this.topTabButtons()}
       </React.Fragment>
     )
   }
@@ -505,62 +524,56 @@ export default class TabContent extends React.PureComponent<Props, State> {
   }
 
   /**
-   * Buttons that are placed in the TopTabStripe and which controller
-   * the visibility of various views.
+   * If given, use the top tab button controller to provide the
+   * latest button model.
+   *
    */
-  protected topTabButtons() {
-    if (this.props.active && (this.state.sidecarHasContent || this.state.watchPaneHasContent)) {
-      const buttonNum = this.state.sidecarHasContent && this.state.watchPaneHasContent ? 4 : 2
+  private updateTopTabButtons(newState: Pick<State, 'activeView' | 'sidecarHasContent'>) {
+    if (this.props.willUpdateTopTabButtons) {
+      this.props.willUpdateTopTabButtons([this.terminalButton(newState), this.sidecarButton(newState)].filter(_ => _))
+    }
+  }
 
-      return (
-        <div
-          id="kui--custom-top-tab-stripe-button-container"
-          num-button={buttonNum} // helps with css to calculate the right position of the container
-          className="kui--hide-in-narrower-windows" // re: kui--hide-in-narrower-windows, see https://github.com/IBM/kui/issues/4459
-        >
+  /**
+   * Note how we use the argument `state` to initialize things, but we
+   * intentionally use this.state in the onClick. The onClick handler
+   * may be invoked after any number of onClicks in this or other
+   * buttons. So it must pay attention to the *current* state, not the
+   * state at the time of creation.
+   *
+   */
+  private terminalButton(state: Pick<State, 'activeView'>): TopTabButton {
+    const key = 'show only terminal'
+
+    return {
+      icon: (
+        <Icons
+          icon="TerminalOnly"
+          key={key}
+          data-mode={key}
+          data-active={state.activeView === 'TerminalOnly' || undefined}
+          onClick={this.showIfNot.bind(this, 'TerminalOnly')}
+        />
+      )
+    }
+  }
+
+  /** Caution: see the Note for this.terminalButton, re: `state` versus `this.state` */
+  private sidecarButton(state: Pick<State, 'activeView' | 'sidecarHasContent'>): TopTabButton {
+    if (state.sidecarHasContent) {
+      const key = 'show terminal and sidecar'
+
+      return {
+        icon: (
           <Icons
-            icon="TerminalOnly"
-            data-mode="show only terminal"
-            data-active={this.state.activeView === 'TerminalOnly' || undefined}
-            onClick={this.state.activeView !== 'TerminalOnly' ? () => this.show('TerminalOnly') : undefined}
+            icon="TerminalPlusSidecar"
+            key={key}
+            data-mode={key}
+            data-active={state.activeView === 'TerminalPlusSidecar' || undefined}
+            onClick={this.showIfNot.bind(this, 'TerminalPlusSidecar')}
           />
-
-          {this.state.sidecarHasContent && (
-            <Icons
-              icon="TerminalPlusSidecar"
-              data-mode="show terminal and sidecar"
-              data-active={this.state.activeView === 'TerminalPlusSidecar' || undefined}
-              onClick={
-                this.state.activeView !== 'TerminalPlusSidecar' ? () => this.show('TerminalPlusSidecar') : undefined
-              }
-            />
-          )}
-
-          {this.state.watchPaneHasContent && (
-            <Icons
-              icon="TerminalPlusWatcher"
-              data-mode="show terminal and watcher"
-              data-active={this.state.activeView === 'TerminalPlusWatcher' || undefined}
-              onClick={
-                this.state.activeView !== 'TerminalPlusWatcher' ? () => this.show('TerminalPlusWatcher') : undefined
-              }
-            />
-          )}
-
-          {this.state.watchPaneHasContent && this.state.sidecarHasContent && (
-            <Icons
-              icon="TerminalSidecarWatcher"
-              data-mode="show terminal sidecar and watcher"
-              data-active={this.state.activeView === 'TerminalSidecarWatcher' || undefined}
-              onClick={
-                this.state.activeView !== 'TerminalSidecarWatcher'
-                  ? () => this.show('TerminalSidecarWatcher')
-                  : undefined
-              }
-            />
-          )}
-        </div>
-      )
+        )
+      }
     }
   }
 }

Diff for: plugins/plugin-client-common/src/components/Client/TabModel.ts

@@ -22,13 +22,16 @@ export function uuid() {
   return (_uuidCounter++).toString()
 }
 
-export default class TabModel {
-  private readonly _uuid: string
-  private readonly _state: TabState
+export interface TopTabButton<P extends { key: string } = { key: string }> {
+  icon: React.ReactElement<P>
+}
 
-  public constructor() {
-    this._uuid = uuid()
-    this._state = new TabState(this._uuid)
+export default class TabModel {
+  public constructor(
+    private readonly _uuid = uuid(),
+    private readonly _state = new TabState(_uuid),
+    private readonly _buttons: TopTabButton[] = []
+  ) {
     this._state.capture()
   }
 
@@ -39,4 +42,12 @@ export default class TabModel {
   public get state() {
     return this._state
   }
+
+  public get buttons() {
+    return this._buttons
+  }
+
+  public update(buttons: TopTabButton[]) {
+    return new TabModel(this.uuid, this.state, buttons)
+  }
 }

Diff for: plugins/plugin-client-common/src/components/Client/TopTabStripe/index.tsx

@@ -163,6 +163,22 @@ export default class TopTabStripe extends React.PureComponent<Props> {
     )
   }
 
+  /** Buttons that appear in the top right */
+  private buttons() {
+    if (this.props.tabs[this.props.activeIdx]) {
+      const { buttons } = this.props.tabs[this.props.activeIdx]
+      return (
+        <div
+          id="kui--custom-top-tab-stripe-button-container"
+          num-button={buttons.length} // helps with css to calculate the right position of the container
+          className="kui--hide-in-narrower-windows" // re: kui--hide-in-narrower-windows, see https://github.com/IBM/kui/issues/4459
+        >
+          {buttons.map(_ => _.icon)}
+        </div>
+      )
+    }
+  }
+
   /**
    * React render handler
    *
@@ -173,6 +189,7 @@ export default class TopTabStripe extends React.PureComponent<Props> {
         {/* this.headerMenu(args) */}
         {this.headerName()}
         {this.tabs()}
+        {this.buttons()}
         {/* this.sidenav(args) */}
       </Header>
     )

Diff for: plugins/plugin-client-common/web/css/static/TopTabStripe.scss

@@ -218,8 +218,7 @@
 #kui--custom-top-tab-stripe-button-container {
   display: flex;
   justify-content: flex-end;
-  position: absolute;
-  z-index: 10000;
+  position: relative;
 
   &[num-button='2'] {
     /* number of buttons in the container */