Merge pull request #2990 from github/robertbrignull/model-table-alignment

Use custom grid element instead of VSCodeDataGrid
2023-10-19 09:57:26 +01:00
parent df55e039a1 c88ecf76aa
commit 947f495d0b
7 changed files with 232 additions and 91 deletions
--- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx
+++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx
@@ -5,7 +5,6 @@ import { Meta, StoryFn } from "@storybook/react";
 import { MethodRow as MethodRowComponent } from "../../view/model-editor/MethodRow";
 import { CallClassification, Method } from "../../model-editor/method";
 import { ModeledMethod } from "../../model-editor/modeled-method";
-import { VSCodeDataGrid } from "@vscode/webview-ui-toolkit/react";
 import {
  MULTIPLE_MODELS_GRID_TEMPLATE_COLUMNS,
  SINGLE_MODEL_GRID_TEMPLATE_COLUMNS,
@@ -13,6 +12,7 @@ import {
 import { ModelEditorViewState } from "../../model-editor/shared/view-state";
 import { createMockExtensionPack } from "../../../test/factories/model-editor/extension-pack";
 import { Mode } from "../../model-editor/shared/mode";
+import { DataGrid } from "../../view/common/DataGrid";

 export default {
  title: "CodeQL Model Editor/Method Row",
@@ -24,9 +24,9 @@ const Template: StoryFn<typeof MethodRowComponent> = (args) => {
    ? MULTIPLE_MODELS_GRID_TEMPLATE_COLUMNS
    : SINGLE_MODEL_GRID_TEMPLATE_COLUMNS;
  return (
-    <VSCodeDataGrid gridTemplateColumns={gridTemplateColumns}>
+    <DataGrid gridTemplateColumns={gridTemplateColumns}>
      <MethodRowComponent {...args} />
-    </VSCodeDataGrid>
+    </DataGrid>
  );
 };

--- a/extensions/ql-vscode/src/view/common/DataGrid.tsx
+++ b/extensions/ql-vscode/src/view/common/DataGrid.tsx
@@ -0,0 +1,127 @@
+import * as React from "react";
+import { ReactNode, forwardRef } from "react";
+import { styled } from "styled-components";
+
+/*
+ * A drop-in replacement for the VSCodeDataGrid family of components.
+ *
+ * The difference is that the `display: grid` styling is applied to `DataGrid`, whereas
+ * in the VS Code version that styling is applied to `VSCodeDataGridRow`. This gives
+ * column alignment across rows in situation with dynamic contents. It also allows
+ * for cells to span multiple rows and all the other features of data grids.
+ */
+
+const StyledDataGrid = styled.div<{ $gridTemplateColumns: string | number }>`
+  display: grid;
+  grid-template-columns: ${(props) => props.$gridTemplateColumns};
+  box-sizing: border-box;
+  width: 100%;
+  background: transparent;
+`;
+
+interface DataGridProps {
+  gridTemplateColumns: string;
+  children: ReactNode;
+}
+
+/**
+ * The top level for a grid systemm that will contain `DataGridRow` and `DataGridCell` components.
+ *
+ * See https://developer.mozilla.org/en-US/docs/Web/CSS/grid-template-columns for how to use `gridTemplateColumns`.
+ */
+export function DataGrid({ gridTemplateColumns, children }: DataGridProps) {
+  return (
+    <StyledDataGrid $gridTemplateColumns={gridTemplateColumns}>
+      {children}
+    </StyledDataGrid>
+  );
+}
+
+const StyledDataGridRow = styled.div<{ $focused?: boolean }>`
+  display: contents;
+
+  &:hover > * {
+    background-color: var(--list-hover-background);
+  }
+
+  & > * {
+    // Use !important to override the background color set by the hover state
+    background-color: ${(props) =>
+      props.$focused
+        ? "var(--vscode-editor-selectionBackground) !important"
+        : "inherit"};
+  }
+`;
+
+interface DataGridRowProps {
+  focused?: boolean;
+  children: ReactNode;
+  "data-testid"?: string;
+}
+
+/**
+ * Optional component for encompasing a single row in a `DataGrid`.
+ * Implements hover and focus states that highlight all cells in the row.
+ *
+ * Note that using this component is not mandatory. Cells can be placed directly
+ * inside a `DataGrid`. Feel free to skip this component if your cells do not
+ * line up into neat rows, or you do not need the hover and focus states.
+ */
+export const DataGridRow = forwardRef(
+  (
+    { focused, children, "data-testid": testId }: DataGridRowProps,
+    ref?: React.Ref<HTMLElement | undefined>,
+  ) => (
+    <StyledDataGridRow $focused={focused} ref={ref} data-testid={testId}>
+      {children}
+    </StyledDataGridRow>
+  ),
+);
+DataGridRow.displayName = "DataGridRow";
+
+const StyledDataGridCell = styled.div<{
+  $rowType: "default" | "header";
+  $gridRow?: string | number;
+  $gridColumn?: string | number;
+}>`
+  ${({ $rowType }) => ($rowType === "header" ? "font-weight: 600;" : "")}
+  ${({ $gridRow }) => ($gridRow ? `grid-row: ${$gridRow};` : "")}
+  ${({ $gridColumn }) => ($gridColumn ? `grid-column: ${$gridColumn};` : "")}
+  padding: 4px 12px;
+`;
+
+interface DataGridCellProps {
+  rowType?: "default" | "header";
+  gridRow?: string | number;
+  gridColumn?: string | number;
+  className?: string;
+  children: ReactNode;
+}
+
+/**
+ * A cell in a `DataGrid`.
+ *
+ * By default, the position of cells in the grid is determined by the order in which
+ * they appear in the DOM. Cells will fill up the current row and then move on to the
+ * next row. This can be overridden using the `gridRow` and `gridColumn` to place
+ * cells anywhere within the grid. You can also configure cells to span multiple rows
+ * or columns. See https://developer.mozilla.org/en-US/docs/Web/CSS/grid-column.
+ */
+export function DataGridCell({
+  rowType = "default",
+  gridRow,
+  gridColumn,
+  className,
+  children,
+}: DataGridCellProps) {
+  return (
+    <StyledDataGridCell
+      $rowType={rowType}
+      $gridRow={gridRow}
+      $gridColumn={gridColumn}
+      className={className}
+    >
+      {children}
+    </StyledDataGridCell>
+  );
+}
--- a/extensions/ql-vscode/src/view/model-editor/HiddenMethodsRow.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/HiddenMethodsRow.tsx
@@ -1,35 +1,37 @@
-import {
-  VSCodeDataGridCell,
-  VSCodeDataGridRow,
-} from "@vscode/webview-ui-toolkit/react";
 import * as React from "react";
 import { styled } from "styled-components";
 import { pluralize } from "../../common/word";
+import { DataGridCell, DataGridRow } from "../common/DataGrid";
+import { ModelEditorViewState } from "../../model-editor/shared/view-state";

-const HiddenMethodsCell = styled(VSCodeDataGridCell)`
+const HiddenMethodsCell = styled(DataGridCell)`
  text-align: center;
 `;

 interface Props {
  numHiddenMethods: number;
  someMethodsAreVisible: boolean;
+  viewState: ModelEditorViewState;
 }

 export function HiddenMethodsRow({
  numHiddenMethods,
  someMethodsAreVisible,
+  viewState,
 }: Props) {
  if (numHiddenMethods === 0) {
    return null;
  }

+  const gridColumn = viewState.showMultipleModels ? "span 6" : "span 5";
+
  return (
-    <VSCodeDataGridRow>
-      <HiddenMethodsCell gridColumn="span 5">
+    <DataGridRow>
+      <HiddenMethodsCell gridColumn={gridColumn}>
        {someMethodsAreVisible && "And "}
        {pluralize(numHiddenMethods, "method", "methods")} modeled in other
        CodeQL packs
      </HiddenMethodsCell>
-    </VSCodeDataGridRow>
+    </DataGridRow>
  );
 }
--- a/extensions/ql-vscode/src/view/model-editor/MethodName.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/MethodName.tsx
@@ -4,6 +4,7 @@ import { Method } from "../../model-editor/method";

 const Name = styled.span`
  font-family: var(--vscode-editor-font-family);
+  word-break: break-all;
 `;

 export const MethodName = (method: Method): JSX.Element => {
--- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
@@ -1,7 +1,5 @@
 import {
  VSCodeButton,
-  VSCodeDataGridCell,
-  VSCodeDataGridRow,
  VSCodeLink,
  VSCodeProgressRing,
 } from "@vscode/webview-ui-toolkit/react";
@@ -25,8 +23,9 @@ import { ModelOutputDropdown } from "./ModelOutputDropdown";
 import { ModelEditorViewState } from "../../model-editor/shared/view-state";
 import { Codicon } from "../common";
 import { canAddNewModeledMethod } from "../../model-editor/shared/multiple-modeled-methods";
+import { DataGridCell, DataGridRow } from "../common/DataGrid";

-const MultiModelColumn = styled(VSCodeDataGridCell)`
+const MultiModelColumn = styled(DataGridCell)`
  display: flex;
  flex-direction: column;
  gap: 0.5em;
@@ -63,11 +62,6 @@ const CodiconRow = styled(VSCodeButton)`
  align-items: center;
 `;

-const DataGridRow = styled(VSCodeDataGridRow)<{ focused?: boolean }>`
-  outline: ${(props) =>
-    props.focused ? "1px solid var(--vscode-focusBorder)" : "none"};
-`;
-
 export type MethodRowProps = {
  method: Method;
  methodCanBeModeled: boolean;
@@ -168,7 +162,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
        ref={ref}
        focused={revealedMethodSignature === method.signature}
      >
-        <VSCodeDataGridCell gridColumn={1}>
+        <DataGridCell>
          <ApiOrMethodRow>
            <ModelingStatusIndicator status={modelingStatus} />
            <MethodClassifications method={method} />
@@ -181,33 +175,33 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
            <ViewLink onClick={jumpToMethod}>View</ViewLink>
            {props.modelingInProgress && <ProgressRing />}
          </ApiOrMethodRow>
-        </VSCodeDataGridCell>
+        </DataGridCell>
        {props.modelingInProgress && (
          <>
-            <VSCodeDataGridCell gridColumn={2}>
+            <DataGridCell>
              <InProgressDropdown />
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell gridColumn={3}>
+            </DataGridCell>
+            <DataGridCell>
              <InProgressDropdown />
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell gridColumn={4}>
+            </DataGridCell>
+            <DataGridCell>
              <InProgressDropdown />
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell gridColumn={5}>
+            </DataGridCell>
+            <DataGridCell>
              <InProgressDropdown />
-            </VSCodeDataGridCell>
+            </DataGridCell>
            {viewState.showMultipleModels && (
-              <VSCodeDataGridCell gridColumn={6}>
+              <DataGridCell>
                <CodiconRow appearance="icon" disabled={true}>
                  <Codicon name="add" label="Add new model" />
                </CodiconRow>
-              </VSCodeDataGridCell>
+              </DataGridCell>
            )}
          </>
        )}
        {!props.modelingInProgress && (
          <>
-            <MultiModelColumn gridColumn={2}>
+            <MultiModelColumn>
              {modeledMethods.map((modeledMethod, index) => (
                <ModelTypeDropdown
                  key={index}
@@ -217,7 +211,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
                />
              ))}
            </MultiModelColumn>
-            <MultiModelColumn gridColumn={3}>
+            <MultiModelColumn>
              {modeledMethods.map((modeledMethod, index) => (
                <ModelInputDropdown
                  key={index}
@@ -227,7 +221,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
                />
              ))}
            </MultiModelColumn>
-            <MultiModelColumn gridColumn={4}>
+            <MultiModelColumn>
              {modeledMethods.map((modeledMethod, index) => (
                <ModelOutputDropdown
                  key={index}
@@ -237,7 +231,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
                />
              ))}
            </MultiModelColumn>
-            <MultiModelColumn gridColumn={5}>
+            <MultiModelColumn>
              {modeledMethods.map((modeledMethod, index) => (
                <ModelKindDropdown
                  key={index}
@@ -248,7 +242,7 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
              ))}
            </MultiModelColumn>
            {viewState.showMultipleModels && (
-              <MultiModelColumn gridColumn={6}>
+              <MultiModelColumn>
                {modeledMethods.map((_, index) =>
                  index === modeledMethods.length - 1 ? (
                    <CodiconRow
@@ -298,7 +292,7 @@ const UnmodelableMethodRow = forwardRef<
      ref={ref}
      focused={revealedMethodSignature === method.signature}
    >
-      <VSCodeDataGridCell gridColumn={1}>
+      <DataGridCell>
        <ApiOrMethodRow>
          <ModelingStatusIndicator status="saved" />
          <MethodName {...props.method} />
@@ -310,10 +304,8 @@ const UnmodelableMethodRow = forwardRef<
          <ViewLink onClick={jumpToMethod}>View</ViewLink>
          <MethodClassifications method={method} />
        </ApiOrMethodRow>
-      </VSCodeDataGridCell>
-      <VSCodeDataGridCell gridColumn="span 4">
-        Method already modeled
-      </VSCodeDataGridCell>
+      </DataGridCell>
+      <DataGridCell gridColumn="span 4">Method already modeled</DataGridCell>
    </DataGridRow>
  );
 });
--- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx
@@ -1,9 +1,4 @@
 import * as React from "react";
-import {
-  VSCodeDataGrid,
-  VSCodeDataGridCell,
-  VSCodeDataGridRow,
-} from "@vscode/webview-ui-toolkit/react";
 import { MethodRow } from "./MethodRow";
 import { Method, canMethodBeModeled } from "../../model-editor/method";
 import { ModeledMethod } from "../../model-editor/modeled-method";
@@ -12,6 +7,7 @@ import { sortMethods } from "../../model-editor/shared/sorting";
 import { HiddenMethodsRow } from "./HiddenMethodsRow";
 import { ModelEditorViewState } from "../../model-editor/shared/view-state";
 import { ScreenReaderOnly } from "../common/ScreenReaderOnly";
+import { DataGrid, DataGridCell } from "../common/DataGrid";

 export const SINGLE_MODEL_GRID_TEMPLATE_COLUMNS =
  "0.5fr 0.125fr 0.125fr 0.125fr 0.125fr";
@@ -72,53 +68,44 @@ export const ModeledMethodDataGrid = ({
    : SINGLE_MODEL_GRID_TEMPLATE_COLUMNS;

  return (
-    <VSCodeDataGrid gridTemplateColumns={gridTemplateColumns}>
+    <DataGrid gridTemplateColumns={gridTemplateColumns}>
      {someMethodsAreVisible && (
        <>
-          <VSCodeDataGridRow rowType="header">
-            <VSCodeDataGridCell cellType="columnheader" gridColumn={1}>
-              API or method
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell cellType="columnheader" gridColumn={2}>
-              Model type
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell cellType="columnheader" gridColumn={3}>
-              Input
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell cellType="columnheader" gridColumn={4}>
-              Output
-            </VSCodeDataGridCell>
-            <VSCodeDataGridCell cellType="columnheader" gridColumn={5}>
-              Kind
-            </VSCodeDataGridCell>
-            {viewState.showMultipleModels && (
-              <VSCodeDataGridCell cellType="columnheader" gridColumn={6}>
-                <ScreenReaderOnly>Add or remove models</ScreenReaderOnly>
-              </VSCodeDataGridCell>
-            )}
-          </VSCodeDataGridRow>
-          {methodsWithModelability.map(({ method, methodCanBeModeled }) => {
-            const modeledMethods = modeledMethodsMap[method.signature] ?? [];
-            return (
-              <MethodRow
-                key={method.signature}
-                method={method}
-                methodCanBeModeled={methodCanBeModeled}
-                modeledMethods={modeledMethods}
-                methodIsUnsaved={modifiedSignatures.has(method.signature)}
-                modelingInProgress={inProgressMethods.has(method.signature)}
-                viewState={viewState}
-                revealedMethodSignature={revealedMethodSignature}
-                onChange={onChange}
-              />
-            );
-          })}
+          <DataGridCell rowType="header">API or method</DataGridCell>
+          <DataGridCell rowType="header">Model type</DataGridCell>
+          <DataGridCell rowType="header">Input</DataGridCell>
+          <DataGridCell rowType="header">Output</DataGridCell>
+          <DataGridCell rowType="header">Kind</DataGridCell>
+          {viewState.showMultipleModels && (
+            <DataGridCell rowType="header">
+              <ScreenReaderOnly>Add or remove models</ScreenReaderOnly>
+            </DataGridCell>
+          )}
+          {methodsWithModelability.map(
+            ({ method, methodCanBeModeled }, index) => {
+              const modeledMethods = modeledMethodsMap[method.signature] ?? [];
+              return (
+                <MethodRow
+                  key={method.signature}
+                  method={method}
+                  methodCanBeModeled={methodCanBeModeled}
+                  modeledMethods={modeledMethods}
+                  methodIsUnsaved={modifiedSignatures.has(method.signature)}
+                  modelingInProgress={inProgressMethods.has(method.signature)}
+                  viewState={viewState}
+                  revealedMethodSignature={revealedMethodSignature}
+                  onChange={onChange}
+                />
+              );
+            },
+          )}
        </>
      )}
      <HiddenMethodsRow
        numHiddenMethods={numHiddenMethods}
        someMethodsAreVisible={someMethodsAreVisible}
+        viewState={viewState}
      />
-    </VSCodeDataGrid>
+    </DataGrid>
  );
 };
--- a/extensions/ql-vscode/src/view/model-editor/tests/HiddenMethodsRow.spec.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/tests/HiddenMethodsRow.spec.tsx
@@ -1,11 +1,27 @@
 import * as React from "react";
 import { render, screen } from "@testing-library/react";
 import { HiddenMethodsRow } from "../HiddenMethodsRow";
+import { createMockExtensionPack } from "../../../../test/factories/model-editor/extension-pack";
+import { ModelEditorViewState } from "../../../model-editor/shared/view-state";
+import { Mode } from "../../../model-editor/shared/mode";

 describe(HiddenMethodsRow.name, () => {
+  const viewState: ModelEditorViewState = {
+    mode: Mode.Application,
+    showFlowGeneration: false,
+    showLlmButton: false,
+    showMultipleModels: false,
+    extensionPack: createMockExtensionPack(),
+    sourceArchiveAvailable: true,
+  };
+
  it("does not render with 0 hidden methods", () => {
    const { container } = render(
-      <HiddenMethodsRow numHiddenMethods={0} someMethodsAreVisible={true} />,
+      <HiddenMethodsRow
+        numHiddenMethods={0}
+        someMethodsAreVisible={true}
+        viewState={viewState}
+      />,
    );

    expect(container).toBeEmptyDOMElement();
@@ -13,7 +29,11 @@ describe(HiddenMethodsRow.name, () => {

  it("renders with 1 hidden methods and no visible methods", () => {
    render(
-      <HiddenMethodsRow numHiddenMethods={1} someMethodsAreVisible={false} />,
+      <HiddenMethodsRow
+        numHiddenMethods={1}
+        someMethodsAreVisible={false}
+        viewState={viewState}
+      />,
    );

    expect(
@@ -23,7 +43,11 @@ describe(HiddenMethodsRow.name, () => {

  it("renders with 1 hidden methods and visible methods", () => {
    render(
-      <HiddenMethodsRow numHiddenMethods={1} someMethodsAreVisible={true} />,
+      <HiddenMethodsRow
+        numHiddenMethods={1}
+        someMethodsAreVisible={true}
+        viewState={viewState}
+      />,
    );

    expect(
@@ -33,7 +57,11 @@ describe(HiddenMethodsRow.name, () => {

  it("renders with 3 hidden methods and no visible methods", () => {
    render(
-      <HiddenMethodsRow numHiddenMethods={3} someMethodsAreVisible={false} />,
+      <HiddenMethodsRow
+        numHiddenMethods={3}
+        someMethodsAreVisible={false}
+        viewState={viewState}
+      />,
    );

    expect(
@@ -43,7 +71,11 @@ describe(HiddenMethodsRow.name, () => {

  it("renders with 3 hidden methods and visible methods", () => {
    render(
-      <HiddenMethodsRow numHiddenMethods={3} someMethodsAreVisible={true} />,
+      <HiddenMethodsRow
+        numHiddenMethods={3}
+        someMethodsAreVisible={true}
+        viewState={viewState}
+      />,
    );

    expect(