fix: address mobile workflow PR feedback
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
Redesign the workflow editor mobile surface with a graph outline, mobile add flow, and first-class workflow settings destinations.
|
||||
|
||||
@@ -549,6 +549,48 @@
|
||||
display: none;
|
||||
}
|
||||
|
||||
.wf-layout-toggle {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
gap: var(--space-xs);
|
||||
width: fit-content;
|
||||
min-height: 30px;
|
||||
padding: var(--space-xs) var(--space-sm);
|
||||
border: 1px solid var(--border);
|
||||
border-radius: var(--radius-sm);
|
||||
background: var(--bg-secondary);
|
||||
color: var(--text-muted);
|
||||
font: inherit;
|
||||
font-size: 0.8rem;
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.wf-layout-toggle:hover {
|
||||
background: var(--bg-tertiary);
|
||||
color: var(--text);
|
||||
}
|
||||
|
||||
.wf-layout-toggle:focus-visible {
|
||||
outline: 2px solid var(--accent);
|
||||
outline-offset: 2px;
|
||||
}
|
||||
|
||||
.wf-editor-body--simple-layout :where(.wf-editor-readonly-banner),
|
||||
.wf-editor-body--simple-layout :where(.wf-editor-toolbar),
|
||||
.wf-editor-body--simple-layout :where(.wf-templates),
|
||||
.wf-editor-body--simple-layout :where(.wf-editor-canvas) {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.wf-editor-body--simple-layout .wf-mobile-shell {
|
||||
display: flex;
|
||||
flex: 1 1 auto;
|
||||
min-height: 0;
|
||||
flex-direction: column;
|
||||
border-top: 1px solid var(--border);
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.wf-editor-inspector {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
@@ -1331,10 +1373,10 @@
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
.wf-editor-body--editor-stage .wf-editor-readonly-banner:not(.wf-mobile-shell),
|
||||
.wf-editor-body--editor-stage .wf-editor-toolbar:not(.wf-mobile-shell),
|
||||
.wf-editor-body--editor-stage .wf-templates:not(.wf-mobile-shell),
|
||||
.wf-editor-body--editor-stage .wf-editor-canvas:not(.wf-mobile-shell) {
|
||||
.wf-editor-body--editor-stage :where(.wf-editor-readonly-banner),
|
||||
.wf-editor-body--editor-stage :where(.wf-editor-toolbar),
|
||||
.wf-editor-body--editor-stage :where(.wf-templates),
|
||||
.wf-editor-body--editor-stage :where(.wf-editor-canvas) {
|
||||
display: none;
|
||||
}
|
||||
|
||||
|
||||
@@ -693,9 +693,11 @@ function InnerEditor({
|
||||
const [selectedNodeId, setSelectedNodeId] = useState<string | null>(null);
|
||||
const [selectedEdgeId, setSelectedEdgeId] = useState<string | null>(null);
|
||||
const [inspectorCollapsed, setInspectorCollapsed] = useState(false);
|
||||
const [compactLayoutEnabled, setCompactLayoutEnabled] = useState(false);
|
||||
const [mobilePanel, setMobilePanel] = useState<MobileWorkflowPanel>(() =>
|
||||
initialPanel === "settings" ? "settings" : "graph",
|
||||
);
|
||||
const simpleLayoutEnabled = isMobileViewport || compactLayoutEnabled;
|
||||
const { t } = useTranslation("app");
|
||||
const { confirm } = useConfirm();
|
||||
// Create-workflow dialog (KTD-7) open state + focus-return ref to the
|
||||
@@ -2145,7 +2147,11 @@ function InnerEditor({
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
<div className={`wf-editor-body${workflowListStageOpen ? " wf-editor-body--list-stage" : " wf-editor-body--editor-stage"}`}>
|
||||
<div
|
||||
className={`wf-editor-body${workflowListStageOpen ? " wf-editor-body--list-stage" : " wf-editor-body--editor-stage"}${
|
||||
simpleLayoutEnabled ? " wf-editor-body--simple-layout" : ""
|
||||
}`}
|
||||
>
|
||||
<aside className="wf-editor-sidebar">
|
||||
<button
|
||||
className="wf-editor-new"
|
||||
@@ -2223,7 +2229,7 @@ function InnerEditor({
|
||||
workflow is active (read-only gating preserved via isBuiltin). The
|
||||
disclosure button serves as the section header; the panels' own
|
||||
internal <h3> is suppressed via CSS to avoid a double header. */}
|
||||
{activeWorkflow && !isMobileViewport && (
|
||||
{activeWorkflow && !simpleLayoutEnabled && (
|
||||
<div className="wf-sidebar-panels">
|
||||
<section className="wf-sidebar-section" data-testid="wf-sidebar-columns-section">
|
||||
<button
|
||||
@@ -2392,8 +2398,24 @@ function InnerEditor({
|
||||
{description || t("workflows.descriptionPlaceholder", "Add a description")}
|
||||
</button>
|
||||
)}
|
||||
{!isMobileViewport && (
|
||||
<button
|
||||
type="button"
|
||||
className="wf-layout-toggle"
|
||||
data-testid="wf-layout-toggle"
|
||||
aria-pressed={compactLayoutEnabled}
|
||||
onClick={() => setCompactLayoutEnabled((enabled) => !enabled)}
|
||||
>
|
||||
{compactLayoutEnabled ? <LayoutGrid size={14} /> : <ListChecks size={14} />}
|
||||
<span>
|
||||
{compactLayoutEnabled
|
||||
? t("workflows.showCanvasEditor", "Show canvas editor")
|
||||
: t("workflows.showSimpleEditor", "Show simple editor")}
|
||||
</span>
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
{isMobileViewport && (
|
||||
{simpleLayoutEnabled && (
|
||||
<div className="wf-mobile-shell" data-testid="wf-mobile-shell">
|
||||
<nav className="wf-mobile-tabs" aria-label={t("workflows.mobileEditorNav", "Workflow editor sections")}>
|
||||
{([
|
||||
@@ -3064,7 +3086,8 @@ function InnerEditor({
|
||||
{selectedNode &&
|
||||
selectedNode.data.kind !== "start" &&
|
||||
selectedNode.data.kind !== "end" &&
|
||||
!(isMobileViewport && inspectorCollapsed) && (
|
||||
!(isMobileViewport && inspectorCollapsed) &&
|
||||
!(compactLayoutEnabled && !isMobileViewport) && (
|
||||
<aside className="wf-editor-inspector" data-testid="wf-node-inspector">
|
||||
<div className="wf-inspector-heading">
|
||||
<h3>Node</h3>
|
||||
|
||||
@@ -70,12 +70,6 @@
|
||||
align-items: stretch;
|
||||
}
|
||||
|
||||
.workflow-selector select,
|
||||
.workflow-selector-manage,
|
||||
.workflow-selector-collapsed-button {
|
||||
width: 100%;
|
||||
}
|
||||
|
||||
.workflow-selector select,
|
||||
.workflow-selector-manage {
|
||||
width: 100%;
|
||||
|
||||
@@ -339,6 +339,40 @@ describe("WorkflowNodeEditor", () => {
|
||||
expect(screen.getAllByRole("button", { name: "QA" })[0]).toHaveClass("active");
|
||||
});
|
||||
|
||||
it("lets desktop users switch to the simple graph layout and back", async () => {
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([def()]);
|
||||
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
|
||||
expect(await screen.findByTestId("wf-workflow-name")).toHaveTextContent("QA");
|
||||
expect(screen.queryByTestId("wf-mobile-shell")).not.toBeInTheDocument();
|
||||
|
||||
fireEvent.click(screen.getByTestId("wf-layout-toggle"));
|
||||
|
||||
expect(await screen.findByTestId("wf-mobile-shell")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("wf-mobile-tab-graph")).toHaveAttribute("aria-current", "page");
|
||||
expect(screen.getByRole("button", { name: "start start" })).toBeInTheDocument();
|
||||
expect(screen.getByTestId("wf-layout-toggle")).toHaveTextContent("Show canvas editor");
|
||||
|
||||
fireEvent.click(screen.getByTestId("wf-layout-toggle"));
|
||||
|
||||
await waitFor(() => expect(screen.queryByTestId("wf-mobile-shell")).not.toBeInTheDocument());
|
||||
expect(screen.getByTestId("wf-layout-toggle")).toHaveTextContent("Show simple editor");
|
||||
});
|
||||
|
||||
it("lets tablet users switch to the simple graph layout", async () => {
|
||||
mockWorkflowEditorViewport("tablet");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([def()]);
|
||||
|
||||
render(<WorkflowNodeEditor isOpen onClose={() => {}} addToast={() => {}} />);
|
||||
|
||||
expect(await screen.findByTestId("wf-workflow-name")).toHaveTextContent("QA");
|
||||
fireEvent.click(screen.getByTestId("wf-layout-toggle"));
|
||||
|
||||
expect(await screen.findByTestId("wf-mobile-shell")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("wf-mobile-tab-actions")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("opens populated mobile workflows on the list with no preselected workflow", async () => {
|
||||
mockWorkflowEditorViewport("mobile");
|
||||
vi.mocked(fetchWorkflows).mockResolvedValue([def(), v2Def()]);
|
||||
|
||||
@@ -39,8 +39,14 @@ function nodeDisplayLabel(node: FlowNode<WorkflowFlowNodeData>): string {
|
||||
return node.data.label || node.id;
|
||||
}
|
||||
|
||||
function nodeSortValue(node: FlowNode<WorkflowFlowNodeData>): number {
|
||||
return Math.round(node.position.y) * 100000 + Math.round(node.position.x);
|
||||
function compareNodePosition(
|
||||
a: FlowNode<WorkflowFlowNodeData>,
|
||||
b: FlowNode<WorkflowFlowNodeData>,
|
||||
): number {
|
||||
const ay = Math.round(a.position.y);
|
||||
const by = Math.round(b.position.y);
|
||||
if (ay !== by) return ay - by;
|
||||
return Math.round(a.position.x) - Math.round(b.position.x);
|
||||
}
|
||||
|
||||
function buildColumnNameMap(columns: WorkflowIrColumn[], nodes: FlowNode<WorkflowFlowNodeData>[]) {
|
||||
@@ -63,6 +69,7 @@ export function buildMobileWorkflowGraph(
|
||||
const columnNames = buildColumnNameMap(columns, nodes);
|
||||
const nodesById = new Map(nodes.map((node) => [node.id, node]));
|
||||
const childNodesByParent = new Map<string, FlowNode<WorkflowFlowNodeData>[]>();
|
||||
const edgesBySource = new Map<string, FlowEdge[]>();
|
||||
|
||||
for (const node of nodes) {
|
||||
if (!node.parentId) continue;
|
||||
@@ -72,7 +79,13 @@ export function buildMobileWorkflowGraph(
|
||||
}
|
||||
|
||||
for (const list of childNodesByParent.values()) {
|
||||
list.sort((a, b) => nodeSortValue(a) - nodeSortValue(b));
|
||||
list.sort(compareNodePosition);
|
||||
}
|
||||
|
||||
for (const edge of edges) {
|
||||
const list = edgesBySource.get(edge.source) ?? [];
|
||||
list.push(edge);
|
||||
edgesBySource.set(edge.source, list);
|
||||
}
|
||||
|
||||
const summarizeEdge = (edge: FlowEdge): MobileWorkflowEdgeSummary => {
|
||||
@@ -99,14 +112,14 @@ export function buildMobileWorkflowGraph(
|
||||
editable: node.data.kind !== "start" && node.data.kind !== "end" && !isColumnBandNode(node.id),
|
||||
parentId: node.parentId,
|
||||
templateLocalId: node.parentId ? templateNodeIdFromChild(node.parentId, node.id) : undefined,
|
||||
outgoing: edges.filter((edge) => edge.source === node.id).map(summarizeEdge),
|
||||
outgoing: (edgesBySource.get(node.id) ?? []).map(summarizeEdge),
|
||||
children,
|
||||
};
|
||||
};
|
||||
|
||||
const topLevelNodes = nodes
|
||||
.filter((node) => !node.parentId && !isColumnBandNode(node.id))
|
||||
.sort((a, b) => nodeSortValue(a) - nodeSortValue(b));
|
||||
.sort(compareNodePosition);
|
||||
|
||||
return topLevelNodes.map(summarizeNode);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user