From ef813e7eb27fc44737f32a892b4af41b0f3ad8d9 Mon Sep 17 00:00:00 2001
From: Nick Mitchell <nickm@us.ibm.com>
Date: Fri, 4 Jun 2021 12:04:27 -0400
Subject: [PATCH] fix(plugins/plugin-client-common): content does not always
 reliably scroll into view after command execution

part of #7524
---
 .../src/components/Content/Commentary.tsx     |  2 ++
 .../src/components/Content/Markdown.tsx       |  6 ++++
 .../src/components/Content/Scalar/index.tsx   | 36 ++++++++++++++++---
 .../Content/Table/PaginatedTable.tsx          |  6 ++++
 .../src/components/Content/Table/index.tsx    |  1 +
 .../Views/Terminal/Block/Output.tsx           |  1 +
 .../components/Views/Terminal/Block/index.tsx | 13 ++++++-
 .../Views/Terminal/ScrollableTerminal.tsx     | 12 ++++---
 .../src/components/spi/Breadcrumb/index.tsx   |  2 +-
 .../src/components/spi/Card/index.tsx         |  1 -
 10 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/plugins/plugin-client-common/src/components/Content/Commentary.tsx b/plugins/plugin-client-common/src/components/Content/Commentary.tsx
index 8875612be6b..ca4260b1f23 100644
--- a/plugins/plugin-client-common/src/components/Content/Commentary.tsx
+++ b/plugins/plugin-client-common/src/components/Content/Commentary.tsx
@@ -36,6 +36,7 @@ type Props = CommentaryResponse['props'] & {
   willUpdateResponse?: (text: string) => void
   willRemove?: () => void
   willUpdateCommand?: (command: string) => void
+  onRender: () => void
 }
 
 export default class Commentary extends React.PureComponent<Props, State> {
@@ -230,6 +231,7 @@ export default class Commentary extends React.PureComponent<Props, State> {
   }
 
   public render() {
+    this.props.onRender()
     return (
       <div className="kui--commentary" data-is-editing={this.state.isEdit || undefined}>
         {this.card()}
diff --git a/plugins/plugin-client-common/src/components/Content/Markdown.tsx b/plugins/plugin-client-common/src/components/Content/Markdown.tsx
index 7f549da8525..1edd834ff66 100644
--- a/plugins/plugin-client-common/src/components/Content/Markdown.tsx
+++ b/plugins/plugin-client-common/src/components/Content/Markdown.tsx
@@ -50,6 +50,8 @@ interface Props {
 
   /** Base HTTP Url? */
   baseUrl?: string
+
+  onRender?: () => void
 }
 
 export default class Markdown extends React.PureComponent<Props> {
@@ -101,6 +103,10 @@ export default class Markdown extends React.PureComponent<Props> {
   }
 
   public render() {
+    if (this.props.onRender) {
+      this.props.onRender()
+    }
+
     return (
       <ReactMarkdown
         plugins={[gfm]}
diff --git a/plugins/plugin-client-common/src/components/Content/Scalar/index.tsx b/plugins/plugin-client-common/src/components/Content/Scalar/index.tsx
index 63fc5aae01c..53f3115823b 100644
--- a/plugins/plugin-client-common/src/components/Content/Scalar/index.tsx
+++ b/plugins/plugin-client-common/src/components/Content/Scalar/index.tsx
@@ -95,15 +95,26 @@ export default class Scalar extends React.PureComponent<Props, State> {
     console.error('catastrophic error in Scalar', error, errorInfo)
   }
 
+  private onRender() {
+    if (this.props.onRender) {
+      setTimeout(() => this.props.onRender(true), 0)
+    }
+  }
+
+  private readonly _onRender = this.onRender.bind(this)
+
   private renderResponse(response: Props['response']) {
     const { tab } = this.props
 
     if (typeof response === 'boolean') {
+      this.onRender()
       return <React.Fragment />
     } else if (typeof response === 'number') {
+      this.onRender()
       return <pre>{response}</pre>
     } else if (isUsageError(response)) {
       // hopefully we can do away with this shortly
+      this.onRender()
       if (typeof response.raw === 'string') {
         return <pre>{response.raw}</pre>
       } else if (isMessageWithUsageModel(response.raw) || isMessageWithCode(response.raw)) {
@@ -112,11 +123,13 @@ export default class Scalar extends React.PureComponent<Props, State> {
         return <HTMLDom content={response.raw} />
       }
     } else if (isXtermResponse(response)) {
+      this.onRender()
       return <XtermDom response={response} />
     } else if (typeof response === 'string' || isError(response)) {
       const message = isError(response) ? response.message : response
 
       // Markdown interprets escapes, so we need to double-escape
+      this.onRender()
       return (
         <pre>
           <Markdown tab={tab} repl={tab.REPL} source={message.replace(/\\/g, '\\\\').replace(/\n/g, '\n\n')} />
@@ -130,6 +143,7 @@ export default class Scalar extends React.PureComponent<Props, State> {
             repl={tab.REPL}
             tabUUID={getPrimaryTabId(tab)}
             isPartOfMiniSplit={this.props.isPartOfMiniSplit}
+            onRender={this._onRender}
             willRemove={this.props.willRemove}
             willUpdateCommand={this.props.willUpdateCommand}
             willUpdateResponse={(text: string) => {
@@ -139,6 +153,7 @@ export default class Scalar extends React.PureComponent<Props, State> {
         </span>
       )
     } else if (isRadioTable(response)) {
+      this.onRender()
       return (
         <KuiContext.Consumer>
           {config => <RadioTableSpi table={response} title={!config.disableTableTitle} repl={tab.REPL} />}
@@ -158,7 +173,7 @@ export default class Scalar extends React.PureComponent<Props, State> {
         undefined,
         renderBottomToolbar,
         renderGrid,
-        this.props.onRender,
+        this._onRender,
         this.props.isPartOfMiniSplit,
         this.props.isWidthConstrained
       )
@@ -174,19 +189,30 @@ export default class Scalar extends React.PureComponent<Props, State> {
         </React.Fragment>
       )
     } else if (isReactResponse(response)) {
+      this.onRender()
       return response.react
     } else if (isHTML(response)) {
       // ^^^ intentionally using an "else" so that typescript double
       // checks that we've covered every case of ScalarResponse
+      this.onRender()
       return <HTMLDom content={response} />
     } else if (isMarkdownResponse(response)) {
-      return <Markdown tab={tab} repl={tab.REPL} source={response.content} />
+      return <Markdown tab={tab} repl={tab.REPL} source={response.content} onRender={this._onRender} />
     } else if (isRandomErrorResponse1(response)) {
       // maybe this is an error response from some random API?
-      return <Markdown tab={tab} repl={tab.REPL} source={strings('randomError1', response.code)} />
+      return (
+        <Markdown tab={tab} repl={tab.REPL} source={strings('randomError1', response.code)} onRender={this._onRender} />
+      )
     } else if (isRandomErrorResponse2(response)) {
       // maybe this is an error response from some random API?
-      return <Markdown tab={tab} repl={tab.REPL} source={strings('randomError2', response.errno)} />
+      return (
+        <Markdown
+          tab={tab}
+          repl={tab.REPL}
+          source={strings('randomError2', response.errno)}
+          onRender={this._onRender}
+        />
+      )
     } else if (isMultiModalResponse(response)) {
       return (
         <TopNavSidecar
@@ -216,10 +242,12 @@ export default class Scalar extends React.PureComponent<Props, State> {
         />
       )
     } else if (isAbortableResponse(response)) {
+      this.onRender()
       return this.renderResponse(response.response)
     }
 
     console.error('unexpected null return from Scalar:', this.props.response)
+    this.onRender()
     return <pre className="oops">Internal Error in command execution</pre>
   }
 
diff --git a/plugins/plugin-client-common/src/components/Content/Table/PaginatedTable.tsx b/plugins/plugin-client-common/src/components/Content/Table/PaginatedTable.tsx
index 6a60e73fcf1..fdfc2d0bcae 100644
--- a/plugins/plugin-client-common/src/components/Content/Table/PaginatedTable.tsx
+++ b/plugins/plugin-client-common/src/components/Content/Table/PaginatedTable.tsx
@@ -83,6 +83,8 @@ export type Props<T extends KuiTable = KuiTable> = PaginationConfiguration & {
 
   /** prefix breadcrumbs? */
   prefixBreadcrumbs?: BreadcrumbView[]
+
+  onRender: (hasContent: boolean) => void
 }
 
 /** state of PaginatedTable component */
@@ -419,6 +421,10 @@ export default class PaginatedTable<P extends Props, S extends State> extends Re
   }
 
   private content(includeToolbars = false, lightweightTables = false) {
+    if (this.props.onRender) {
+      this.props.onRender(true)
+    }
+
     return (
       <React.Fragment>
         {includeToolbars && this.topToolbar(lightweightTables)}
diff --git a/plugins/plugin-client-common/src/components/Content/Table/index.tsx b/plugins/plugin-client-common/src/components/Content/Table/index.tsx
index 0fc3c3f8ffa..56273c3c641 100644
--- a/plugins/plugin-client-common/src/components/Content/Table/index.tsx
+++ b/plugins/plugin-client-common/src/components/Content/Table/index.tsx
@@ -69,6 +69,7 @@ export default function renderTable(
                 title={!config.disableTableTitle}
                 toolbars={toolbars}
                 asGrid={asGrid}
+                onRender={onRender}
                 isPartOfMiniSplit={isPartOfMiniSplit}
                 isWidthConstrained={isWidthConstrained}
               />
diff --git a/plugins/plugin-client-common/src/components/Views/Terminal/Block/Output.tsx b/plugins/plugin-client-common/src/components/Views/Terminal/Block/Output.tsx
index 1c04311e134..4ed92c8860b 100644
--- a/plugins/plugin-client-common/src/components/Views/Terminal/Block/Output.tsx
+++ b/plugins/plugin-client-common/src/components/Views/Terminal/Block/Output.tsx
@@ -264,6 +264,7 @@ export default class Output extends React.PureComponent<Props, State> {
                 willFocusBlock={this.props.willFocusBlock}
                 willRemove={this._willRemove}
                 willUpdateCommand={this._willUpdateCommand}
+                onRender={this._onRender}
               />
             </React.Suspense>
           )}
diff --git a/plugins/plugin-client-common/src/components/Views/Terminal/Block/index.tsx b/plugins/plugin-client-common/src/components/Views/Terminal/Block/index.tsx
index 0e029f95e76..ecec79e0d6b 100644
--- a/plugins/plugin-client-common/src/components/Views/Terminal/Block/index.tsx
+++ b/plugins/plugin-client-common/src/components/Views/Terminal/Block/index.tsx
@@ -131,6 +131,17 @@ export default class Block extends React.PureComponent<Props, State> {
 
   private readonly _willChangeSize = this.willChangeSize.bind(this)
 
+  private onOutputRender() {
+    if (this.props.onOutputRender) {
+      this.props.onOutputRender()
+    }
+    if (this.props.noActiveInput && this.state._block) {
+      this.state._block.scrollIntoView()
+    }
+  }
+
+  private readonly _onOutputRender = this.onOutputRender.bind(this)
+
   private output() {
     if (isFinished(this.props.model) || isProcessing(this.props.model)) {
       return (
@@ -142,7 +153,7 @@ export default class Block extends React.PureComponent<Props, State> {
           isBeingRerun={isBeingRerun(this.props.model)}
           willRemove={this.props.willRemove}
           willChangeSize={this._willChangeSize}
-          onRender={this.props.onOutputRender}
+          onRender={this._onOutputRender}
           willUpdateCommand={this.props.willUpdateCommand}
           isPartOfMiniSplit={this.props.isPartOfMiniSplit}
           isWidthConstrained={this.props.isWidthConstrained}
diff --git a/plugins/plugin-client-common/src/components/Views/Terminal/ScrollableTerminal.tsx b/plugins/plugin-client-common/src/components/Views/Terminal/ScrollableTerminal.tsx
index a029e3d6653..33667091ca8 100644
--- a/plugins/plugin-client-common/src/components/Views/Terminal/ScrollableTerminal.tsx
+++ b/plugins/plugin-client-common/src/components/Views/Terminal/ScrollableTerminal.tsx
@@ -432,10 +432,14 @@ export default class ScrollableTerminal extends React.PureComponent<Props, State
 
     /** Output.tsx finished rendering something */
     state.onOutputRender = () => {
-      const sbidx = this.findSplit(this.state, sbuuid)
-      if (sbidx >= 0) {
-        const scrollback = this.state.splits[sbidx]
-        setTimeout(() => scrollback.facade.scrollToBottom())
+      if (!this.props.noActiveInput) {
+        // if we are using inline input, then scroll to the bottom
+        // whenever an output is rendered in this split
+        const sbidx = this.findSplit(this.state, sbuuid)
+        if (sbidx >= 0) {
+          const scrollback = this.state.splits[sbidx]
+          setTimeout(() => scrollback.facade.scrollToBottom())
+        }
       }
     }
 
diff --git a/plugins/plugin-client-common/src/components/spi/Breadcrumb/index.tsx b/plugins/plugin-client-common/src/components/spi/Breadcrumb/index.tsx
index 2279a05f4a5..66997607526 100644
--- a/plugins/plugin-client-common/src/components/spi/Breadcrumb/index.tsx
+++ b/plugins/plugin-client-common/src/components/spi/Breadcrumb/index.tsx
@@ -16,7 +16,7 @@
 
 import React from 'react'
 
-const PatternFly4 = React.lazy(() => import('./impl/PatternFly'))
+import PatternFly4 from './impl/PatternFly'
 
 import Props, { BreadcrumbView } from './model'
 export { Props, BreadcrumbView }
diff --git a/plugins/plugin-client-common/src/components/spi/Card/index.tsx b/plugins/plugin-client-common/src/components/spi/Card/index.tsx
index bb8df55969d..2a8bd7a2c66 100644
--- a/plugins/plugin-client-common/src/components/spi/Card/index.tsx
+++ b/plugins/plugin-client-common/src/components/spi/Card/index.tsx
@@ -25,7 +25,6 @@ import Props from './model'
  */
 const PatternFly4 = React.lazy(() => import('./impl/PatternFly'))
 
-// FIXME There's no ideal Card component in Carbon Component Libary, so we use Patternfly
 export default function CardSpi(props: Props): React.ReactElement {
   return (
     <React.Suspense fallback={<div />}>