fix(VDisk): display donor and vdisk popups separately#3444
fix(VDisk): display donor and vdisk popups separately#3444artemmufazalov merged 2 commits intomainfrom
Conversation
| position: relative; | ||
|
|
||
| &__layer { | ||
| background: var(--g-color-base-background); |
There was a problem hiding this comment.
Prevent black angles around VDisk
| --ydb-stack-offset-y: 4px; | ||
| --ydb-stack-offset-x-hover: 4px; | ||
| --ydb-stack-offset-y-hover: 6px; | ||
| --ydb-stack-offset-y-hover: 8px; |
There was a problem hiding this comment.
Increase offset a little, so it's easier to point at needed disk
Before:
After:
|
|
||
| border-radius: 4px; // to match interactive area with disk shape | ||
| border-radius: var(--g-border-radius-m); // to match interactive area with disk shape | ||
| background: var(--g-color-base-background); |
There was a problem hiding this comment.
Added here background that was set for Stack previously
| delayClose={delayClose} | ||
| delayOpen={delayOpen} | ||
| // Allow all placement options, component should choose first available | ||
| placement={['top', 'bottom', 'left', 'right']} |
There was a problem hiding this comment.
Prevent popup from displaying only at top or bottom - it causes additional scroll when popup is too big
|
|
||
| const commonVDiskProps: Partial<VDiskProps> = { | ||
| withIcon, | ||
| showPopup: isHighlighted, |
There was a problem hiding this comment.
Prevent donor and vdisk popups displaying at the same time
artemmufazalov
requested review from
Raubzeug,
adameat and
astandrik
as code owners
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/VDisk/VDiskWithDonorsStack.tsx
Line: 44:49
Comment:
**Highlighted state no longer opens**
`isHighlighted` is still computed and passed to `VDisk` as `highlighted`, but after removing `showPopup: isHighlighted` the highlight state will no longer actually force the popup open (HoverPopup only uses `showPopup` for external control). This makes `highlightedVDisk/setHighlightedVDisk` ineffective for keeping a specific vdisk+donors popup visible (e.g., when moving between stacked layers), so the “separate popups” behavior may regress to “no externally-controlled popup” in the highlighted case.
How can I resolve this? If you propose a fix, please make it concise.
Changing Prompt To Fix With AIThis is a comment left during a code review.
Path: src/components/VDisk/VDisk.tsx
Line: 49:59
Comment:
**Non-deterministic popup placement**
Changing `placement` from the HoverPopup default (`['top', 'bottom']`) to `['top', 'bottom', 'left', 'right']` means the popup may now choose left/right in tight layouts, which can overlap adjacent stacked disks and make it harder to move the cursor into the popup (especially in the Stack hover interaction). If the goal is to avoid donors/vdisk popups colliding, this is a behavioral change that should be constrained to the specific stacked case (or use an explicit placement order that matches the layout otherwise non-stacked VDisk popups will start flipping laterally.
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Pull request overview
This PR fixes issues where VDisk and donor tooltips were shown simultaneously (#3403) and where clicking on VDisks with donors was difficult (#3430). The fix removes the forced popup visibility that was causing all VDisks in a stack to display their tooltips at once, allowing each VDisk to manage its own popup state independently while maintaining visual highlighting coordination.
Changes:
- Removed forced popup display (
showPopup: isHighlighted) from stacked VDisks, allowing independent tooltip management - Added flexible tooltip placement options to better handle space constraints in stacked layouts
- Improved styling with CSS variables and adjusted hover offsets for better user interaction
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/components/VDisk/VDiskWithDonorsStack.tsx | Removed showPopup: isHighlighted from common props to allow independent popup management per VDisk |
| src/components/VDisk/VDisk.tsx | Added flexible placement options for tooltips to better handle stacked disk scenarios |
| src/components/VDisk/VDisk.scss | Replaced hardcoded border-radius with CSS variable and added background color to content element |
| src/components/Stack/Stack.scss | Increased hover offset and removed background from stack layer (moved to VDisk content) |
Part of #3403 and #3430
Now popups are displayed separately as before. I am not sure it is a final fix - I do not know why it was done this way, but it should help for now
Stand (data is from solomon cluster, it may be not 100% correct within VLA stand): https://nda.ya.ru/t/5_UZlaDo7Tm5BV
Greptile Overview
Greptile Summary
VDiskhover popup behavior by expanding allowed popup placements and tweaking stack hover offsets/background styling.VDiskCSS to use design-token border radius and ensure content has a background.VDiskWithDonorsStackto stop forcingshowPopupbased on highlighted state, aiming to separate donor/vdisk popups.Confidence Score: 3/5
showPopupwhile still computing highlight state likely breaks intended synchronized popup behavior in VDiskWithDonorsStack; popup placement broadening may also affect usability across the app.Important Files Changed
Sequence Diagram
CI Results
Test Status: ✅ PASSED
📊 Full Report
Test Changes Summary 🗑️192
🗑️ Deleted Tests (192)
Bundle Size: ✅
Current: 62.87 MB | Main: 62.87 MB
Diff: +0.22 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information