fix: ReadOnlyProperties view

packages/open-workflow-diagram-editor/src/side-panel/Fields.tsx

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
+import { useI18n } from "@openworkflowspec/i18n";
 import type { DetailField } from "@/core/taskDetails";
-import { FieldControl } from "./FieldControls";
 
 export function SectionHeader({ label }: { label: string }) {
   return (
@@ -40,7 +40,7 @@ export function PropertyField({ label, field }: { label: string; field: DetailFi
     <div className="dec-sidebar-prop">
       <dt className="dec-sidebar-prop-label">{label}</dt>
       <dd className="dec-sidebar-prop-value">
-        <FieldControl field={field} />
+        <PropertyValue field={field} />
       </dd>
     </div>
   );
@@ -65,3 +65,33 @@ export function YamlField({ yaml, summary = "{...}" }: { yaml: string; summary?:
     </div>
   );
 }
+
+export function PropertyValue({ field }: { field: DetailField }) {
+  const { t } = useI18n();
+
+  switch (field.kind) {
+    case "array":
+      return (
+        <span className="dec-sidebar-value-shape">
+          {`${field.count} ${t(field.count === 1 ? "sidebar.field.item" : "sidebar.field.items")}`}
+        </span>
+      );
+
+    case "object":
+      return <span className="dec-sidebar-value-shape">{"{...}"}</span>;
+
+    /* A shell command or a script body, which can run to dozens of lines. Its own block,
+       capped and scrollable, so one long value cannot bury every row beneath it. */
+    case "long-string":
+      return <pre className="dec-sidebar-value-block">{field.value}</pre>;
+
+    default: {
+      const numeric = typeof field.value === "number";
+      return (
+        <span className={numeric ? "dec-sidebar-value numeric" : "dec-sidebar-value"}>
+          {String(field.value)}
+        </span>
+      );
+    }
+  }
+}

packages/open-workflow-diagram-editor/src/side-panel/SidePanel.css

@@ -185,6 +185,38 @@
   .dec-root.dark .dec-sidebar-prop-value {
     @apply dec:text-gray-100;
   }
+
+  .dec-root .dec-sidebar-value {
+    font-family: var(--dec-font-mono);
+    overflow-wrap: anywhere;
+  }
+
+  .dec-root .dec-sidebar-value.numeric {
+    font-variant-numeric: tabular-nums;
+  }
+
+  .dec-root .dec-sidebar-value-shape {
+    @apply dec:text-xs dec:text-gray-500 dec:italic;
+  }
+
+  .dec-root.dark .dec-sidebar-value-shape {
+    @apply dec:text-gray-400;
+  }
+
+  /* A shell command or a script body. Capped and scrollable so one thirty-line script cannot
+     bury every row beneath it. */
+  .dec-root .dec-sidebar-value-block {
+    @apply dec:mt-0.5 dec:max-h-32 dec:overflow-auto dec:rounded-sm dec:bg-gray-100
+      dec:px-2 dec:py-1.5 dec:text-xs dec:leading-relaxed dec:text-gray-800;
+    font-family: var(--dec-font-mono);
+    white-space: pre-wrap;
+    overflow-wrap: anywhere;
+  }
+
+  .dec-root.dark .dec-sidebar-value-block {
+    @apply dec:bg-gray-800/60 dec:text-gray-200;
+  }
+
   /* Stacked field (label above value) */
   .dec-root .dec-sidebar-stacked-field {
     @apply dec:py-1.5;
@@ -338,7 +370,7 @@
 
   .dec-root .dec-sidebar-error-field {
     @apply dec:text-sm dec:font-semibold dec:text-red-800;
-    font-family: ui-monospace, SFMono-Regular, Menlo, monospace;
+    font-family: var(--dec-font-mono);
   }
 
   .dec-root .dec-sidebar-error-field::after {

packages/open-workflow-diagram-editor/src/styles.css

@@ -49,6 +49,8 @@
     /* Z-index layering for edges and labels */
     --dec-zindex-edge-label-regular: 1000;
 
+    --dec-font-mono: ui-monospace, SFMono-Regular, Menlo, monospace;
+
     /* canvas */
     --dec-canvas-bg: #e9eef3;
     --dec-canvas-pattern: #8897ac;

packages/open-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx

@@ -46,14 +46,14 @@ describe("NodeDetailsView", () => {
     expect(screen.getByTestId("node-details")).toBeInTheDocument();
     expect(screen.getByText("Properties")).toBeInTheDocument();
     expect(screen.getByText("call")).toBeInTheDocument();
-    expect(screen.getByDisplayValue("http")).toBeInTheDocument();
+    expect(screen.getByText("http")).toBeInTheDocument();
     expect(screen.getByText("with.endpoint")).toBeInTheDocument();
-    expect(screen.getByDisplayValue("https://api.example.com")).toBeInTheDocument();
+    expect(screen.getByText("https://api.example.com")).toBeInTheDocument();
     expect(screen.getByText("then")).toBeInTheDocument();
-    expect(screen.getByDisplayValue("continue")).toBeInTheDocument();
+    expect(screen.getByText("continue")).toBeInTheDocument();
   });
 
-  it("renders number fields as number inputs", () => {
+  it("renders a number field as its literal value", () => {
     const node = makeNode({
       label: "step",
       task: {
@@ -66,14 +66,10 @@ describe("NodeDetailsView", () => {
     renderWithProviders(<NodeDetailsView node={node} />);
 
     expect(screen.getByText("with.retries")).toBeInTheDocument();
-
-    const input = screen.getByDisplayValue("42");
-
-    expect(input).toHaveAttribute("type", "number");
-    expect(input).toBeDisabled();
+    expect(screen.getByText("42")).toBeInTheDocument();
   });
 
-  it("renders boolean fields as switches", () => {
+  it("renders a boolean field as plain text", () => {
     const node = makeNode({
       label: "step",
       task: {
@@ -86,11 +82,7 @@ describe("NodeDetailsView", () => {
     renderWithProviders(<NodeDetailsView node={node} />);
 
     expect(screen.getByText("with.enabled")).toBeInTheDocument();
-
-    const switchControl = screen.getByRole("switch");
-
-    expect(switchControl).toBeChecked();
-    expect(switchControl).toBeDisabled();
+    expect(screen.getByText("true")).toBeInTheDocument();
   });
 
   it.each([
@@ -268,16 +260,15 @@ describe("NodeDetailsView", () => {
 
       expect(screen.getByText("Properties")).toBeInTheDocument();
       expect(screen.getByText("call")).toBeInTheDocument();
-      expect(screen.getByDisplayValue("http")).toBeInTheDocument();
+      expect(screen.getByText("http")).toBeInTheDocument();
       expect(screen.getByText("with.endpoint")).toBeInTheDocument();
-      expect(screen.getByDisplayValue("https://api.example.com")).toBeInTheDocument();
+      expect(screen.getByText("https://api.example.com")).toBeInTheDocument();
     });
 
-    it.each(modes)("renders property controls as disabled in %s mode", (_mode, isReadOnly) => {
-      renderWithProviders(<NodeDetailsView node={node} />, { isReadOnly });
+    it.each(modes)("renders no form controls in %s mode", (_mode, isReadOnly) => {
+      const { container } = renderWithProviders(<NodeDetailsView node={node} />, { isReadOnly });
 
-      expect(screen.getByDisplayValue("http")).toBeDisabled();
-      expect(screen.getByDisplayValue("https://api.example.com")).toBeDisabled();
+      expect(container.querySelector("input, textarea, select, [role='switch']")).toBeNull();
     });
   });
 });

packages/open-workflow-diagram-editor/tests/side-panel/ReadOnlyProperties.test.tsx

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2021-Present The Open Workflow Specification Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { describe, it, expect } from "vitest";
+import { screen } from "@testing-library/react";
+import { ReadOnlyProperties } from "../../src/side-panel/ReadOnlyProperties";
+import type { DetailField } from "../../src/core/taskDetails";
+import { renderWithProviders } from "../test-utils/render-helpers";
+
+const render = (...fields: DetailField[]) =>
+  renderWithProviders(<ReadOnlyProperties fields={fields} />);
+
+describe("ReadOnlyProperties", () => {
+  it("renders one labelled row per field", () => {
+    const { container } = render(
+      { path: "call", kind: "scalar", value: "http" },
+      { path: "with.endpoint", kind: "scalar", value: "https://api.example.com" },
+    );
+
+    expect(container.querySelectorAll(".dec-sidebar-prop")).toHaveLength(2);
+    expect(screen.getByText("call")).toBeInTheDocument();
+    expect(screen.getByText("with.endpoint")).toBeInTheDocument();
+  });
+
+  it.each([
+    ["scalar string", { path: "call", kind: "scalar", value: "http" }],
+    ["scalar number", { path: "with.port", kind: "scalar", value: 8080 }],
+    ["scalar boolean", { path: "fork.compete", kind: "scalar", value: true }],
+    ["enum", { path: "with.output", kind: "enum", value: "content", options: ["raw", "content"] }],
+    ["duration", { path: "timeout.after", kind: "duration", value: "PT5M" }],
+    ["runtime-expression", { path: "if", kind: "runtime-expression", value: "${ .ok }" }],
+    ["long-string", { path: "run.script.code", kind: "long-string", value: "echo hi" }],
+    ["array", { path: "switch", kind: "array", count: 3 }],
+    ["object", { path: "with.headers", kind: "object" }],
+  ] as Array<[string, DetailField]>)("renders no form control for a %s", (_label, field) => {
+    const { container } = render(field);
+
+    expect(container.querySelector("input, textarea, select, [role='switch']")).toBeNull();
+  });
+
+  it.each([
+    ["a string", { path: "call", kind: "scalar", value: "http" }, "http"],
+    ["a number", { path: "with.port", kind: "scalar", value: 8080 }, "8080"],
+    ["a boolean as plain text", { path: "fork.compete", kind: "scalar", value: true }, "true"],
+    [
+      "an enum",
+      { path: "with.output", kind: "enum", value: "content", options: ["raw"] },
+      "content",
+    ],
+    ["a duration", { path: "timeout.after", kind: "duration", value: "PT5M" }, "PT5M"],
+    ["an expression", { path: "if", kind: "runtime-expression", value: "${ .ok }" }, "${ .ok }"],
+  ] as Array<[string, DetailField, string]>)(
+    "renders %s as its literal value",
+    (_l, field, text) => {
+      render(field);
+
+      expect(screen.getByText(text)).toBeInTheDocument();
+    },
+  );
+
+  it("renders a long value in its own capped block", () => {
+    const code = "const total = items.reduce((a, b) => a + b, 0);\nreturn total;";
+    const { container } = render({ path: "run.script.code", kind: "long-string", value: code });
+
+    const block = container.querySelector(".dec-sidebar-value-block");
+    expect(block).toBeInTheDocument();
+    expect(block?.tagName).toBe("PRE");
+    expect(block).toHaveTextContent("return total;");
+  });
+
+  /* A count and a placeholder describe shape rather than anything the task literally says. */
+  it.each([
+    [1, "1 item"],
+    [3, "3 items"],
+  ])("summarises an array of %i as %s", (count, expected) => {
+    const { container } = render({ path: "switch", kind: "array", count });
+
+    expect(container.querySelector(".dec-sidebar-value-shape")).toHaveTextContent(expected);
+  });
+
+  it("renders an object as a shape placeholder", () => {
+    const { container } = render({ path: "with.headers", kind: "object" });
+
+    expect(container.querySelector(".dec-sidebar-value-shape")).toHaveTextContent("{...}");
+  });
+
+  it("renders nothing when the task has no fields", () => {
+    const { container } = render();
+
+    expect(container.querySelectorAll(".dec-sidebar-prop")).toHaveLength(0);
+  });
+});