Merge pull request #2524 from github/koesie10/grouping-improvements

Improve grouping of libraries
2023-06-20 15:22:33 +02:00
parent 23173bf441 2800ccb74c
commit 8fb1229c49
9 changed files with 249 additions and 52 deletions
--- a/extensions/ql-vscode/src/data-extensions-editor/bqrs.ts
+++ b/extensions/ql-vscode/src/data-extensions-editor/bqrs.ts
@@ -47,17 +47,5 @@ export function decodeBqrsToExternalApiUsages(
    method.usages.push(usage);
  });

-  const externalApiUsages = Array.from(methodsByApiName.values());
-  externalApiUsages.sort((a, b) => {
-    // Sort first by supported, putting unmodeled methods first.
-    if (a.supported && !b.supported) {
-      return 1;
-    }
-    if (!a.supported && b.supported) {
-      return -1;
-    }
-    // Then sort by number of usages descending
-    return b.usages.length - a.usages.length;
-  });
-  return externalApiUsages;
+  return Array.from(methodsByApiName.values());
 }
--- a/extensions/ql-vscode/src/pure/word.ts
+++ b/extensions/ql-vscode/src/pure/word.ts
@@ -7,8 +7,9 @@ export function pluralize(
  numItems: number | undefined,
  singular: string,
  plural: string,
+  numberFormatter: (value: number) => string = (value) => value.toString(),
 ): string {
  return numItems !== undefined
-    ? `${numItems} ${numItems === 1 ? singular : plural}`
+    ? `${numberFormatter(numItems)} ${numItems === 1 ? singular : plural}`
    : "";
 }
--- a/extensions/ql-vscode/src/view/data-extensions-editor/DataExtensionsEditor.tsx
+++ b/extensions/ql-vscode/src/view/data-extensions-editor/DataExtensionsEditor.tsx
@@ -16,6 +16,7 @@ import { basename } from "../common/path";
 import { ViewTitle } from "../common";
 import { DataExtensionEditorViewState } from "../../data-extensions-editor/shared/view-state";
 import { ModeledMethodsList } from "./ModeledMethodsList";
