fix(plugins/plugin-client-common): improved focus behavior of terminals

1) avoid spurious scrolling: when clicking on a table entry while the active prompt is not in view
2) ibid: when switching tabs while the active prompt is not in view

this PR removes the old hackFocus() hack in TabContainer, which did ugly querySelectors

Fixes #5615

Author: starpit

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

@@ -69,28 +69,6 @@ export default class TabContainer extends React.PureComponent<Props, State> {
     })
   }
 
-  /**
-   * Temporary hack to regrab focus to the repl. The hack part is the
-   * querySelector. This really needs to be done in TabContent, which
-   * owns the Tab impl.
-   *
-   */
-  private hackFocus() {
-    setTimeout(() => {
-      try {
-        const selector = `.kui--tab-content.visible .repl-active[data-is-focused] input`
-        const selector2 = `.kui--tab-content.visible .repl-active input`
-        const input =
-          (document.querySelector(selector) as HTMLElement) || (document.querySelector(selector2) as HTMLElement)
-        if (input) {
-          input.focus()
-        }
-      } catch (err) {
-        console.error(err)
-      }
-    })
-  }
-
   /** save tab state such as CWD prior to a tab switch */
   private captureState() {
     try {
@@ -118,8 +96,6 @@ export default class TabContainer extends React.PureComponent<Props, State> {
       this.setState({
         activeIdx: idx
       })
-
-      this.hackFocus()
     }
 
     setTimeout(() => eventBus.emit('/tab/switch/request/done', idx))
@@ -142,8 +118,6 @@ export default class TabContainer extends React.PureComponent<Props, State> {
         tabs: residualTabs,
         activeIdx
       })
-
-      this.hackFocus()
     }
   }
 

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

@@ -66,6 +66,9 @@ type State = Partial<WithTab> & {
   sidecarHasContent: boolean
 
   activeView: CurrentlyShowing
+
+  /** grab a ref (below) so that we can maintain focus */
+  _terminal: ScrollableTerminal
 }
 
 /**
@@ -88,9 +91,6 @@ export default class TabContent extends React.PureComponent<Props, State> {
   /** switching back or away from this tab */
   private activateHandlers: ((isActive: boolean) => void)[] = []
 
-  /** grab a ref (below) so that we can maintain focus */
-  private _terminal: ScrollableTerminal
-
   public constructor(props: Props) {
     super(props)
 
@@ -101,7 +101,8 @@ export default class TabContent extends React.PureComponent<Props, State> {
       sidecarWidth: Width.Closed,
       priorSidecarWidth: Width.Closed,
       sidecarHasContent: false,
-      activeView: 'TerminalOnly'
+      activeView: 'TerminalOnly',
+      _terminal: undefined
     }
   }
 
@@ -177,6 +178,9 @@ export default class TabContent extends React.PureComponent<Props, State> {
         console.error(err)
       }
     } else {
+      if (props.active && state._terminal) {
+        state._terminal.doFocus()
+      }
       return state
     }
   }
@@ -225,9 +229,9 @@ export default class TabContent extends React.PureComponent<Props, State> {
                   // eslint-disable-next-line react/no-direct-mutation-state
                   this.state.tab.state.desiredStatusStripeDecoration = { type: 'default' }
                 }}
