feat: [EPMRPP-105379] make use of sortable component for steps in create test cases #4782
Conversation
…se-of-sortable-component-for-steps-in-create-test-cases # Conflicts: # app/package-lock.json # app/package.json
kayarmolenka
requested review from
allaprischepa and
maria-hambardzumian
as code owners
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces drag-and-drop reordering functionality for test case steps. It updates the UI-kit dependency, enhances the AttachmentArea component with drag handles, implements step reordering through new hooks and types, adds comprehensive drag-and-drop styling, and restructures the route provider hierarchy to support drag-and-drop operations at scale. Changes
Sequence DiagramsequenceDiagram
actor User
participant Steps as Steps Component
participant DragLayer as DragLayer
participant DndContext as DnD Context
participant State as Internal State
User->>Steps: Initiates drag on step handle
Steps->>DndContext: useDrop detects drag over
DndContext->>DragLayer: Renders drag preview (Step N)
Steps->>Steps: isDraggingAny = true
Steps->>Steps: HideOnDrag hides content (50ms delay)
User->>Steps: Hovers over drop target
Steps->>Steps: .steps__step-container--drop-target applied
Steps->>DragLayer: Preview follows cursor
User->>Steps: Releases on target position
Steps->>State: handleDrop reorders steps array
State->>Steps: Calls onReorderSteps callback
Steps->>Steps: Apply .steps__step-container--just-dropped
Steps->>Steps: Trigger dropHighlight animation
Steps->>Steps: Reset dragging state after 500ms
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.scss
Outdated
Show resolved
Hide resolved
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.scss
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.scss (1)
41-43: Optional: align drag-preview colors and typography with design system tokensThis block still uses legacy SCSS color tokens (
$COLOR--topaz-focused,$COLOR--e-200,$COLOR--e-300) and an explicitfont-size: 13px. Given the prior feedback about preferring--rp-ui-*CSS vars and the@font-scalemixin, it may be worth switching to the design-system tokens/mixins here for consistency (border, highlight, and.drag-previewtext).Also applies to: 66-67, 110-116, 133-133, 136-136, 146-146
🧹 Nitpick comments (2)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts (1)
5-20: Hide-on-drag delay logic looks good; minor timeout guarding improvement possibleThe delayed hide / instant show behavior is correct and the effect cleanup clears the timer when
isDraggingflips. To make this a bit more robust against future edits, you could initializetimeoutIdexplicitly and check againstnullinstead of relying on truthiness:- useEffect(() => { - let timeoutId: ReturnType<typeof setTimeout>; + useEffect(() => { + let timeoutId: ReturnType<typeof setTimeout> | null = null; @@ - return () => { - if (timeoutId) clearTimeout(timeoutId); - }; + return () => { + if (timeoutId !== null) { + clearTimeout(timeoutId); + } + }; }, [isDragging]);Purely optional; current behavior is functionally fine.
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.tsx (1)
49-63: Reorder logic is correct; consider clearing the drop-highlight timeout on unmount
handleDropcorrectly reorders thestepsarray and tracksjustDroppedIdfor the highlight. The only thing missing is cleanup of thesetTimeoutif the component unmounts (or if multiple drops happen quickly), which can lead to state updates on an unmounted component.You can store the timeout id in a ref and clear it on unmount / before scheduling a new one:
-import { useCallback, useState, Ref } from 'react'; +import { useCallback, useEffect, useRef, useState, Ref } from 'react'; @@ const { formatMessage } = useIntl(); const [justDroppedId, setJustDroppedId] = useState<number | null>(null); + const dropTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); @@ const handleDrop = useCallback( (fromIndex: number, toIndex: number) => { @@ - setJustDroppedId(movedStep.id); - setTimeout(() => setJustDroppedId(null), DROP_ANIMATION_DURATION); + setJustDroppedId(movedStep.id); + if (dropTimeoutRef.current) { + clearTimeout(dropTimeoutRef.current); + } + dropTimeoutRef.current = setTimeout(() => setJustDroppedId(null), DROP_ANIMATION_DURATION); @@ - [steps, onReorderSteps], + [steps, onReorderSteps], ); + + useEffect( + () => () => { + if (dropTimeoutRef.current) { + clearTimeout(dropTimeoutRef.current); + } + }, + [], + );Not critical, but it will avoid potential dev warnings and keeps the animation timer under control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
app/package.json(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.scss(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.tsx(4 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hoc.tsx(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/messages.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.scss(2 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.tsx(2 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/types.ts(1 hunks)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/testCaseDetails.tsx(2 hunks)app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlan.ts(1 hunks)app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlanModal.tsx(1 hunks)app/src/routes/pageSwitcher.jsx(1 hunks)app/webpack/base.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-18T14:36:19.317Z
Learnt from: yelyzavetakhokhlova
Repo: reportportal/service-ui PR: 4416
File: app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/template/template.tsx:75-86
Timestamp: 2025-06-18T14:36:19.317Z
Learning: In ReportPortal TypeScript components, noop handlers are sometimes used to satisfy TypeScript requirements for required props, not because functionality is missing. This is a common pattern when the actual form handling is managed by wrapper components.
Applied to files:
app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlan.tsapp/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.tsxapp/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlanModal.tsx
📚 Learning: 2025-07-24T11:26:08.671Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4487
File: app/src/pages/organization/projectTeamPage/projectTeamListTable/projectTeamListTable.jsx:105-105
Timestamp: 2025-07-24T11:26:08.671Z
Learning: In the ReportPortal service-ui project, the react-hooks/exhaustive-deps ESLint rule is strictly enforced, requiring ALL dependencies used within useEffect, useMemo, and useCallback hooks to be included in the dependency array, even stable references like formatMessage from useIntl(). This prevents ESLint errors and maintains code consistency.
Applied to files:
app/package.jsonapp/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.tsx
📚 Learning: 2025-08-14T12:06:14.147Z
Learnt from: allaprischepa
Repo: reportportal/service-ui PR: 4543
File: app/src/pages/inside/projectSettingsPageContainer/content/analyzerContainer/autoAnalysis/autoAnalysis.jsx:17-17
Timestamp: 2025-08-14T12:06:14.147Z
Learning: In the reportportal/service-ui project, webpack uses ProvidePlugin to automatically provide React as a global variable, so explicit `import React from 'react'` is not needed in .jsx files for JSX to work. The project uses classic JSX runtime with automatic React injection via webpack.
Applied to files:
app/package.json
📚 Learning: 2025-10-24T13:55:44.224Z
Learnt from: yelyzavetakhokhlova
Repo: reportportal/service-ui PR: 4674
File: app/src/pages/inside/testPlansPage/testPlanDetailsPage/testPlanDetailsPage.tsx:164-164
Timestamp: 2025-10-24T13:55:44.224Z
Learning: In app/src/controllers/testPlan/constants.ts, the TestPlanDto type defines executionStatistic as a required field (not optional), containing covered: number and total: number. When testPlan exists, executionStatistic is guaranteed to be present, so testPlan?.executionStatistic.total does not require additional optional chaining on executionStatistic.
Applied to files:
app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlanModal.tsx
🧬 Code graph analysis (6)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hoc.tsx (2)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts (1)
useHideOnDrag(5-23)app/src/pages/inside/projectSettingsPageContainer/content/elements/ruleList/ruleItem/draggable/draggableRuleItem.jsx (1)
isDragging(37-37)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/types.ts (1)
app/src/controllers/log/nestedSteps/utils.js (1)
stepId(50-50)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/testCaseDetails.tsx (3)
app/src/pages/inside/testCaseLibraryPage/types.ts (1)
Step(34-40)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/step/step.tsx (1)
Step(64-129)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/step/index.ts (1)
Step(17-17)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts (1)
app/src/pages/inside/projectSettingsPageContainer/content/elements/ruleList/ruleItem/draggable/draggableRuleItem.jsx (1)
isDragging(37-37)
app/src/routes/pageSwitcher.jsx (2)
app/src/components/containers/pageErrorBoundary/pageErrorBoundary.jsx (1)
PageErrorBoundary(21-25)app/src/routes/routesMap.js (2)
page(524-524)page(527-527)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.tsx (6)
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/types.ts (1)
StepsProps(3-11)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/messages.ts (1)
messages(3-12)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hoc.tsx (1)
HideOnDrag(10-16)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.tsx (1)
AttachmentArea(66-193)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/step/step.tsx (1)
Step(64-129)app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/step/index.ts (1)
Step(17-17)
🔇 Additional comments (17)
app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlanModal.tsx (1)
18-18: LGTM! Import path updated to match UI-kit restructuring.The import path for
VoidFnhas been updated to a more specific location within the UI-kit library, aligning with the version bump to alpha.139. This is a type-only import with no runtime impact.app/webpack/base.config.js (1)
174-181: LGTM! Correct module federation configuration for drag-and-drop libraries.The addition of
react-dndandreact-dnd-html5-backendas shared singleton modules is the proper approach for micro-frontend architectures. The singleton configuration ensures a single DnD context exists across all micro-frontends, which is required for drag-and-drop functionality to work correctly.app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlan.ts (1)
19-19: LGTM! Consistent import path update.The
VoidFnimport path has been updated to match the UI-kit library restructuring, consistent with the changes inuseDeleteTestPlanModal.tsx.app/package.json (1)
30-30: UI-kit dependency updated to alpha.139 with internal restructuring.The version bump introduces subpath imports for components like
popover,button,fileDropArea, andcommon/types/commonTypes, indicating internal reorganization of the package structure. Verify that all component imports are compatible with this version and that no unexpected API changes affect existing implementations beyond the path reorganization already reflected in the codebase.app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.scss (1)
122-135: LGTM! Well-implemented drag handle styling.The styling appropriately supports the drag-and-drop UX with proper cursor states, touch handling, and smooth transitions.
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/messages.ts (1)
1-12: LGTM! Clean localization setup.The message definitions follow the project's i18n conventions with clear IDs and default messages.
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/messages.ts (1)
20-23: LGTM! Accessibility-focused localization.The new
dragToReorderkey provides clear labeling for the drag handle, improving accessibility.app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/attachmentArea/attachmentArea.tsx (3)
17-17: LGTM! Necessary import for ref prop.The
Reftype import is correctly added to support the newdragHandleRefprop.
62-63: LGTM! Well-typed drag-and-drop props.The new props are correctly typed as optional, maintaining backward compatibility while enabling drag-and-drop functionality.
125-133: LGTM! Comprehensive drag handle implementation.The drag handle button is well-implemented with:
- Proper ref forwarding for drag-and-drop integration
- Dynamic styling based on drag state
- Accessibility label for screen readers
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/types.ts (1)
1-11: LGTM! Well-designed interface.The
StepsPropsinterface provides a clear contract for the Steps component with appropriate callback signatures supporting both traditional move operations and drag-and-drop reordering.app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/testCaseDetails.tsx (2)
182-187: LGTM! Properly memoized reorder callback.The
handleReorderStepscallback is correctly implemented with proper dependency management, following the project's strict ESLint rules for exhaustive dependencies.
220-220: LGTM! Proper callback wiring.The
onReorderStepsprop correctly connects the drag-and-drop functionality to the parent component's state management.app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hoc.tsx (1)
1-16: LGTM! Clean drag UX enhancement component.The
HideOnDragcomponent provides a clean abstraction for hiding content during drag operations, allowing custom drag previews. The implementation is straightforward and well-typed.app/src/routes/pageSwitcher.jsx (1)
72-80: DndProvider scope change extends drag-and-drop context correctly to modal components.Moving the DndProvider to wrap both Layout and ModalContainer is the correct architectural change. The new test case step reordering feature in the CreateTestCaseModal requires DnD context via the useDrop hook in steps.tsx, which is now available since the provider encompasses the ModalContainer. Existing drag-and-drop functionality in draggableLink (server settings) and draggableRuleItem (project rules) will continue working without regression since the DnD context now covers both the main layout and all modals rendered via ModalContainer.
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.scss (1)
29-73: DnD container, drop-target, and highlight animation look coherentThe new
.steps__listdragging state, drop-target offset + top border (&--drop-target), anddropHighlightanimation align well with the drag-and-drop behavior insteps.tsx. Transitions are short and scoped, so this should feel responsive without jank. No blocking issues from a styling/behavior standpoint.Also applies to: 108-122
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/steps.tsx (1)
113-159: Keying and form field mapping for reordered steps looks consistentUsing
idfor React keys andfieldKey(switching betweenidandindexviaisKeyById) for form paths keeps the DOM stable while allowing you to choose between index-based and id-based form field naming. Combined withonReorderSteps(reorderedSteps), this should preserve field/attachment state correctly across drag reorders.
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts
Outdated
Show resolved
Hide resolved
.../pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/useHideOnDrag.ts
Show resolved
Hide resolved
app/src/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/hooks.ts
Outdated
Show resolved
Hide resolved
...rc/pages/inside/testCaseLibraryPage/createTestCaseModal/testCaseDetails/steps/HideOnDrag.tsx
Show resolved
Hide resolved
…se-of-sortable-component-for-steps-in-create-test-cases # Conflicts: # app/src/pages/inside/testPlansPage/testPlanModals/deleteTestPlanModal/useDeleteTestPlanModal.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/test-library #4782 +/- ##
=====================================================
Coverage 72.59% 72.59%
=====================================================
Files 79 79
Lines 905 905
Branches 124 124
=====================================================
Hits 657 657
Misses 224 224
Partials 24 24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…se-of-sortable-component-for-steps-in-create-test-cases
|
kayarmolenka
deleted the
feature/EPMRPP-105379-make-use-of-sortable-component-for-steps-in-create-test-cases
branch
5 participants
PR Checklist
developfor features/bugfixes, other if mentioned in the task)npm run lint) prior to submission? Enable the git hook on commit in your IDE to run it and format the code automatically.npm run build)?devnpm script)manage:translationsscript?Visuals
Screen.Recording.2025-12-03.at.11.06.18.PM.mov
Summary by CodeRabbit
New Features
Style
Accessibility
Chores
✏️ Tip: You can customize this high-level summary in your review settings.