Bringing the dialog back to life!

app/src/ui/check-runs/ci-check-run-list-item.tsx

@@ -28,6 +28,12 @@ interface ICICheckRunListItemProps {
   /** Whether or not to show the action workflow event in the title */
   readonly showEventInTitle: boolean
 
+  /** Whether the list item can be selected */
+  readonly selectable: boolean
+
+  /** Whether the list item is selected */
+  readonly selected: boolean
+
   /** Callback for when a check run is clicked */
   readonly onCheckRunExpansionToggleClick: (checkRun: IRefCheck) => void
 
@@ -35,7 +41,7 @@ interface ICICheckRunListItemProps {
   readonly onViewCheckExternally: (checkRun: IRefCheck) => void
 
   /** Callback to open a job steps link on dotcom*/
-  readonly onViewJobStep: (
+  readonly onViewJobStep?: (
     checkRun: IRefCheck,
     step: IAPIWorkflowJobStep
   ) => void
@@ -54,7 +60,7 @@ export class CICheckRunListItem extends React.PureComponent<
   }
 
   private onViewJobStep = (step: IAPIWorkflowJobStep) => {
-    this.props.onViewJobStep(this.props.checkRun, step)
+    this.props.onViewJobStep?.(this.props.checkRun, step)
   }
 
   private renderCheckStatusSymbol = (): JSX.Element => {
@@ -74,9 +80,9 @@ export class CICheckRunListItem extends React.PureComponent<
   }
 
   private renderCheckJobStepToggle = (): JSX.Element | null => {
-    const { checkRun, isCheckRunExpanded } = this.props
+    const { checkRun, isCheckRunExpanded, selectable } = this.props
 
-    if (checkRun.actionJobSteps === undefined) {
+    if (checkRun.actionJobSteps === undefined || selectable) {
       return null
     }
 
@@ -119,7 +125,10 @@ export class CICheckRunListItem extends React.PureComponent<
     return (
       <>
         <div
-          className="ci-check-list-item list-item"
+          className={classNames('ci-check-list-item list-item', {
+            selected: this.props.selected,
+          })}
+          tabIndex={0}
           onClick={this.toggleCheckRunExpansion}
         >
           {this.renderCheckStatusSymbol()}

app/src/ui/check-runs/ci-check-run-list.tsx

@@ -6,6 +6,7 @@ import {
   isFailure,
 } from '../../lib/ci-checks/ci-checks'
 import { CICheckRunListItem } from './ci-check-run-list-item'
+import { FocusContainer } from '../lib/focus-container'
 
 interface ICICheckRunListProps {
   /** List of check runs to display */
@@ -17,11 +18,17 @@ interface ICICheckRunListProps {
   /** Whether loading workflow  */
   readonly loadingActionWorkflows: boolean
 
+  /** Whether check runs can be selected. Default: false */
+  readonly selectable?: boolean
+
   /** Callback to opens check runs target url (maybe GitHub, maybe third party) */
   readonly onViewCheckDetails: (checkRun: IRefCheck) => void
 
+  /** Callback when a check run is clicked */
+  readonly onCheckRunClick?: (checkRun: IRefCheck) => void
+
   /** Callback to open a job steps link on dotcom*/
-  readonly onViewJobStep: (
+  readonly onViewJobStep?: (
     checkRun: IRefCheck,
     step: IAPIWorkflowJobStep
   ) => void
@@ -54,8 +61,7 @@ export class CICheckRunList extends React.PureComponent<
     props: ICICheckRunListProps,
     currentState: ICICheckRunListState | null
   ): ICICheckRunListState {
-    let checkRunExpanded =
-      currentState !== null ? currentState.checkRunExpanded : null
+    let checkRunExpanded = currentState?.checkRunExpanded ?? null
 
     if (currentState === null || !currentState.hasUserToggledCheckRun) {
       // If there is a failure, we want the first check run with a failure, to
@@ -64,10 +70,9 @@ export class CICheckRunList extends React.PureComponent<
       const firstFailure = props.checkRuns.find(
         cr => isFailure(cr) && cr.actionJobSteps !== undefined
       )
-      checkRunExpanded =
-        firstFailure !== undefined
-          ? firstFailure.id.toString()
-          : props.checkRuns[0].id.toString()
+
+      const checkRun = firstFailure ?? props.checkRuns[0]
+      checkRunExpanded = checkRun.id.toString()
     }
 
     const checkRunEvents = new Set(
@@ -83,13 +88,20 @@ export class CICheckRunList extends React.PureComponent<
   }
 
   private onCheckRunClick = (checkRun: IRefCheck): void => {
+    // If the list is selectable, we don't want to toggle when the selected
+    // item is clicked again.
+    const checkRunExpanded =
+      this.state.checkRunExpanded === checkRun.id.toString() &&
+      !this.props.selectable
+        ? null
+        : checkRun.id.toString()
+
     this.setState({
-      checkRunExpanded:
-        this.state.checkRunExpanded === checkRun.id.toString()
-          ? null
-          : checkRun.id.toString(),
+      checkRunExpanded,
       hasUserToggledCheckRun: true,
     })
+
+    this.props.onCheckRunClick?.(checkRun)
   }
 
   private renderList = (): JSX.Element | null => {
@@ -103,13 +115,19 @@ export class CICheckRunList extends React.PureComponent<
         )
       )
       .map((c, i) => {
+        const checkRunExpanded = this.state.checkRunExpanded === c.id.toString()
+        const selectable = this.props.selectable === true
+
         return (
           <CICheckRunListItem
             key={i}
             checkRun={c}
             loadingActionLogs={this.props.loadingActionLogs}
             loadingActionWorkflows={this.props.loadingActionWorkflows}
-            isCheckRunExpanded={this.state.checkRunExpanded === c.id.toString()}
+            selectable={selectable}
+            selected={selectable && checkRunExpanded}
+            // Only expand check runs if the list is not selectable
+            isCheckRunExpanded={!selectable && checkRunExpanded}
             onCheckRunExpansionToggleClick={this.onCheckRunClick}
             onViewCheckExternally={this.props.onViewCheckDetails}
             onViewJobStep={this.props.onViewJobStep}
@@ -118,7 +136,9 @@ export class CICheckRunList extends React.PureComponent<
         )
       })
 
-    return <>{list}</>
+    return (
+      <FocusContainer className="list-focus-container">{list}</FocusContainer>
+    )
   }
 
   public render() {

app/src/ui/notifications/pull-request-checks-failed.tsx

@@ -11,13 +11,13 @@ import {
   getCheckRunActionsJobsAndLogURLS,
   isFailure,
 } from '../../lib/ci-checks/ci-checks'
-import { CICheckRunLogs } from '../check-runs/ci-check-run-item-logs'
 import { Account } from '../../models/account'
-import { API, getHTMLURL } from '../../lib/api'
+import { API, IAPIWorkflowJobStep } from '../../lib/api'
 import { Octicon, syncClockwise } from '../octicons'
 import * as OcticonSymbol from '../octicons/octicons.generated'
 import { Button } from '../lib/button'
 import { RepositoryWithGitHubRepository } from '../../models/repository'
+import { CICheckRunActionsJobStepList } from '../check-runs/ci-check-run-actions-job-step-list'
 
 interface IPullRequestChecksFailedProps {
   readonly dispatcher: Dispatcher
@@ -64,6 +64,12 @@ export class PullRequestChecksFailed extends React.Component<
     }
   }
 
+  private get selectedCheck(): IRefCheck | undefined {
+    return this.state.checks.find(
+      check => check.id === this.state.selectedCheckID
+    )
+  }
+
   public render() {
     let okButtonTitle = __DARWIN__
       ? 'Switch to Pull Request'
@@ -75,7 +81,7 @@ export class PullRequestChecksFailed extends React.Component<
         : 'Switch to repository and pull request'
     }
 
-    const { pullRequest, repository } = this.props
+    const { pullRequest } = this.props
 
     const dialogTitle = (
       <span className="custom-title">
@@ -88,9 +94,7 @@ export class PullRequestChecksFailed extends React.Component<
       </span>
     )
 
-    const selectedCheck = this.state.checks.find(
-      check => check.id === this.state.selectedCheckID
-    )
+    const selectedCheck = this.selectedCheck
 
     const failedChecks = this.state.checks.filter(
       check => check.conclusion === 'failure'
@@ -103,8 +107,6 @@ export class PullRequestChecksFailed extends React.Component<
       this.state.loadingActionLogs ||
       this.state.switchingToPullRequest
 
-    const baseHref = getHTMLURL(repository.gitHubRepository.endpoint)
-
     return (
       <Dialog
         id="pull-request-checks-failed"
@@ -140,21 +142,17 @@ export class PullRequestChecksFailed extends React.Component<
                   checkRuns={this.state.checks}
                   loadingActionLogs={this.state.loadingActionLogs}
                   loadingActionWorkflows={this.state.loadingActionWorkflows}
-                  showLogsInline={false}
                   selectable={true}
-                  selectedCheckRun={selectedCheck}
-                  baseHref={baseHref}
-                  onViewOnGitHub={this.onViewOnGitHub}
+                  onViewCheckDetails={this.onViewOnGitHub}
                   onCheckRunClick={this.onCheckRunClick}
                 />
-                {selectedCheck !== undefined && (
-                  <CICheckRunLogs
-                    checkRun={selectedCheck}
-                    loadingActionLogs={this.state.loadingActionLogs}
-                    loadingActionWorkflows={this.state.loadingActionWorkflows}
-                    baseHref={baseHref}
-                  />
-                )}
+                {selectedCheck !== undefined &&
+                  selectedCheck.actionJobSteps !== undefined && (
+                    <CICheckRunActionsJobStepList
+                      steps={selectedCheck.actionJobSteps}
+                      onViewJobStep={this.onViewJobStep}
+                    />
+                  )}
               </div>
             </div>
           </Row>
@@ -171,6 +169,27 @@ export class PullRequestChecksFailed extends React.Component<
     )
   }
 
+  private onViewJobStep = (step: IAPIWorkflowJobStep): void => {
+    const repository = this.props.repository.gitHubRepository
+    const checkRun = this.selectedCheck
+
+    if (
+      checkRun === undefined ||
+      (checkRun.htmlUrl === null && repository.htmlURL === null)
+    ) {
+      // A check run may not have a url depending on how it is setup.
+      // However, the repository should have one; Thus, we shouldn't hit this
+      return
+    }
+
+    const url =
+      checkRun.htmlUrl !== null
+        ? `${checkRun.htmlUrl}/#step:${step.number}:1`
+        : `${repository.htmlURL}/pull/${this.props.pullRequest.pullRequestNumber}`
+
+    this.props.dispatcher.openInBrowser(url)
+  }
+
   public componentDidMount() {
     this.loadCheckRunLogs()
   }

app/styles/_variables.scss

@@ -32,6 +32,7 @@ $overlay-background-color: rgba(0, 0, 0, 0.4);
 
   --link-button-color: #{$blue};
   --link-button-hover-color: #{$blue-600};
+  --link-button-selected-hover-color: #{$blue-200};
 
   --secondary-button-background: #{$gray-000};
   --secondary-button-hover-background: #{$white};

app/styles/themes/_dark.scss

@@ -28,6 +28,7 @@ body.theme-dark {
 
   --link-button-color: #{lighten($blue-400, 3%)};
   --link-button-hover-color: #{$blue-400};
+  --link-button-selected-hover-color: #{$blue-300};
 
   --secondary-button-background: #{$gray-800};
   --secondary-button-hover-background: var(--secondary-button-background);

app/styles/ui/_pull-request-checks-failed.scss

@@ -65,25 +65,15 @@
     }
 
     .ci-check-run-list {
-      width: 30%;
+      width: 40%;
       border-radius: 0;
     }
-    .ci-check-list-item-logs {
-      width: 70%;
+    .ci-check-run-job-steps-list {
+      width: 60%;
       box-shadow: none;
       border: none;
       border-left: var(--base-border);
       border-right: none;
     }
-    .ci-check-list-item-logs-output {
-      max-height: none;
-      height: 100%;
-    }
-    .loading-logs {
-      box-shadow: none;
-      border: none;
-      border-left: var(--base-border);
-      width: 70%;
-    }
   }
 }

app/styles/ui/check-runs/_ci-check-run-list-item.scss

@@ -2,9 +2,16 @@
   display: flex;
   width: 100%;
   border-bottom: var(--base-border);
-  height: 100%;
+  height: auto;
 
-  .ci-check-name span {
+  &.selected .ci-check-name span {
+    &:hover {
+      color: var(--link-button-selected-hover-color);
+      cursor: pointer !important;
+    }
+  }
+
+  &:not(.selected) .ci-check-name span {
     &:hover {
       color: var(--link-button-color);
       cursor: pointer !important;