From a76e82c72150fd8ab75039621187511ab9c94662 Mon Sep 17 00:00:00 2001 From: Andras Eszes Date: Thu, 8 Aug 2024 15:23:34 +0200 Subject: [PATCH] feat: reorder steps on workflow card (#1118) * feat: reorder steps on workflow card * refactor: duplicated style on StepCard * fix: too small hitbox on StepCard button --- .../components/StepCard/StepCard.tsx | 137 +++++++++++++----- .../components/WorkflowCard/WorkflowCard.tsx | 94 +++++++----- .../WorkflowCard/WorkflowCard.utils.ts | 19 +++ .../contexts/BitriseYmlProvider.tsx | 8 + .../models/BitriseYmlService.spec.ts | 51 +++++++ .../javascripts/models/BitriseYmlService.ts | 39 +++-- .../WorkflowCanvasPanel.tsx | 4 + 7 files changed, 265 insertions(+), 87 deletions(-) create mode 100644 source/javascripts/components/WorkflowCard/WorkflowCard.utils.ts diff --git a/source/javascripts/components/StepCard/StepCard.tsx b/source/javascripts/components/StepCard/StepCard.tsx index e5b1a8ab5..7c1aadbe1 100644 --- a/source/javascripts/components/StepCard/StepCard.tsx +++ b/source/javascripts/components/StepCard/StepCard.tsx @@ -1,15 +1,38 @@ import { Avatar, Box, Card, Skeleton, SkeletonBox, Text } from '@bitrise/bitkit'; +import { useSortable } from '@dnd-kit/sortable'; +import { CSS } from '@dnd-kit/utilities'; import useStep from '@/hooks/useStep'; +import { getSortableStepId } from '../WorkflowCard/WorkflowCard.utils'; type StepCardProps = { workflowId: string; stepIndex: number; - onClick?: VoidFunction; + isDraggable?: boolean; showSecondary?: boolean; + onClick?: VoidFunction; +}; + +const DragHandle = () => { + return ( + + + + + + + + + ); }; -const StepCard = ({ workflowId, stepIndex, showSecondary = true, onClick }: StepCardProps) => { +const StepCard = ({ workflowId, stepIndex, isDraggable, showSecondary = true, onClick }: StepCardProps) => { + const isButton = Boolean(onClick); + const sortableStepId = getSortableStepId(workflowId, stepIndex); + const step = useStep(workflowId, stepIndex); + const sortable = useSortable({ id: sortableStepId, disabled: !isDraggable }); + + const isActive = sortable.active?.id === sortableStepId; if (!step) { return null; @@ -19,9 +42,22 @@ const StepCard = ({ workflowId, stepIndex, showSecondary = true, onClick }: Step if (isLoading) { return ( - - - + + {isDraggable && ( + + + + )} + + + {showSecondary && } @@ -31,40 +67,67 @@ const StepCard = ({ workflowId, stepIndex, showSecondary = true, onClick }: Step ); } - const content = ( - <> - - - - {title} - - {showSecondary && ( - - {selectedVersion || 'Always latest'} + return ( + + {isDraggable && ( + + + + )} + + + + + + {title} - )} + {showSecondary && ( + + {selectedVersion || 'Always latest'} + + )} + - - ); - - if (onClick) { - return ( - - {content} - - ); - } - - return ( - - {content} ); }; diff --git a/source/javascripts/components/WorkflowCard/WorkflowCard.tsx b/source/javascripts/components/WorkflowCard/WorkflowCard.tsx index 7d939f3f2..29f499d7b 100644 --- a/source/javascripts/components/WorkflowCard/WorkflowCard.tsx +++ b/source/javascripts/components/WorkflowCard/WorkflowCard.tsx @@ -1,6 +1,9 @@ import { Fragment } from 'react'; import { Box, ButtonGroup, Card, CardProps, Collapse, ControlButton, Icon, Text, useDisclosure } from '@bitrise/bitkit'; import { useShallow } from 'zustand/react/shallow'; +import { DndContext, DragEndEvent, PointerSensor, useSensor, useSensors } from '@dnd-kit/core'; +import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'; +import { restrictToParentElement, restrictToVerticalAxis } from '@dnd-kit/modifiers'; import StepCard from '@/components/StepCard/StepCard'; import { Step } from '@/models/Step'; import { ChainedWorkflowPlacement as Placement } from '@/models/Workflow'; @@ -8,9 +11,11 @@ import useBitriseYmlStore from '@/hooks/useBitriseYmlStore'; import useWorkflowUsedBy from '@/pages/WorkflowsPage/hooks/useWorkflowUsedBy'; import useWorkflow from './hooks/useWorkflow'; import AddStepButton from './components/AddStepButton'; +import { getSortableStepId, getUsedByText, parseSortableStepId } from './WorkflowCard.utils'; type StepEditCallback = (workflowId: string, stepIndex: number) => void; type WorkflowEditCallback = (workflowId: string) => void; +type MoveStepCallback = (workflowId: string, stepIndex: number, to: number) => void; type WorkflowCardProps = CardProps & { workflowId: string; @@ -21,22 +26,12 @@ type WorkflowCardProps = CardProps & { isExpanded?: boolean; isEditable?: boolean; onAddStep?: StepEditCallback; + onMoveStep?: MoveStepCallback; onSelectStep?: StepEditCallback; onEditWorkflow?: WorkflowEditCallback; onChainWorkflow?: WorkflowEditCallback; }; -const getUsedByText = (usedBy: string[]) => { - switch (usedBy.length) { - case 0: - return 'not used by other Workflow'; - case 1: - return 'used by 1 Workflow'; - default: - return `used by ${usedBy.length} Workflows`; - } -}; - const WorkflowCard = ({ workflowId, parentWorkflowId, @@ -46,16 +41,33 @@ const WorkflowCard = ({ isExpanded, isEditable, onAddStep, + onMoveStep, onSelectStep, onEditWorkflow, onChainWorkflow, ...props }: WorkflowCardProps) => { const workflow = useWorkflow(workflowId); + const sensors = useSensors(useSensor(PointerSensor)); const workflowUsedBy = useWorkflowUsedBy(workflowId); const { isOpen, onToggle } = useDisclosure({ defaultIsOpen: isExpanded || isFixed }); const deleteChainedWorkflow = useBitriseYmlStore(useShallow((s) => s.deleteChainedWorkflow)); + const sortableItemIdentifiers = (workflow?.steps ?? []).map((_, stepIndex) => { + return getSortableStepId(workflowId, stepIndex); + }); + + const isRoot = !parentWorkflowId; + const hasNoSteps = !workflow?.steps?.length; + const hasAfterRunWorkflows = !!workflow?.after_run?.length; + const hasBeforeRunWorkflows = !!workflow?.before_run?.length; + + const handleMoveStep = (e: DragEndEvent) => { + const { stepIndex } = parseSortableStepId(e.active.id.toString()); + const { stepIndex: to } = parseSortableStepId(e.over?.id.toString() || ''); + onMoveStep?.(workflowId, stepIndex, to); + }; + if (!workflow) { // TODO: Missing mpty state // eslint-disable-next-line no-console @@ -63,11 +75,6 @@ const WorkflowCard = ({ return null; } - const isRoot = !parentWorkflowId; - const hasNoSteps = !workflow.steps?.length; - const hasAfterRunWorkflows = Boolean(workflow.after_run?.length); - const hasBeforeRunWorkflows = Boolean(workflow.before_run?.length); - return ( @@ -141,6 +148,7 @@ const WorkflowCard = ({ placement="before_run" isEditable={isEditable} onAddStep={onAddStep} + onMoveStep={onMoveStep} onSelectStep={onSelectStep} onEditWorkflow={onEditWorkflow} onChainWorkflow={onChainWorkflow} @@ -158,26 +166,39 @@ const WorkflowCard = ({ )} - {workflow.steps?.map((s, stepIndex, steps) => { - const isLastStep = stepIndex === steps.length - 1; - const [cvs = ''] = Object.entries(s)[0] as [string, Step]; - - return ( - // eslint-disable-next-line react/no-array-index-key - - {isEditable && onAddStep?.(workflowId, stepIndex)} my={-8} />} - onSelectStep(workflowId, stepIndex))} - showSecondary - /> - {isEditable && isLastStep && ( - onAddStep?.(workflowId, stepIndex + 1)} my={-8} /> - )} - - ); - })} + + + + {workflow?.steps?.map((s, stepIndex, steps) => { + const isLastStep = stepIndex === steps.length - 1; + const [cvs = ''] = Object.entries(s)[0] as [string, Step]; + + return ( + // eslint-disable-next-line react/no-array-index-key + + {isEditable && onAddStep?.(workflowId, stepIndex)} my={-8} />} + + onSelectStep(workflowId, stepIndex))} + /> + + {isEditable && isLastStep && ( + onAddStep?.(workflowId, stepIndex + 1)} my={-8} /> + )} + + ); + })} + + + {hasAfterRunWorkflows && } @@ -192,6 +213,7 @@ const WorkflowCard = ({ placement="after_run" isEditable={isEditable} onAddStep={onAddStep} + onMoveStep={onMoveStep} onSelectStep={onSelectStep} onEditWorkflow={onEditWorkflow} onChainWorkflow={onChainWorkflow} diff --git a/source/javascripts/components/WorkflowCard/WorkflowCard.utils.ts b/source/javascripts/components/WorkflowCard/WorkflowCard.utils.ts new file mode 100644 index 000000000..70c910fd8 --- /dev/null +++ b/source/javascripts/components/WorkflowCard/WorkflowCard.utils.ts @@ -0,0 +1,19 @@ +export function getUsedByText(usedBy: string[]) { + switch (usedBy.length) { + case 0: + return 'not used by other Workflow'; + case 1: + return 'used by 1 Workflow'; + default: + return `used by ${usedBy.length} Workflows`; + } +} + +export function getSortableStepId(workflowId: string, stepIndex: number) { + return `${workflowId}->${stepIndex}`; +} + +export function parseSortableStepId(sortableStepId: string) { + const [workflowId, stepIndexAsString] = sortableStepId.split('->'); + return { workflowId, stepIndex: Number(stepIndexAsString) }; +} diff --git a/source/javascripts/contexts/BitriseYmlProvider.tsx b/source/javascripts/contexts/BitriseYmlProvider.tsx index bac54197d..27a1b0832 100644 --- a/source/javascripts/contexts/BitriseYmlProvider.tsx +++ b/source/javascripts/contexts/BitriseYmlProvider.tsx @@ -15,6 +15,7 @@ export type BitriseYmlProviderState = { defaultMeta?: Meta; // Workflow related actions + moveStep: (workflowId: string, stepIndex: number, to: number) => void; createWorkflow: (workflowId: string, baseWorkflowId?: string) => void; deleteWorkflow: (workflowId: string) => void; deleteChainedWorkflow: ( @@ -35,6 +36,13 @@ const createBitriseYmlStore = (yml: BitriseYml, defaultMeta?: Meta) => { return createStore()((set) => ({ yml, defaultMeta, + moveStep(workflowId, stepIndex, to) { + return set((state) => { + return { + yml: BitriseYmlService.moveStep(workflowId, stepIndex, to, state.yml), + }; + }); + }, createWorkflow(workflowId, baseWorkflowId) { return set((state) => { return { diff --git a/source/javascripts/models/BitriseYmlService.spec.ts b/source/javascripts/models/BitriseYmlService.spec.ts index 60abe9217..3e32fc269 100644 --- a/source/javascripts/models/BitriseYmlService.spec.ts +++ b/source/javascripts/models/BitriseYmlService.spec.ts @@ -35,6 +35,57 @@ const toMatchBitriseYml: MatcherFunction<[expected: BitriseYml]> = function m(ac expect.extend({ toMatchBitriseYml }); describe('BitriseYmlService', () => { + describe('moveStep', () => { + it('should move step to the expected place', () => { + const sourceYml: BitriseYml = { + format_version: '', + workflows: { wf1: { steps: [{ script: {} }, { clone: {} }, { deploy: {} }] } }, + }; + + const expectedYml: BitriseYml = { + format_version: '', + workflows: { wf1: { steps: [{ deploy: {} }, { script: {} }, { clone: {} }] } }, + }; + + const actualYml = BitriseYmlService.moveStep('wf1', 2, 0, sourceYml); + + expect(actualYml).toMatchBitriseYml(expectedYml); + }); + + it('should return the original BitriseYml if workflow is not exist', () => { + const sourceAndExpectedYml: BitriseYml = { + format_version: '', + workflows: { wf1: { steps: [{ script: {} }, { clone: {} }, { deploy: {} }] } }, + }; + + const actualYml = BitriseYmlService.moveStep('wf2', 2, 0, sourceAndExpectedYml); + + expect(actualYml).toMatchBitriseYml(sourceAndExpectedYml); + }); + + it('should return the original BitriseYml if step on is not exist', () => { + const sourceAndExpectedYml: BitriseYml = { + format_version: '', + workflows: { wf1: { steps: [{ script: {} }, { clone: {} }, { deploy: {} }] } }, + }; + + const actualYml = BitriseYmlService.moveStep('wf1', 3, 0, sourceAndExpectedYml); + + expect(actualYml).toMatchBitriseYml(sourceAndExpectedYml); + }); + + it('should return the original BitriseYml if destination index is out of range', () => { + const sourceAndExpectedYml: BitriseYml = { + format_version: '', + workflows: { wf1: { steps: [{ script: {} }, { clone: {} }, { deploy: {} }] } }, + }; + + const actualYml = BitriseYmlService.moveStep('wf1', 2, 3, sourceAndExpectedYml); + + expect(actualYml).toMatchBitriseYml(sourceAndExpectedYml); + }); + }); + describe('createWorkflow', () => { it('should create an empty workflow if base workflow is missing', () => { const sourceYml: BitriseYml = { diff --git a/source/javascripts/models/BitriseYmlService.ts b/source/javascripts/models/BitriseYmlService.ts index bc373798d..30544ae9f 100644 --- a/source/javascripts/models/BitriseYmlService.ts +++ b/source/javascripts/models/BitriseYmlService.ts @@ -9,6 +9,16 @@ import { ChainedWorkflowPlacement as Placement, Workflows } from './Workflow'; import { Pipelines } from './Pipeline'; import { TriggerMap } from './TriggerMap'; +function moveStep(workflowId: string, stepIndex: number, to: number, yml: BitriseYml): BitriseYml { + const copy = deepCloneSimpleObject(yml); + + if (copy.workflows?.[workflowId]?.steps?.[stepIndex]) { + copy.workflows[workflowId].steps.splice(to, 0, copy.workflows[workflowId].steps.splice(stepIndex, 1)[0]); + } + + return copy; +} + function createWorkflow(workflowId: string, yml: BitriseYml, baseWorkflowId?: string): BitriseYml { const copy = deepCloneSimpleObject(yml); @@ -95,20 +105,6 @@ function addChainedWorkflow( return copy; } -function deleteWorkflowFromChains(workflowId: string, workflows: Workflows = {}): Workflows { - return mapValues(workflows, (workflow) => { - const workflowCopy = deepCloneSimpleObject(workflow); - - workflowCopy.after_run = workflowCopy.after_run?.filter((id) => id !== workflowId); - workflowCopy.before_run = workflowCopy.before_run?.filter((id) => id !== workflowId); - - if (isEmpty(workflowCopy.after_run)) delete workflowCopy.after_run; - if (isEmpty(workflowCopy.before_run)) delete workflowCopy.before_run; - - return workflowCopy; - }); -} - // UTILITY FUNCTIONS function isNotEmpty(v: T) { @@ -125,6 +121,20 @@ function omitEmptyIfKeyNotExistsIn(o: Record, keys: string[]) { // PRIVATE FUNCTIONS +function deleteWorkflowFromChains(workflowId: string, workflows: Workflows = {}): Workflows { + return mapValues(workflows, (workflow) => { + const workflowCopy = deepCloneSimpleObject(workflow); + + workflowCopy.after_run = workflowCopy.after_run?.filter((id) => id !== workflowId); + workflowCopy.before_run = workflowCopy.before_run?.filter((id) => id !== workflowId); + + if (isEmpty(workflowCopy.after_run)) delete workflowCopy.after_run; + if (isEmpty(workflowCopy.before_run)) delete workflowCopy.before_run; + + return workflowCopy; + }); +} + function deleteWorkflowFromStages(workflowId: string, stages: Stages = {}): Stages { return mapValues(stages, (stage) => { const stageCopy = deepCloneSimpleObject(stage); @@ -159,6 +169,7 @@ function deleteWorkflowFromTriggerMap(workflowId: string, triggerMap: TriggerMap } export default { + moveStep, createWorkflow, deleteWorkflow, addChainedWorkflow, diff --git a/source/javascripts/pages/WorkflowsPage/components.new/WorkflowCanvasPanel/WorkflowCanvasPanel.tsx b/source/javascripts/pages/WorkflowsPage/components.new/WorkflowCanvasPanel/WorkflowCanvasPanel.tsx index 9978396eb..c3654977d 100644 --- a/source/javascripts/pages/WorkflowsPage/components.new/WorkflowCanvasPanel/WorkflowCanvasPanel.tsx +++ b/source/javascripts/pages/WorkflowsPage/components.new/WorkflowCanvasPanel/WorkflowCanvasPanel.tsx @@ -1,5 +1,7 @@ import { Box, IconButton, Menu, MenuButton, MenuItem, MenuList } from '@bitrise/bitkit'; +import { useShallow } from 'zustand/react/shallow'; import WorkflowCard from '@/components/WorkflowCard/WorkflowCard'; +import useBitriseYmlStore from '@/hooks/useBitriseYmlStore'; import useSelectedWorkflow from '../../hooks/useSelectedWorkflow'; import { isWebsiteMode } from '../../utils/isWebsiteMode'; import { useWorkflowsPageStore } from '../../WorkflowsPage.store'; @@ -7,6 +9,7 @@ import WorkflowSelector from './components/WorkflowSelector/WorkflowSelector'; const WorkflowCanvasPanel = () => { const [{ id: selectedWorkflowId }] = useSelectedWorkflow(); + const { moveStep } = useBitriseYmlStore(useShallow((s) => ({ moveStep: s.moveStep }))); const { openStepConfigDrawer, @@ -45,6 +48,7 @@ const WorkflowCanvasPanel = () => { mx="auto" isFixed isEditable + onMoveStep={moveStep} onAddStep={openStepSelectorDrawer} onSelectStep={openStepConfigDrawer} onEditWorkflow={openWorkflowConfigDrawer}