Merge pull request #1755 from github/koesie10/checkbox-state

Keep track of checkbox state in view

Author: koesie10

extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { useCallback, useEffect, useState } from 'react';
+import { ChangeEvent, useCallback, useEffect, useState } from 'react';
 import styled from 'styled-components';
 import { VSCodeBadge, VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react';
 import {
@@ -80,6 +80,9 @@ export type RepoRowProps = {
 
   interpretedResults?: AnalysisAlert[];
   rawResults?: AnalysisRawResults;
+
+  selected?: boolean;
+  onSelectedChange?: (repositoryId: number, selected: boolean) => void;
 }
 
 const canExpand = (
@@ -101,6 +104,11 @@ const canExpand = (
   return downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded || downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed;
 };
 
+const canSelect = (
+  status: VariantAnalysisRepoStatus | undefined,
+  downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined,
+) => status == VariantAnalysisRepoStatus.Succeeded && downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded;
+
 const isExpandableContentLoaded = (
   status: VariantAnalysisRepoStatus | undefined,
   downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined,
@@ -133,6 +141,8 @@ export const RepoRow = ({
   resultCount,
   interpretedResults,
   rawResults,
+  selected,
+  onSelectedChange,
 }: RepoRowProps) => {
   const [isExpanded, setExpanded] = useState(false);
   const resultsLoaded = !!interpretedResults || !!rawResults;
@@ -163,13 +173,35 @@ export const RepoRow = ({
     }
   }, [resultsLoaded, resultsLoading]);
 
+  const onClickCheckbox = useCallback((e: React.MouseEvent) => {
+    // Prevent calling the onClick event of the container, which would toggle the expanded state
+    e.stopPropagation();
+  }, []);
+  const onChangeCheckbox = useCallback((e: ChangeEvent<HTMLInputElement>) => {
+    // This is called on first render, but we don't really care about this value
+    if (e.target.checked === undefined) {
+      return;
+    }
+
+    if (!repository.id) {
+      return;
+    }
+
+    onSelectedChange?.(repository.id, e.target.checked);
+  }, [onSelectedChange, repository]);
+
   const disabled = !canExpand(status, downloadStatus);
   const expandableContentLoaded = isExpandableContentLoaded(status, downloadStatus, resultsLoaded);
 
   return (
     <div>
       <TitleContainer onClick={toggleExpanded} disabled={disabled} aria-expanded={isExpanded}>
-        <VSCodeCheckbox disabled />
+        <VSCodeCheckbox
+          onChange={onChangeCheckbox}
+          onClick={onClickCheckbox}
+          checked={selected}
+          disabled={!repository.id || !canSelect(status, downloadStatus)}
+        />
         {isExpanded ? <ExpandCollapseCodicon name="chevron-down" label="Collapse" /> :
           <ExpandCollapseCodicon name="chevron-right" label="Expand" />}
         <VSCodeBadge>{resultCount === undefined ? '-' : formatDecimal(resultCount)}</VSCodeBadge>

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx

@@ -57,6 +57,8 @@ export function VariantAnalysis({
   const [repoStates, setRepoStates] = useState<VariantAnalysisScannedRepositoryState[]>(initialRepoStates);
   const [repoResults, setRepoResults] = useState<VariantAnalysisScannedRepositoryResult[]>(initialRepoResults);
 
+  const [selectedRepositoryIds, setSelectedRepositoryIds] = useState<number[]>([]);
+
   useEffect(() => {
     const listener = (evt: MessageEvent) => {
       if (evt.origin === window.origin) {
@@ -109,6 +111,8 @@ export function VariantAnalysis({
         variantAnalysis={variantAnalysis}
         repositoryStates={repoStates}
         repositoryResults={repoResults}
+        selectedRepositoryIds={selectedRepositoryIds}
+        setSelectedRepositoryIds={setSelectedRepositoryIds}
       />
     </>
   );

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { useMemo } from 'react';
+import { Dispatch, SetStateAction, useCallback, useMemo } from 'react';
 import styled from 'styled-components';
 import { RepoRow } from './RepoRow';
 import {
@@ -22,13 +22,18 @@ export type VariantAnalysisAnalyzedReposProps = {
   repositoryResults?: VariantAnalysisScannedRepositoryResult[];
 
   filterSortState?: RepositoriesFilterSortState;
+
+  selectedRepositoryIds?: number[];
+  setSelectedRepositoryIds?: Dispatch<SetStateAction<number[]>>;
 }
 
 export const VariantAnalysisAnalyzedRepos = ({
   variantAnalysis,
   repositoryStates,
   repositoryResults,
   filterSortState,
+  selectedRepositoryIds,
+  setSelectedRepositoryIds,
 }: VariantAnalysisAnalyzedReposProps) => {
   const repositoryStateById = useMemo(() => {
     const map = new Map<number, VariantAnalysisScannedRepositoryState>();
@@ -52,6 +57,20 @@ export const VariantAnalysisAnalyzedRepos = ({
     })?.sort(compareWithResults(filterSortState));
   }, [filterSortState, variantAnalysis.scannedRepos]);
 
+  const onSelectedChange = useCallback((repositoryId: number, selected: boolean) => {
+    setSelectedRepositoryIds?.((prevSelectedRepositoryIds) => {
+      if (selected) {
+        if (prevSelectedRepositoryIds.includes(repositoryId)) {
+          return prevSelectedRepositoryIds;
+        }
+
+        return [...prevSelectedRepositoryIds, repositoryId];
+      } else {
+        return prevSelectedRepositoryIds.filter((id) => id !== repositoryId);
+      }
+    });
+  }, [setSelectedRepositoryIds]);
+
   return (
     <Container>
       {repositories?.map(repository => {
@@ -67,6 +86,8 @@ export const VariantAnalysisAnalyzedRepos = ({
             resultCount={repository.resultCount}
             interpretedResults={results?.interpretedResults}
             rawResults={results?.rawResults}
+            selected={selectedRepositoryIds?.includes(repository.repository.id)}
+            onSelectedChange={onSelectedChange}
           />
         );
       })}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { useState } from 'react';
+import { Dispatch, SetStateAction, useState } from 'react';
 import styled from 'styled-components';
 import { VSCodeBadge, VSCodePanels, VSCodePanelTab, VSCodePanelView } from '@vscode/webview-ui-toolkit/react';
 import { formatDecimal } from '../../pure/number';
@@ -20,6 +20,9 @@ export type VariantAnalysisOutcomePanelProps = {
   variantAnalysis: VariantAnalysis;
   repositoryStates?: VariantAnalysisScannedRepositoryState[];
   repositoryResults?: VariantAnalysisScannedRepositoryResult[];
+
+  selectedRepositoryIds?: number[];
+  setSelectedRepositoryIds?: Dispatch<SetStateAction<number[]>>;
 };
 
 const Tab = styled(VSCodePanelTab)`
@@ -46,6 +49,8 @@ export const VariantAnalysisOutcomePanels = ({
   variantAnalysis,
   repositoryStates,
   repositoryResults,
+  selectedRepositoryIds,
+  setSelectedRepositoryIds,
 }: VariantAnalysisOutcomePanelProps) => {
   const [filterSortState, setFilterSortState] = useState<RepositoriesFilterSortState>(defaultFilterSortState);
 
@@ -94,6 +99,8 @@ export const VariantAnalysisOutcomePanels = ({
           repositoryStates={repositoryStates}
           repositoryResults={repositoryResults}
           filterSortState={filterSortState}
+          selectedRepositoryIds={selectedRepositoryIds}
+          setSelectedRepositoryIds={setSelectedRepositoryIds}
         />
       </>
     );
@@ -126,6 +133,8 @@ export const VariantAnalysisOutcomePanels = ({
             repositoryStates={repositoryStates}
             repositoryResults={repositoryResults}
             filterSortState={filterSortState}
+            selectedRepositoryIds={selectedRepositoryIds}
+            setSelectedRepositoryIds={setSelectedRepositoryIds}
           />
         </VSCodePanelView>
         {notFoundRepos?.repositoryCount &&

extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx

@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { render as reactRender, screen } from '@testing-library/react';
+import { render as reactRender, screen, waitFor } from '@testing-library/react';
 import {
   VariantAnalysisRepoStatus,
   VariantAnalysisScannedRepositoryDownloadStatus
@@ -330,4 +330,42 @@ describe(RepoRow.name, () => {
       expanded: false
     })).toBeDisabled();
   });
+
+  it('does not allow selecting the item if the item has not succeeded', async () => {
+    render({
+      status: VariantAnalysisRepoStatus.InProgress,
+    });
+
+    expect(screen.getByRole('checkbox')).toBeDisabled();
+  });
+
+  it('does not allow selecting the item if the item has not been downloaded', async () => {
+    render({
+      status: VariantAnalysisRepoStatus.Succeeded,
+    });
+
+    expect(screen.getByRole('checkbox')).toBeDisabled();
+  });
+
+  it('does not allow selecting the item if the item has not been downloaded successfully', async () => {
+    render({
+      status: VariantAnalysisRepoStatus.Succeeded,
+      downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Failed,
+    });
+
+    // It seems like sometimes the first render doesn't have the checkbox disabled
+    // Might be related to https://github.com/microsoft/vscode-webview-ui-toolkit/issues/404
+    await waitFor(() => {
+      expect(screen.getByRole('checkbox')).toBeDisabled();
+    });
+  });
+
+  it('allows selecting the item if the item has been downloaded', async () => {
+    render({
+      status: VariantAnalysisRepoStatus.Succeeded,
+      downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus.Succeeded,
+    });
+
+    expect(screen.getByRole('checkbox')).toBeEnabled();
+  });
 });