Bug 1808144: Pipeline builder ux changes

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.scss

@@ -1,3 +1,5 @@
+@import '../../../topology/topology-utils.scss';
+
 $border-color: var(--pf-global--BorderColor--light-100);
 .odc-pipeline-vis-task {
   position: relative;
@@ -18,6 +20,10 @@ $border-color: var(--pf-global--BorderColor--light-100);
     &:hover {
       box-shadow: 0 0 16px rgba(0, 0, 0, 0.12);
     }
+
+    &.is-selected {
+      border: 2px solid $interactive-stroke-color;
+    }
   }
   &:not(:first-child) {
     margin-top: 1.5em;

frontend/packages/dev-console/src/components/pipelines/detail-page-tabs/pipeline-details/PipelineVisualizationTask.tsx

@@ -25,6 +25,7 @@ interface TaskProps {
   namespace: string;
   isPipelineRun: boolean;
   disableTooltip?: boolean;
+  selected?: boolean;
 }
 
 interface PipelineVisualizationTaskProp {
@@ -41,6 +42,7 @@ interface PipelineVisualizationTaskProp {
   taskRun?: string;
   pipelineRunStatus?: string;
   disableTooltip?: boolean;
+  selected?: boolean;
 }
 
 export const PipelineVisualizationTask: React.FC<PipelineVisualizationTaskProp> = ({
@@ -49,6 +51,7 @@ export const PipelineVisualizationTask: React.FC<PipelineVisualizationTaskProp>
   namespace,
   pipelineRunStatus,
   disableTooltip,
+  selected,
 }) => {
   const taskStatus = task.status || {
     duration: '',
@@ -72,6 +75,7 @@ export const PipelineVisualizationTask: React.FC<PipelineVisualizationTaskProp>
       status={taskStatus}
       isPipelineRun={!!pipelineRunStatus}
       disableTooltip={disableTooltip}
+      selected={selected}
     />
   );
 
@@ -108,6 +112,7 @@ const TaskComponent: React.FC<TaskProps> = ({
   name,
   isPipelineRun,
   disableTooltip,
+  selected,
 }) => {
   const stepList = _.get(task, ['data', 'spec', 'steps'], []);
   const stepStatusList: StepStatus[] = stepList.map((step) => createStepStatus(step, status));
@@ -118,7 +123,7 @@ const TaskComponent: React.FC<TaskProps> = ({
     : undefined;
 
   let taskPill = (
-    <div className="odc-pipeline-vis-task__content">
+    <div className={cx('odc-pipeline-vis-task__content', { 'is-selected': selected })}>
       <div
         className={cx('odc-pipeline-vis-task__title-wrapper', {
           'is-text-center': !isPipelineRun,

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderForm.tsx

@@ -39,6 +39,8 @@ type PipelineBuilderFormProps = FormikProps<FormikValues> & {
 
 const PipelineBuilderForm: React.FC<PipelineBuilderFormProps> = (props) => {
   const [selectedTask, setSelectedTask] = React.useState<SelectedBuilderTask>(null);
+  const selectedTaskRef = React.useRef<SelectedBuilderTask>(null);
+  selectedTaskRef.current = selectedTask;
 
   const {
     existingPipeline,
@@ -78,7 +80,14 @@ const PipelineBuilderForm: React.FC<PipelineBuilderFormProps> = (props) => {
     updateErrors(taskErrors);
   };
 
-  const taskGroup: PipelineBuilderTaskGroup = { tasks: values.tasks, listTasks: values.listTasks };
+  const selectedId = values.tasks[selectedTask?.taskIndex]?.name;
+  const selectedIds = selectedId ? [selectedId] : [];
+
+  const taskGroup: PipelineBuilderTaskGroup = {
+    tasks: values.tasks,
+    listTasks: values.listTasks,
+    highlightedIds: selectedIds,
+  };
 
   return (
     <Stack className="odc-pipeline-builder-form">
@@ -149,9 +158,18 @@ const PipelineBuilderForm: React.FC<PipelineBuilderFormProps> = (props) => {
               </Button>
             </ActionGroup>
           </ButtonBar>
-          <Sidebar open={!!selectedTask} onRequestClose={() => setSelectedTask(null)}>
+          <Sidebar
+            open={!!selectedTask}
+            onRequestClose={() => {
+              if (selectedTask?.taskIndex === selectedTaskRef.current?.taskIndex) {
+                setSelectedTask(null);
+              }
+            }}
+          >
             {() => (
               <TaskSidebar
+                // Intentional remount when selection changes
+                key={selectedTask.taskIndex}
                 resourceList={values.resources || []}
                 errorMap={status?.tasks || {}}
                 onUpdateTask={(data: UpdateOperationUpdateTaskData) => {

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/PipelineBuilderHeader.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { Button, Flex, FlexItem, FlexItemModifiers } from '@patternfly/react-core';
 import { PipelineModel } from '../../../models';
-import { warnAction } from './modals';
+import { warnYAML } from './modals';
 import { getBadgeFromType } from '@console/shared/src';
 import { Pipeline } from '../../../utils/pipeline-augment';
 import { goToYAML } from './utils';
@@ -28,12 +28,7 @@ const PipelineBuilderHeader: React.FC<PipelineBuilderHeaderProps> = (props) => {
             variant="link"
             onClick={() => {
               if (formIsDirty) {
-                warnAction(
-                  'Edit YAML',
-                  'You are about to leave',
-                  'The Pipeline Builder content will be lost if you navigate. Are you sure?',
-                  () => goToYAML(existingPipeline, namespace),
-                );
+                warnYAML(() => goToYAML(existingPipeline, namespace));
               } else {
                 goToYAML(existingPipeline, namespace);
               }

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/hooks.ts

@@ -134,18 +134,31 @@ export const useNodes = (
   // TODO: Fix id collisions then remove this utility; we shouldn't need to trim the tasks
   const noDuplicates = (resource: PipelineResourceTask) =>
     !taskGroupRef.current.tasks.find((pt) => pt.name === resource.metadata.name);
-  const newListNode = (name: string, runAfter?: string[]): PipelineTaskListNodeModel =>
+  const newListNode = (
+    name: string,
+    runAfter?: string[],
+    firstTask?: boolean,
+  ): PipelineTaskListNodeModel =>
     createTaskListNode(name, {
       namespaceTaskList: namespacedTasks?.filter(noDuplicates),
       clusterTaskList: clusterTasks?.filter(noDuplicates),
       onNewTask: (resource: PipelineResourceTask) => {
         onNewTask(resource, name, runAfter);
       },
+      onRemoveTask: firstTask
+        ? null
+        : () => {
+            onUpdateTasks(taskGroupRef.current, {
+              type: UpdateOperationType.DELETE_LIST_TASK,
+              data: { listTaskName: name },
+            });
+          },
       task: {
         name,
         runAfter: runAfter || [],
       },
     });
+  const soloTask = (name = 'initial-node') => newListNode(name, undefined, true);
 
   const taskNodes: PipelineBuilderTaskNodeModel[] =
     taskGroup.tasks.length > 0
@@ -154,11 +167,12 @@ export const useNodes = (
           onNewListNode,
           (task) => onTaskSelection(task, getTask(task.taskRef)),
           getErrorMessage(nodeTaskErrors, tasksInError),
+          taskGroup.highlightedIds,
         )
       : [];
   const taskListNodes: PipelineTaskListNodeModel[] =
-    taskGroup.tasks.length === 0 && taskGroup.listTasks.length === 0
-      ? [newListNode('initial-node')]
+    taskGroup.tasks.length === 0 && taskGroup.listTasks.length <= 1
+      ? [soloTask(taskGroup.listTasks[0]?.name)]
       : taskGroup.listTasks.map((listTask) => newListNode(listTask.name, listTask.runAfter));
 
   const nodes: PipelineMixedNodeModel[] = handleParallelToParallelNodes([

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/modals/ModalContent.tsx

@@ -14,8 +14,8 @@ const ModalContent: React.FC<ModalContentProps> = ({ icon, message, title }) =>
     <Split className="odc-modal-content" gutter="md">
       {icon && <SplitItem>{icon}</SplitItem>}
       <SplitItem isFilled>
-        <h2 className="odc-modal-content__confirm-title">{title}</h2>
-        <p>{message}</p>
+        <h2 className="co-break-word odc-modal-content__confirm-title">{title}</h2>
+        <p className="co-break-word">{message}</p>
       </SplitItem>
     </Split>
   );

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/modals/index.tsx

@@ -25,22 +25,17 @@ export const removeTaskModal = (taskName: string, onRemove: ModalCallback) => {
   });
 };
 
-export const warnAction = (
-  modalHeader: string,
-  title: string,
-  message: string,
-  onAccept: ModalCallback,
-) => {
+export const warnYAML = (onAccept: ModalCallback) => {
   confirmModal({
-    title: modalHeader,
     message: (
       <ModalContent
         icon={<ExclamationTriangleIcon size="lg" color={warningColor.value} />}
-        title={title}
-        message={message}
+        title="Switch to YAML Editor?"
+        message="Your changes will be lost if you continue. Are you sure you want to leave this form?"
       />
     ),
-    buttonText: 'Confirm',
+    submitDanger: true,
+    btnText: 'Continue',
     executeFn: () => {
       onAccept();
       return Promise.resolve();

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/task-sidebar/TaskSidebar.scss

@@ -1,6 +1,7 @@
 .odc-task-sidebar {
   &__header {
     font-size: var(--pf-global--FontSize--md);
+    height: 33px; // fixed height header to avoid actions from bouncing the header
     margin: 0;
   }
 

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/types.ts

@@ -15,12 +15,16 @@ export type PipelineBuilderTaskBase = { name: string; runAfter?: string[] };
 
 export type PipelineBuilderListTask = PipelineBuilderTaskBase;
 
-export type PipelineBuilderTaskGroup = {
+export type PipelineBuilderTaskGrouping = {
   tasks: PipelineTask[];
   listTasks: PipelineBuilderListTask[];
 };
 
-export type PipelineBuilderFormValues = PipelineBuilderTaskGroup & {
+export type PipelineBuilderTaskGroup = PipelineBuilderTaskGrouping & {
+  highlightedIds: string[];
+};
+
+export type PipelineBuilderFormValues = PipelineBuilderTaskGrouping & {
   name: string;
   params: PipelineParam[];
   resources: PipelineResource[];
@@ -63,6 +67,9 @@ export type UpdateOperationConvertToTaskData = UpdateOperationBaseData & {
   resource: PipelineResourceTask;
   runAfter?: string[];
 };
+export type UpdateOperationDeleteListTaskData = UpdateOperationBaseData & {
+  listTaskName: string;
+};
 export type UpdateOperationRemoveTaskData = UpdateOperationBaseData & {
   taskName: string;
 };

frontend/packages/dev-console/src/components/pipelines/pipeline-builder/update-utils.ts

@@ -18,6 +18,7 @@ import {
   UpdateOperationAction,
   UpdateOperationAddData,
   UpdateOperationConvertToTaskData,
+  UpdateOperationDeleteListTaskData,
   UpdateOperationRemoveTaskData,
   UpdateOperationUpdateTaskData,
   UpdateTaskParamData,
@@ -220,22 +221,56 @@ const convertListToTask: UpdateOperationAction<UpdateOperationConvertToTaskData>
   };
 };
 
+const removeAndUpdateTasks = <
+  URT extends PipelineBuilderTaskBase,
+  UT extends PipelineBuilderTaskBase
+>(
+  removalTaskName: string,
+  updateAndRemoveTasks: URT[],
+  updateOnlyTasks: UT[],
+): { updateOnlyTasks: UT[]; updateAndRemoveTasks: URT[] } => {
+  const removalTask = updateAndRemoveTasks.find((task) => task.name === removalTaskName);
+  return {
+    updateOnlyTasks: updateOnlyTasks.map((task) => mapStitchReplaceInOthers<UT>(removalTask, task)),
+    updateAndRemoveTasks: updateAndRemoveTasks
+      .filter((task) => task.name !== removalTaskName)
+      .map((task) => mapStitchReplaceInOthers<URT>(removalTask, task)),
+  };
+};
+
+const deleteListTask: UpdateOperationAction<UpdateOperationDeleteListTaskData> = (
+  tasks,
+  listTasks,
+  data,
+) => {
+  const { listTaskName } = data;
+
+  const { updateOnlyTasks, updateAndRemoveTasks } = removeAndUpdateTasks<
+    PipelineBuilderListTask,
+    PipelineTask
+  >(listTaskName, listTasks, tasks);
+  return {
+    tasks: updateOnlyTasks,
+    listTasks: updateAndRemoveTasks,
+    errors: null,
+  };
+};
+
 export const removeTask: UpdateOperationAction<UpdateOperationRemoveTaskData> = (
   tasks,
   listTasks,
   data,
 ) => {
   const { taskName } = data;
 
-  const removalTask = tasks.find((pipelineTask) => pipelineTask.name === taskName);
+  const { updateOnlyTasks, updateAndRemoveTasks } = removeAndUpdateTasks<
+    PipelineTask,
+    PipelineBuilderListTask
+  >(taskName, tasks, listTasks);
   return {
-    tasks: tasks
-      .filter((pipelineTask) => pipelineTask.name !== taskName)
-      .map((pipelineTask) => mapStitchReplaceInOthers<PipelineTask>(removalTask, pipelineTask)),
-    listTasks: listTasks.map((pipelineTask) =>
-      mapStitchReplaceInOthers<PipelineBuilderListTask>(removalTask, pipelineTask),
-    ),
-    errors: null,
+    tasks: updateAndRemoveTasks,
+    listTasks: updateOnlyTasks,
+    errors: { [taskName]: null },
   };
 };
 
@@ -352,6 +387,8 @@ export const applyChange = (
       return addListNode(tasks, listTasks, data as UpdateOperationAddData);
     case UpdateOperationType.CONVERT_LIST_TO_TASK:
       return convertListToTask(tasks, listTasks, data as UpdateOperationConvertToTaskData);
+    case UpdateOperationType.DELETE_LIST_TASK:
+      return deleteListTask(tasks, listTasks, data as UpdateOperationDeleteListTaskData);
     case UpdateOperationType.REMOVE_TASK:
       return removeTask(tasks, listTasks, data as UpdateOperationRemoveTaskData);
     case UpdateOperationType.UPDATE_TASK:

frontend/packages/dev-console/src/components/pipelines/pipeline-topology/TaskListNode.scss

@@ -12,5 +12,9 @@
     max-height: 350px;
     overflow: auto;
   }
+  &__divider {
+    border-color: var(--pf-global--BorderColor--100);
+    margin: 0;
+  }
 }