From e384f2447cf98068a2e399feee2299eb1709398e Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 4 Jul 2023 11:47:59 +0100 Subject: [PATCH 1/8] Convert data table to new column layout --- .../view/data-extensions-editor/MethodRow.tsx | 70 ++++++------------- .../ModeledMethodDataGrid.tsx | 18 ++--- 2 files changed, 26 insertions(+), 62 deletions(-) diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 4ba3ca1b5..8bd821d8f 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -1,7 +1,9 @@ import { + VSCodeCheckbox, VSCodeDataGridCell, VSCodeDataGridRow, VSCodeDropdown, + VSCodeLink, VSCodeOption, } from "@vscode/webview-ui-toolkit/react"; import * as React from "react"; @@ -38,29 +40,18 @@ const SupportSpan = styled.span` }}; `; -type SupportedUnsupportedLinkProps = { - supported: boolean; - modeled: ModeledMethod | undefined; -}; - -const SupportLink = styled.button` - color: ${(props) => { - if (!props.supported && props.modeled && props.modeled?.type !== "none") { - return "orange"; - } else { - return props.supported ? "green" : "red"; - } - }}; - background-color: transparent; - border: none; - cursor: pointer; - padding: 0; +const ApiOrMethodCell = styled(VSCodeDataGridCell)` + display: flex; + flex-direction: row; + align-items: center; + gap: 0.5em; `; const UsagesButton = styled.button` color: var(--vscode-editor-foreground); - background-color: transparent; + background-color: var(--vscode-input-background); border: none; + border-radius: 40%; cursor: pointer; `; @@ -171,43 +162,24 @@ export const MethodRow = ({ return ( - + + - {externalApiUsage.packageName}.{externalApiUsage.typeName} + {externalApiUsage.packageName}.{externalApiUsage.typeName}. + {externalApiUsage.methodName} + {externalApiUsage.methodParameters} - - {mode === Mode.Application && ( - - {externalApiUsage.methodName} - {externalApiUsage.methodParameters} - - )} - {mode === Mode.Framework && ( - - {externalApiUsage.methodName} - {externalApiUsage.methodParameters} - - )} - - {mode === Mode.Application && ( - {externalApiUsage.usages.length} - - )} - + )} + View + + {(!externalApiUsage.supported || (modeledMethod && modeledMethod?.type !== "none")) && ( )} - + {modeledMethod?.type && ["sink", "summary"].includes(modeledMethod?.type) && ( @@ -235,7 +207,7 @@ export const MethodRow = ({ )} - + {modeledMethod?.type && ["source", "summary"].includes(modeledMethod?.type) && ( @@ -249,7 +221,7 @@ export const MethodRow = ({ )} - + {predicate?.supportedKinds && ( + - Type + API or method - Method - - {mode === Mode.Application && ( - - Usages - - )} - Model type - + Input - + Output - + Kind From 870827085d8fcc75b0cfc11ba1dc882b7184d022 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 4 Jul 2023 14:54:21 +0100 Subject: [PATCH 2/8] Remove colouring of the methods --- .../view/data-extensions-editor/MethodRow.tsx | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 8bd821d8f..71f97ff6f 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -25,21 +25,6 @@ const Dropdown = styled(VSCodeDropdown)` width: 100%; `; -type SupportedUnsupportedSpanProps = { - supported: boolean; - modeled: ModeledMethod | undefined; -}; - -const SupportSpan = styled.span` - color: ${(props) => { - if (!props.supported && props.modeled && props.modeled?.type !== "none") { - return "orange"; - } else { - return props.supported ? "green" : "red"; - } - }}; -`; - const ApiOrMethodCell = styled(VSCodeDataGridCell)` display: flex; flex-direction: row; @@ -164,14 +149,11 @@ export const MethodRow = ({ - + {externalApiUsage.packageName}.{externalApiUsage.typeName}. {externalApiUsage.methodName} {externalApiUsage.methodParameters} - + {mode === Mode.Application && ( {externalApiUsage.usages.length} From e82bfb4153d083abe742b407367427528235c030 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 4 Jul 2023 17:10:10 +0100 Subject: [PATCH 3/8] Pull out booleans controlling visibility of each cell --- .../view/data-extensions-editor/MethodRow.tsx | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 71f97ff6f..45f235c39 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -145,6 +145,15 @@ export const MethodRow = ({ ? extensiblePredicateDefinitions[modeledMethod.type] : undefined; + const showModelTypeCell = + !externalApiUsage.supported || + (modeledMethod && modeledMethod?.type !== "none"); + const showInputCell = + modeledMethod?.type && ["sink", "summary"].includes(modeledMethod?.type); + const showOutputCell = + modeledMethod?.type && ["source", "summary"].includes(modeledMethod?.type); + const showKindCell = predicate?.supportedKinds; + return ( @@ -162,8 +171,7 @@ export const MethodRow = ({ View - {(!externalApiUsage.supported || - (modeledMethod && modeledMethod?.type !== "none")) && ( + {showModelTypeCell && ( - {modeledMethod?.type && - ["sink", "summary"].includes(modeledMethod?.type) && ( - - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} - - )} + {showInputCell && ( + + Argument[this] + {argumentsList.map((argument, index) => ( + + Argument[{index}]: {argument} + + ))} + + )} - {modeledMethod?.type && - ["source", "summary"].includes(modeledMethod?.type) && ( - - ReturnValue - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} - - )} + {showOutputCell && ( + + ReturnValue + Argument[this] + {argumentsList.map((argument, index) => ( + + Argument[{index}]: {argument} + + ))} + + )} - {predicate?.supportedKinds && ( + {showKindCell && ( From f75b358e6ce64c82ab0348c613912b3875b926a2 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 4 Jul 2023 17:11:45 +0100 Subject: [PATCH 4/8] Disable buttons instead of hiding them --- .../view/data-extensions-editor/KindInput.tsx | 5 +- .../view/data-extensions-editor/MethodRow.tsx | 82 ++++++++++--------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx index 52f49f6b6..dac3d28e8 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx @@ -13,10 +13,11 @@ type Props = { kinds: Array; value: ModeledMethod["kind"] | undefined; + disabled?: boolean; onChange: (value: ModeledMethod["kind"]) => void; }; -export const KindInput = ({ kinds, value, onChange }: Props) => { +export const KindInput = ({ kinds, value, disabled, onChange }: Props) => { const handleInput = useCallback( (e: InputEvent) => { const target = e.target as HTMLSelectElement; @@ -37,7 +38,7 @@ export const KindInput = ({ kinds, value, onChange }: Props) => { }, [value, kinds, onChange]); return ( - + {kinds.map((kind) => ( {kind} diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 45f235c39..386630581 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -171,52 +171,54 @@ export const MethodRow = ({ View - {showModelTypeCell && ( - - Unmodeled - Source - Sink - Flow summary - Neutral - - )} + + Unmodeled + Source + Sink + Flow summary + Neutral + - {showInputCell && ( - - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} - - )} + + Argument[this] + {argumentsList.map((argument, index) => ( + + Argument[{index}]: {argument} + + ))} + - {showOutputCell && ( - - ReturnValue - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} - - )} + + ReturnValue + Argument[this] + {argumentsList.map((argument, index) => ( + + Argument[{index}]: {argument} + + ))} + - {showKindCell && ( - - )} + ); From d5c4f33d6e5b82088e30506cb03f48073d1ff047 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 6 Jul 2023 11:03:59 +0100 Subject: [PATCH 5/8] Use a native select instead of VSCodeDropdown --- .../ql-vscode/src/view/common/Dropdown.tsx | 21 +++++ .../view/data-extensions-editor/KindInput.tsx | 27 +++---- .../view/data-extensions-editor/MethodRow.tsx | 79 ++++++++++--------- 3 files changed, 76 insertions(+), 51 deletions(-) create mode 100644 extensions/ql-vscode/src/view/common/Dropdown.tsx diff --git a/extensions/ql-vscode/src/view/common/Dropdown.tsx b/extensions/ql-vscode/src/view/common/Dropdown.tsx new file mode 100644 index 000000000..8e192f7a7 --- /dev/null +++ b/extensions/ql-vscode/src/view/common/Dropdown.tsx @@ -0,0 +1,21 @@ +import styled from "styled-components"; + +/** + * Styled to look like a `VSCodeDropdown`. + * + * The reason for doing this is that `VSCodeDropdown` doesn't handle fitting into + * available space and truncating content, and this leads to breaking the + * `VSCodeDataGrid` layout. This version using `select` directly will truncate the + * content as necessary and fit into whatever space is available. + * See https://github.com/github/vscode-codeql/pull/2582#issuecomment-1622164429 + * for more info on the problem and other potential solutions. + */ +export const Dropdown = styled.select` + width: 100%; + height: calc(var(--input-height) * 1px); + background: var(--dropdown-background); + color: var(--foreground); + font-family: var(--font-family); + border: none; + padding: 2px 6px 2px 8px; +`; diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx index dac3d28e8..b507f7825 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx @@ -1,13 +1,8 @@ import * as React from "react"; -import { useCallback, useEffect } from "react"; -import styled from "styled-components"; -import { VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react"; +import { ChangeEvent, useCallback, useEffect } from "react"; import type { ModeledMethod } from "../../data-extensions-editor/modeled-method"; - -const Dropdown = styled(VSCodeDropdown)` - width: 100%; -`; +import { Dropdown } from "../common/Dropdown"; type Props = { kinds: Array; @@ -19,7 +14,7 @@ type Props = { export const KindInput = ({ kinds, value, disabled, onChange }: Props) => { const handleInput = useCallback( - (e: InputEvent) => { + (e: ChangeEvent) => { const target = e.target as HTMLSelectElement; onChange(target.value as ModeledMethod["kind"]); @@ -38,12 +33,16 @@ export const KindInput = ({ kinds, value, disabled, onChange }: Props) => { }, [value, kinds, onChange]); return ( - - {kinds.map((kind) => ( - - {kind} - - ))} + + {!disabled && ( + <> + {kinds.map((kind) => ( + + ))} + + )} ); }; diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 386630581..2d43b0b06 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -2,12 +2,10 @@ import { VSCodeCheckbox, VSCodeDataGridCell, VSCodeDataGridRow, - VSCodeDropdown, VSCodeLink, - VSCodeOption, } from "@vscode/webview-ui-toolkit/react"; import * as React from "react"; -import { useCallback, useMemo } from "react"; +import { ChangeEvent, useCallback, useMemo } from "react"; import styled from "styled-components"; import { vscode } from "../vscode-api"; @@ -20,10 +18,7 @@ import { import { KindInput } from "./KindInput"; import { extensiblePredicateDefinitions } from "../../data-extensions-editor/predicates"; import { Mode } from "../../data-extensions-editor/shared/mode"; - -const Dropdown = styled(VSCodeDropdown)` - width: 100%; -`; +import { Dropdown } from "../common/Dropdown"; const ApiOrMethodCell = styled(VSCodeDataGridCell)` display: flex; @@ -66,9 +61,7 @@ export const MethodRow = ({ }, [externalApiUsage.methodParameters]); const handleTypeInput = useCallback( - (e: InputEvent) => { - const target = e.target as HTMLSelectElement; - + (e: ChangeEvent) => { let newProvenance: Provenance = "manual"; if (modeledMethod?.provenance === "df-generated") { newProvenance = "df-manual"; @@ -82,14 +75,14 @@ export const MethodRow = ({ output: "ReturnType", kind: "value", ...modeledMethod, - type: target.value as ModeledMethodType, + type: e.target.value as ModeledMethodType, provenance: newProvenance, }); }, [onChange, externalApiUsage, modeledMethod, argumentsList], ); const handleInputInput = useCallback( - (e: InputEvent) => { + (e: ChangeEvent) => { if (!modeledMethod) { return; } @@ -104,7 +97,7 @@ export const MethodRow = ({ [onChange, externalApiUsage, modeledMethod], ); const handleOutputInput = useCallback( - (e: InputEvent) => { + (e: ChangeEvent) => { if (!modeledMethod) { return; } @@ -172,44 +165,56 @@ export const MethodRow = ({ - Unmodeled - Source - Sink - Flow summary - Neutral + {showModelTypeCell && ( + <> + + + + + + + )} - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} + {showInputCell && ( + <> + + {argumentsList.map((argument, index) => ( + + ))} + + )} - ReturnValue - Argument[this] - {argumentsList.map((argument, index) => ( - - Argument[{index}]: {argument} - - ))} + {showOutputCell && ( + <> + + + {argumentsList.map((argument, index) => ( + + ))} + + )} From 903e8c668813fc0f608b7eec7b3b3d0bfefe2126 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 6 Jul 2023 11:20:48 +0100 Subject: [PATCH 6/8] Convert dropdown to a full component --- .../ql-vscode/src/view/common/Dropdown.tsx | 51 ++++++++--- .../view/data-extensions-editor/KindInput.tsx | 24 +++--- .../view/data-extensions-editor/MethodRow.tsx | 84 ++++++++++--------- 3 files changed, 96 insertions(+), 63 deletions(-) diff --git a/extensions/ql-vscode/src/view/common/Dropdown.tsx b/extensions/ql-vscode/src/view/common/Dropdown.tsx index 8e192f7a7..9eabc6a06 100644 --- a/extensions/ql-vscode/src/view/common/Dropdown.tsx +++ b/extensions/ql-vscode/src/view/common/Dropdown.tsx @@ -1,16 +1,8 @@ +import * as React from "react"; +import { ChangeEvent } from "react"; import styled from "styled-components"; -/** - * Styled to look like a `VSCodeDropdown`. - * - * The reason for doing this is that `VSCodeDropdown` doesn't handle fitting into - * available space and truncating content, and this leads to breaking the - * `VSCodeDataGrid` layout. This version using `select` directly will truncate the - * content as necessary and fit into whatever space is available. - * See https://github.com/github/vscode-codeql/pull/2582#issuecomment-1622164429 - * for more info on the problem and other potential solutions. - */ -export const Dropdown = styled.select` +const StyledDropdown = styled.select` width: 100%; height: calc(var(--input-height) * 1px); background: var(--dropdown-background); @@ -19,3 +11,40 @@ export const Dropdown = styled.select` border: none; padding: 2px 6px 2px 8px; `; + +type Props = { + value: string | undefined; + options: Array<{ value: string; label: string }>; + disabled?: boolean; + onChange: (event: ChangeEvent) => void; +}; + +/** + * A dropdown implementation styled to look like `VSCodeDropdown`. + * + * The reason for doing this is that `VSCodeDropdown` doesn't handle fitting into + * available space and truncating content, and this leads to breaking the + * `VSCodeDataGrid` layout. This version using `select` directly will truncate the + * content as necessary and fit into whatever space is available. + * See https://github.com/github/vscode-codeql/pull/2582#issuecomment-1622164429 + * for more info on the problem and other potential solutions. + */ +export function Dropdown({ value, options, disabled, onChange }: Props) { + return ( + + {!disabled && ( + <> + {options.map((option) => ( + + ))} + + )} + + ); +} diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx index b507f7825..43fdf1a4d 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/KindInput.tsx @@ -1,5 +1,5 @@ import * as React from "react"; -import { ChangeEvent, useCallback, useEffect } from "react"; +import { ChangeEvent, useCallback, useEffect, useMemo } from "react"; import type { ModeledMethod } from "../../data-extensions-editor/modeled-method"; import { Dropdown } from "../common/Dropdown"; @@ -13,6 +13,11 @@ type Props = { }; export const KindInput = ({ kinds, value, disabled, onChange }: Props) => { + const options = useMemo( + () => kinds.map((kind) => ({ value: kind, label: kind })), + [kinds], + ); + const handleInput = useCallback( (e: ChangeEvent) => { const target = e.target as HTMLSelectElement; @@ -33,16 +38,11 @@ export const KindInput = ({ kinds, value, disabled, onChange }: Props) => { }, [value, kinds, onChange]); return ( - - {!disabled && ( - <> - {kinds.map((kind) => ( - - ))} - - )} - + ); }; diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 2d43b0b06..075bca835 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -141,10 +141,44 @@ export const MethodRow = ({ const showModelTypeCell = !externalApiUsage.supported || (modeledMethod && modeledMethod?.type !== "none"); + const modelTypeOptions = useMemo( + () => [ + { value: "none", label: "Unmodeled" }, + { value: "source", label: "Source" }, + { value: "sink", label: "Sink" }, + { value: "summary", label: "Flow summary" }, + { value: "neutral", label: "Neutral" }, + ], + [], + ); + const showInputCell = modeledMethod?.type && ["sink", "summary"].includes(modeledMethod?.type); + const inputOptions = useMemo( + () => [ + { value: "Argument[this]", label: "Argument[this]" }, + ...argumentsList.map((argument, index) => ({ + value: `Argument[${index}]`, + label: `Argument[${index}]: ${argument}`, + })), + ], + [argumentsList], + ); + const showOutputCell = modeledMethod?.type && ["source", "summary"].includes(modeledMethod?.type); + const outputOptions = useMemo( + () => [ + { value: "ReturnValue", label: "ReturnValue" }, + { value: "Argument[this]", label: "Argument[this]" }, + ...argumentsList.map((argument, index) => ({ + value: `Argument[${index}]`, + label: `Argument[${index}]: ${argument}`, + })), + ], + [argumentsList], + ); + const showKindCell = predicate?.supportedKinds; return ( @@ -165,62 +199,32 @@ export const MethodRow = ({ - {showModelTypeCell && ( - <> - - - - - - - )} - + /> - {showInputCell && ( - <> - - {argumentsList.map((argument, index) => ( - - ))} - - )} - + /> - {showOutputCell && ( - <> - - - {argumentsList.map((argument, index) => ( - - ))} - - )} - + /> From afc9635d432066be3393d4f26be52bbdce91abf1 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 6 Jul 2023 15:25:27 +0100 Subject: [PATCH 7/8] Use --vscode styles --- extensions/ql-vscode/src/view/common/Dropdown.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/view/common/Dropdown.tsx b/extensions/ql-vscode/src/view/common/Dropdown.tsx index 9eabc6a06..b281325a5 100644 --- a/extensions/ql-vscode/src/view/common/Dropdown.tsx +++ b/extensions/ql-vscode/src/view/common/Dropdown.tsx @@ -5,9 +5,8 @@ import styled from "styled-components"; const StyledDropdown = styled.select` width: 100%; height: calc(var(--input-height) * 1px); - background: var(--dropdown-background); - color: var(--foreground); - font-family: var(--font-family); + background: var(--vscode-dropdown-background); + color: var(--vscode-foreground); border: none; padding: 2px 6px 2px 8px; `; From a07e829bf1df5836af0579cdac1840794f4b09b7 Mon Sep 17 00:00:00 2001 From: Robert Date: Thu, 6 Jul 2023 15:25:39 +0100 Subject: [PATCH 8/8] Make view link not wrap --- .../ql-vscode/src/view/data-extensions-editor/MethodRow.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx index 075bca835..6f1a9ef4b 100644 --- a/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/data-extensions-editor/MethodRow.tsx @@ -35,6 +35,10 @@ const UsagesButton = styled.button` cursor: pointer; `; +const ViewLink = styled(VSCodeLink)` + white-space: nowrap; +`; + type Props = { externalApiUsage: ExternalApiUsage; modeledMethod: ModeledMethod | undefined; @@ -195,7 +199,7 @@ export const MethodRow = ({ {externalApiUsage.usages.length} )} - View + View