fix(platform): fix drag leave flicker, upload overlay, and minor UI issues#466
Conversation
…ssues Fix file upload drop zone drag leave firing when hovering over children. Move upload overlay inside drop zone for correct positioning. Remove unnecessary URL.revokeObjectURL cleanup in preview step. Fix navigation overflow and org settings input wrapper class.
Greptile OverviewGreptile SummaryFixed drag-and-drop flicker by checking if
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| services/platform/app/components/ui/forms/file-upload.tsx | Fixed drag leave flicker by checking if relatedTarget is within the drop zone container |
| services/platform/app/features/settings/integrations/components/integration-upload/steps/preview-step.tsx | Removed URL.revokeObjectURL cleanup effect |
| services/platform/app/features/settings/integrations/components/integration-upload/steps/upload-step.tsx | Moved FileUpload.Overlay inside DropZone for correct absolute positioning |
Flowchart
flowchart TD
A[User drags file over drop zone] --> B[onDragOver fires]
B --> C[setIsDragOver true]
C --> D[Overlay becomes visible]
D --> E[User hovers over child element]
E --> F[onDragLeave fires]
F --> G{Is relatedTarget a Node?}
G -->|No| H[setIsDragOver false]
G -->|Yes| I{Is relatedTarget inside currentTarget?}
I -->|Yes - still in drop zone| J[Keep overlay visible]
I -->|No - left drop zone| H
H --> K[Overlay disappears]
J --> L[Overlay stays visible]
Last reviewed commit: 95e2aea
Additional Comments (1)
|
📝 WalkthroughWalkthroughThis PR modifies five UI component files across the platform. It refines drag-and-drop behavior by conditionally preventing premature state changes when dragging within drop zones, adjusts navigation layout by removing vertical scrolling and adding horizontal margins, restructures the file upload overlay by moving it inside the drop zone container, removes cleanup logic for object URLs in the preview component, and updates the Input component's public API to support wrapper-level styling. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/settings/integrations/components/integration-upload/steps/preview-step.tsx (1)
35-38:⚠️ Potential issue | 🟠 MajorReintroduce object URL cleanup to avoid leaks.
URL.createObjectURLallocations persist until revoked. With the cleanup removed, changing icons or unmounting the step can leak memory.✅ Suggested fix
-import { useCallback, useMemo, useRef } from 'react'; +import { useCallback, useMemo, useRef, useEffect } from 'react'; const iconPreviewUrl = useMemo( () => (iconFile ? URL.createObjectURL(iconFile) : null), [iconFile], ); + +useEffect(() => { + if (!iconPreviewUrl) return; + return () => { + URL.revokeObjectURL(iconPreviewUrl); + }; +}, [iconPreviewUrl]);
Summary
instanceof Nodetype guard withcontains()checkFileUpload.Overlayinside the drop zone for correct overlay positioningURL.revokeObjectURLcleanupuseEffectin preview stepmx-1, removedoverflow-y-auto)wrapperClassNameinstead ofclassNameTest plan
Summary by CodeRabbit
Bug Fixes
Style & Layout