+import { percentFormatter } from "./formatters";

 const DataExtensionsEditorContainer = styled.div`
  margin-top: 1rem;
@@ -213,8 +214,12 @@ export function DataExtensionsEditor({
                )}
              </>
            )}
-            <div>{modeledPercentage.toFixed(2)}% modeled</div>
-            <div>{unModeledPercentage.toFixed(2)}% unmodeled</div>
+            <div>
+              {percentFormatter.format(modeledPercentage / 100)} modeled
+            </div>
+            <div>
+              {percentFormatter.format(unModeledPercentage / 100)} unmodeled
+            </div>
          </DetailsContainer>

          <EditorContainer>
--- a/extensions/ql-vscode/src/view/data-extensions-editor/LibraryRow.tsx
+++ b/extensions/ql-vscode/src/view/data-extensions-editor/LibraryRow.tsx
@@ -0,0 +1,125 @@
+import * as React from "react";
+import { useCallback, useMemo, useState } from "react";
+import styled from "styled-components";
+import { ExternalApiUsage } from "../../data-extensions-editor/external-api-usage";
+import { ModeledMethod } from "../../data-extensions-editor/modeled-method";
+import { pluralize } from "../../pure/word";
+import { ModeledMethodDataGrid } from "./ModeledMethodDataGrid";
+import { calculateModeledPercentage } from "./modeled";
+import { decimalFormatter, percentFormatter } from "./formatters";
+import { Codicon } from "../common";
+
+const LibraryContainer = styled.div`
+  margin-bottom: 1rem;
+`;
+
+const TitleContainer = styled.button`
+  display: flex;
+  gap: 0.5em;
+  align-items: center;
+  width: 100%;
+  font-size: 1.2em;
+  font-weight: bold;
+
+  color: var(--vscode-editor-foreground);
+  background-color: transparent;
+  border: none;
+  cursor: pointer;
+`;
+
+const StatusContainer = styled.div`
+  display: flex;
+  gap: 1em;
+  align-items: center;
+
+  margin-top: 0.5em;
+  margin-bottom: 0.5em;
+  margin-left: 1em;
+`;
+
+type Props = {
+  libraryName: string;
+  externalApiUsages: ExternalApiUsage[];
+  modeledMethods: Record<string, ModeledMethod>;
+  onChange: (
+    externalApiUsage: ExternalApiUsage,
+    modeledMethod: ModeledMethod,
+  ) => void;
+};
+
+export const LibraryRow = ({
+  libraryName,
+  externalApiUsages,
+  modeledMethods,
+  onChange,
+}: Props) => {
+  const modeledPercentage = useMemo(() => {
+    return calculateModeledPercentage(externalApiUsages);
+  }, [externalApiUsages]);
+
+  const [isExpanded, setExpanded] = useState(modeledPercentage < 100);
+
+  const toggleExpanded = useCallback(async () => {
+    setExpanded((oldIsExpanded) => !oldIsExpanded);
+  }, []);
+
+  const usagesCount = useMemo(() => {
+    return externalApiUsages.reduce((acc, curr) => acc + curr.usages.length, 0);
+  }, [externalApiUsages]);
+
+  return (
+    <LibraryContainer>
+      <TitleContainer onClick={toggleExpanded} aria-expanded={isExpanded}>
+        {isExpanded ? (
+          <Codicon name="chevron-down" label="Collapse" />
+        ) : (
+          <Codicon name="chevron-right" label="Expand" />
+        )}
+        {libraryName}
+        {isExpanded ? null : (
+          <>
+            {" "}
+            (
+            {pluralize(
+              externalApiUsages.length,
+              "method",
+              "methods",
+              decimalFormatter.format.bind(decimalFormatter),
+            )}
+            , {percentFormatter.format(modeledPercentage / 100)} modeled)
+          </>
+        )}
+      </TitleContainer>
+      {isExpanded && (
+        <>
+          <StatusContainer>
+            <div>
+              {pluralize(
+                externalApiUsages.length,
+                "method",
+                "methods",
+                decimalFormatter.format.bind(decimalFormatter),
+              )}
+            </div>
+            <div>
+              {pluralize(
+                usagesCount,
+                "usage",
+                "usages",
+                decimalFormatter.format.bind(decimalFormatter),
+              )}
+            </div>
+            <div>
+              {percentFormatter.format(modeledPercentage / 100)} modeled
+            </div>
+          </StatusContainer>
+          <ModeledMethodDataGrid
+            externalApiUsages={externalApiUsages}
+            modeledMethods={modeledMethods}
+            onChange={onChange}
+          />
+        </>
+      )}
+    </LibraryContainer>
+  );
+};
--- a/extensions/ql-vscode/src/view/data-extensions-editor/ModeledMethodDataGrid.tsx
+++ b/extensions/ql-vscode/src/view/data-extensions-editor/ModeledMethodDataGrid.tsx
@@ -7,6 +7,7 @@ import {
 import { MethodRow } from "./MethodRow";
 import { ExternalApiUsage } from "../../data-extensions-editor/external-api-usage";
 import { ModeledMethod } from "../../data-extensions-editor/modeled-method";
+import { useMemo } from "react";

 type Props = {
  externalApiUsages: ExternalApiUsage[];
@@ -22,6 +23,22 @@ export const ModeledMethodDataGrid = ({
  modeledMethods,
  onChange,
 }: Props) => {
+  const sortedExternalApiUsages = useMemo(() => {
+    const sortedExternalApiUsages = [...externalApiUsages];
+    sortedExternalApiUsages.sort((a, b) => {
+      // Sort first by supported, putting unmodeled methods first.
+      if (a.supported && !b.supported) {
+        return 1;
+      }
+      if (!a.supported && b.supported) {
+        return -1;
+      }
+      // Then sort by number of usages descending
+      return b.usages.length - a.usages.length;
+    });
+    return sortedExternalApiUsages;
+  }, [externalApiUsages]);
+
  return (
    <VSCodeDataGrid>
      <VSCodeDataGridRow rowType="header">
@@ -47,7 +64,7 @@ export const ModeledMethodDataGrid = ({
          Kind
        </VSCodeDataGridCell>
      </VSCodeDataGridRow>
-      {externalApiUsages.map((externalApiUsage) => (
+      {sortedExternalApiUsages.map((externalApiUsage) => (
        <MethodRow
          key={externalApiUsage.signature}
          externalApiUsage={externalApiUsage}
--- a/extensions/ql-vscode/src/view/data-extensions-editor/ModeledMethodsList.tsx
+++ b/extensions/ql-vscode/src/view/data-extensions-editor/ModeledMethodsList.tsx
@@ -1,13 +1,9 @@
 import * as React from "react";
 import { useMemo } from "react";
-import styled from "styled-components";
 import { ExternalApiUsage } from "../../data-extensions-editor/external-api-usage";
 import { ModeledMethod } from "../../data-extensions-editor/modeled-method";
-import { ModeledMethodDataGrid } from "./ModeledMethodDataGrid";
-
-const LibraryContainer = styled.div`
-  margin-bottom: 1rem;
-`;
+import { calculateModeledPercentage } from "./modeled";
+import { LibraryRow } from "./LibraryRow";

 type Props = {
  externalApiUsages: ExternalApiUsage[];
@@ -35,20 +31,59 @@ export const ModeledMethodsList = ({
  }, [externalApiUsages]);

  const sortedLibraryNames = useMemo(() => {
-    return Object.keys(groupedByLibrary).sort();
+    return Object.keys(groupedByLibrary).sort((a, b) => {
+      const supportedPercentageA = calculateModeledPercentage(
+        groupedByLibrary[a],
+      );
+      const supportedPercentageB = calculateModeledPercentage(
+        groupedByLibrary[b],
+      );
+
+      // Sort first by supported percentage ascending
+      if (supportedPercentageA > supportedPercentageB) {
+        return 1;
+      }
+      if (supportedPercentageA < supportedPercentageB) {
+        return -1;
+      }
+
+      const numberOfUsagesA = groupedByLibrary[a].reduce(
+        (acc, curr) => acc + curr.usages.length,
+        0,
+      );
+      const numberOfUsagesB = groupedByLibrary[b].reduce(
+        (acc, curr) => acc + curr.usages.length,
+        0,
+      );
+
+      // If the number of usages is equal, sort by number of methods descending
+      if (numberOfUsagesA === numberOfUsagesB) {
+        const numberOfMethodsA = groupedByLibrary[a].length;
+        const numberOfMethodsB = groupedByLibrary[b].length;
+
+        // If the number of methods is equal, sort by library name ascending
+        if (numberOfMethodsA === numberOfMethodsB) {
+          return a.localeCompare(b);
+        }
+
+        return numberOfMethodsB - numberOfMethodsA;
+      }
+
+      // Then sort by number of usages descending
+      return numberOfUsagesB - numberOfUsagesA;
+    });
  }, [groupedByLibrary]);

  return (
    <>
      {sortedLibraryNames.map((libraryName) => (
-        <LibraryContainer key={libraryName}>
-          <h3>{libraryName}</h3>
-          <ModeledMethodDataGrid
-            externalApiUsages={groupedByLibrary[libraryName]}
-            modeledMethods={modeledMethods}
-            onChange={onChange}
-          />
-        </LibraryContainer>
+        <LibraryRow
+          key={libraryName}
+          libraryName={libraryName}
+          externalApiUsages={groupedByLibrary[libraryName]}
+          modeledMethods={modeledMethods}
+          onChange={onChange}
+        />
      ))}
    </>
  );
--- a/extensions/ql-vscode/src/view/data-extensions-editor/formatters.ts
+++ b/extensions/ql-vscode/src/view/data-extensions-editor/formatters.ts
@@ -0,0 +1,9 @@
+export const decimalFormatter = new Intl.NumberFormat("en-US", {
+  style: "decimal",
+  maximumFractionDigits: 2,
+});
+
+export const percentFormatter = new Intl.NumberFormat("en-US", {
+  style: "percent",
+  maximumFractionDigits: 2,
+});
--- a/extensions/ql-vscode/test/unit-tests/data-extensions-editor/bqrs.test.ts
+++ b/extensions/ql-vscode/test/unit-tests/data-extensions-editor/bqrs.test.ts
@@ -168,6 +168,26 @@ describe("decodeBqrsToExternalApiUsages", () => {
    // - Iterating over a map (as done by .values()) is guaranteed to be in insertion order
    // - Sorting the array of usages is guaranteed to be a stable sort
    expect(decodeBqrsToExternalApiUsages(chunk)).toEqual([
+      {
+        signature: "java.io.PrintStream#println(String)",
+        packageName: "java.io",
+        typeName: "PrintStream",
+        methodName: "println",
+        methodParameters: "(String)",
+        supported: true,
+        usages: [
+          {
+            label: "println(...)",
+            url: {
+              uri: "file:/home/runner/work/sql2o-example/sql2o-example/src/main/java/org/example/HelloController.java",
+              startLine: 29,
+              startColumn: 9,
+              endLine: 29,
+              endColumn: 49,
+            },
+          },
+        ],
+      },
      {
        signature:
          "org.springframework.boot.SpringApplication#run(Class,String[])",
@@ -279,26 +299,6 @@ describe("decodeBqrsToExternalApiUsages", () => {
          },
        ],
      },
-      {
-        signature: "java.io.PrintStream#println(String)",
-        packageName: "java.io",
-        typeName: "PrintStream",
-        methodName: "println",
-        methodParameters: "(String)",
-        supported: true,
-        usages: [
-          {
-            label: "println(...)",
-            url: {
-              uri: "file:/home/runner/work/sql2o-example/sql2o-example/src/main/java/org/example/HelloController.java",
-              startLine: 29,
-              startColumn: 9,
-              endLine: 29,
-              endColumn: 49,
-            },
-          },
-        ],
-      },
      {
        signature: "org.sql2o.Sql2o#Sql2o(String,String,String)",
        packageName: "org.sql2o",
--- a/extensions/ql-vscode/test/unit-tests/pure/word.test.ts
+++ b/extensions/ql-vscode/test/unit-tests/pure/word.test.ts
@@ -14,5 +14,22 @@ describe("word helpers", () => {
    it("should return the empty string if the number is undefined", () => {
      expect(pluralize(undefined, "thing", "things")).toBe("");
    });
+    it("should return an unformatted number when no formatter is specified", () => {
+      expect(pluralize(1_000_000, "thing", "things")).toBe("1000000 things");
+    });
+    it("should return a formatted number when a formatter is specified", () => {
+      const formatter = new Intl.NumberFormat("en-US", {
+        style: "decimal",
+      });
+
+      expect(
+        pluralize(
+          1_000_000,
+          "thing",
+          "things",
+          formatter.format.bind(formatter),
+        ),
+      ).toBe("1,000,000 things");
+    });
  });
 });