From e90f3d0b46ef81912a1635deec1285cf3079a7c9 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Fri, 11 Nov 2022 16:31:12 +0100 Subject: [PATCH 1/2] Keep track of checkbox state in view This will add a new `useState` call on the top-level to keep track of the checkbox state. It will allow all downloaded repositories to be selected. This will allow us to make the copy repository list and export results button dependent on the selected repositories. --- .../src/view/variant-analysis/RepoRow.tsx | 46 ++++++++++++++++++- .../view/variant-analysis/VariantAnalysis.tsx | 4 ++ .../VariantAnalysisAnalyzedRepos.tsx | 23 +++++++++- .../VariantAnalysisOutcomePanels.tsx | 11 ++++- .../__tests__/RepoRow.spec.tsx | 40 +++++++++++++++- 5 files changed, 119 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx b/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx index a38d1e4f9ab..659c139b25f 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx +++ b/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,21 @@ const canExpand = ( return downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded || downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Failed; }; +const canSelect = ( + status: VariantAnalysisRepoStatus | undefined, + downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined, +): boolean => { + if (!status) { + return false; + } + + if (status !== VariantAnalysisRepoStatus.Succeeded) { + return false; + } + + return downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded; +}; + const isExpandableContentLoaded = ( status: VariantAnalysisRepoStatus | undefined, downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined, @@ -133,6 +151,8 @@ export const RepoRow = ({ resultCount, interpretedResults, rawResults, + selected, + onSelectedChange, }: RepoRowProps) => { const [isExpanded, setExpanded] = useState(false); const resultsLoaded = !!interpretedResults || !!rawResults; @@ -163,13 +183,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) => { + // 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 (
- + {isExpanded ? : } {resultCount === undefined ? '-' : formatDecimal(resultCount)} diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx index f489e755edd..46ec7198f99 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx @@ -51,6 +51,8 @@ export function VariantAnalysis({ const [repoStates, setRepoStates] = useState(initialRepoStates); const [repoResults, setRepoResults] = useState(initialRepoResults); + const [selectedRepositoryIds, setSelectedRepositoryIds] = useState([]); + useEffect(() => { const listener = (evt: MessageEvent) => { if (evt.origin === window.origin) { @@ -103,6 +105,8 @@ export function VariantAnalysis({ variantAnalysis={variantAnalysis} repositoryStates={repoStates} repositoryResults={repoResults} + selectedRepositoryIds={selectedRepositoryIds} + setSelectedRepositoryIds={setSelectedRepositoryIds} /> ); diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx index a98cd5f44a7..71f6feecd35 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisAnalyzedRepos.tsx +++ b/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,6 +22,9 @@ export type VariantAnalysisAnalyzedReposProps = { repositoryResults?: VariantAnalysisScannedRepositoryResult[]; filterSortState?: RepositoriesFilterSortState; + + selectedRepositoryIds?: number[]; + setSelectedRepositoryIds?: Dispatch>; } export const VariantAnalysisAnalyzedRepos = ({ @@ -29,6 +32,8 @@ export const VariantAnalysisAnalyzedRepos = ({ repositoryStates, repositoryResults, filterSortState, + selectedRepositoryIds, + setSelectedRepositoryIds, }: VariantAnalysisAnalyzedReposProps) => { const repositoryStateById = useMemo(() => { const map = new Map(); @@ -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 ( {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} /> ); })} diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx index 5f480610567..db4c9fda8b2 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx +++ b/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>; }; const Tab = styled(VSCodePanelTab)` @@ -46,6 +49,8 @@ export const VariantAnalysisOutcomePanels = ({ variantAnalysis, repositoryStates, repositoryResults, + selectedRepositoryIds, + setSelectedRepositoryIds, }: VariantAnalysisOutcomePanelProps) => { const [filterSortState, setFilterSortState] = useState(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} /> {notFoundRepos?.repositoryCount && diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx index 5cdf1c0918d..ed6fc514975 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/RepoRow.spec.tsx +++ b/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(); + }); }); From 30988993be79400effd091f87481254de3607c46 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Mon, 14 Nov 2022 16:33:08 +0100 Subject: [PATCH 2/2] Simplify `canSelect` method Co-authored-by: Robert --- .../ql-vscode/src/view/variant-analysis/RepoRow.tsx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx b/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx index 659c139b25f..c541127d2ab 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/RepoRow.tsx @@ -107,17 +107,7 @@ const canExpand = ( const canSelect = ( status: VariantAnalysisRepoStatus | undefined, downloadStatus: VariantAnalysisScannedRepositoryDownloadStatus | undefined, -): boolean => { - if (!status) { - return false; - } - - if (status !== VariantAnalysisRepoStatus.Succeeded) { - return false; - } - - return downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded; -}; +) => status == VariantAnalysisRepoStatus.Succeeded && downloadStatus === VariantAnalysisScannedRepositoryDownloadStatus.Succeeded; const isExpandableContentLoaded = ( status: VariantAnalysisRepoStatus | undefined,