-                ref={c => {
+                ref={_terminal => {
                   // so that we can refocus/blur
-                  this._terminal = c
+                  this.setState({ _terminal })
                 }}
               >
                 {this.children()}
@@ -284,8 +288,8 @@ export default class TabContent extends React.PureComponent<Props, State> {
   }
 
   private onWillLoseFocus() {
-    if (this._terminal) {
-      this._terminal.doFocus()
+    if (this.state._terminal) {
+      this.state._terminal.doFocus()
     }
   }
 

plugins/plugin-client-common/src/components/Views/Terminal/Block/Input.tsx

@@ -23,6 +23,7 @@ import Actions from './Actions'
 import onPaste from './OnPaste'
 import onKeyDown from './OnKeyDown'
 import onKeyPress from './OnKeyPress'
+import isInViewport from '../visible'
 import KuiContext from '../../../Client/context'
 import { TabCompletionState } from './TabCompletion'
 import ActiveISearch, { onKeyUp } from './ActiveISearch'
@@ -381,27 +382,37 @@ export default class Input extends InputProvider {
     if (c && !this.state.prompt) {
       c.value = hasValue(this.props.model) ? this.props.model.value : ''
       this.setState({ prompt: c })
-    } else if (c && this.props.isFocused) {
+    } else if (c && this.props.isFocused && isInViewport(c)) {
       c.focus()
     }
   }
 
   private readonly _onRef = this.onRef.bind(this)
 
+  /** This is the onFocus property of the active prompt */
+  private readonly _onFocus = (evt: React.FocusEvent<HTMLInputElement>) => {
+    this.props.onInputFocus && this.props.onInputFocus(evt)
+    this.props.willFocusBlock(evt)
+  }
+
   /** the element that represents the command being/having been/going to be executed */
   protected input() {
     const active = isActive(this.props.model)
 
     if (active) {
-      if (this.props.isFocused && this.state.prompt) {
-        this.state.prompt.focus()
+      if (this.props.isFocused && this.state.prompt && document.activeElement !== this.state.prompt) {
+        setTimeout(() => {
+          if (isInViewport(this.state.prompt)) {
+            this.state.prompt.focus()
+          }
+        })
       }
 
       return (
         <React.Fragment>
           <input
             type="text"
-            autoFocus={this.props.isFocused}
+            autoFocus={this.props.isFocused && isInViewport(this.props._block)}
             autoCorrect="off"
             autoComplete="off"
             spellCheck="false"
@@ -413,10 +424,7 @@ export default class Input extends InputProvider {
             tabIndex={1}
             placeholder={this.props.promptPlaceholder}
             onBlur={this.props.onInputBlur}
-            onFocus={evt => {
-              this.props.onInputFocus && this.props.onInputFocus(evt)
-              this.props.willFocusBlock(evt)
-            }}
+            onFocus={this._onFocus}
             onMouseDown={this.props.onInputMouseDown}
             onMouseMove={this.props.onInputMouseMove}
             onChange={this.props.onInputChange}

plugins/plugin-client-common/src/components/Views/Terminal/Block/index.tsx

@@ -118,6 +118,7 @@ export default class Block extends React.PureComponent<Props, State> {
     if (isFinished(this.props.model) || isProcessing(this.props.model)) {
       return (
         <Output
+          key={`Output-${this.props.idx}`}
           uuid={this.props.uuid}
           tab={this.props.tab}
           idx={this.props.idx}

plugins/plugin-client-common/src/components/Views/Terminal/ScrollableTerminal.tsx

@@ -53,13 +53,15 @@ import {
   isOk,
   isOutputOnly,
   isProcessing,
+  isPresentedElsewhere,
   hasStartEvent,
   hasCommand,
   snapshot,
   hasUUID,
   BlockModel
 } from './Block/BlockModel'
 
+import isInViewport from './visible'
 import '../../../../web/scss/components/Terminal/_index.scss'
 
 const strings = i18n('plugin-client-common')
@@ -136,13 +138,6 @@ interface State {
   splits: ScrollbackState[]
 }
 
-/** Is the given `elm` on visible in the current viewport? */
-function isInViewport(elm: HTMLElement) {
-  const rect = elm.getBoundingClientRect()
-  const viewHeight = Math.max(document.documentElement.clientHeight, window.innerHeight)
-  return !(rect.bottom < 0 || rect.top - viewHeight >= 0)
-}
-
 /** get the selected texts in window */
 function getSelectionText() {
   let text = ''
@@ -392,8 +387,13 @@ export default class ScrollableTerminal extends React.PureComponent<Props, State
   }
 
   /** Output.tsx finished rendering something */
-  private onOutputRender(scrollback: ScrollbackState) {
-    setTimeout(() => scrollback.facade.scrollToBottom())
+  private onOutputRender(scrollback: ScrollbackState, idx) {
+    const block = scrollback.blocks[idx]
+    if (isOk(block) && (isTabLayoutModificationResponse(block.response) || isPresentedElsewhere(block))) {
+      // no scroll to bottom for these responses
+    } else {
+      setTimeout(() => scrollback.facade.scrollToBottom())
+    }
   }
 
   /** the REPL started executing a command */
@@ -965,7 +965,7 @@ export default class ScrollableTerminal extends React.PureComponent<Props, State
                     uuid={scrollback.uuid}
                     tab={tab}
                     noActiveInput={this.props.noActiveInput || isOfflineClient()}
-                    onOutputRender={this.onOutputRender.bind(this, scrollback)}
+                    onOutputRender={this.onOutputRender.bind(this, scrollback, idx)}
                     willInsertBlock={this.willInsertBlock.bind(this, scrollback.uuid, idx)}
                     willRemove={this.willRemoveBlock.bind(this, scrollback.uuid, idx)}
                     hasBlockAfter={this.hasBlockAfter(scrollback.blocks, idx)}

plugins/plugin-client-common/src/components/Views/Terminal/visible.ts

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2020 IBM Corporation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/** Is the given `elm` on visible in the current viewport? */
+export default function isInViewport(elm: HTMLElement) {
+  const rect = elm.getBoundingClientRect()
+  const viewHeight = Math.max(document.documentElement.clientHeight, window.innerHeight)
+  return !(rect.bottom < 0 || rect.top - viewHeight >= 0)
+}