-
Notifications
You must be signed in to change notification settings - Fork 18
feat(ui): update Disk indicators visual design across console #3142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
13 files reviewed, no comments
|
ToDo: Fix pdisks page and dimming disks for one vdisk page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
13 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the visual design of disk indicators throughout the console with improved styling, hover states, and more compact layouts. The changes modernize the appearance of PDisk and VDisk progress bars while making their interactive states more visually clear.
- Updated entity state colors with hover/highlighted states and adjusted border colors for better visual hierarchy
- Modified donor disk logic to display donor indicators regardless of replication state
- Adjusted component widths and spacing for more compact, consistent layouts
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/disks/helpers.ts | Simplified donor icon logic to show for all donors, not just replicating ones |
| src/styles/mixins.scss | Enhanced entity-state-colors mixin with hover/highlighted states and adjusted color values |
| src/containers/Storage/utils/useStorageColumnsSettings.ts | Increased minimum PDisk width from 120px to 165px |
| src/containers/Storage/VDisks/VDisks.scss | Reduced VDisk item width from 90px to 55px and updated margins |
| src/containers/Storage/PaginatedStorageGroupsTable/columns/columns.tsx | Reduced VDisks and Disks column widths for more compact layout |
| src/containers/Storage/PDisk/PDisk.tsx | Added highlighted state when popup is shown |
| src/containers/Storage/Disks/Disks.tsx | Updated VDisks container width constant |
| src/containers/Storage/Disks/Disks.scss | Updated spacing, sizing, and border-radius values |
| src/components/YDBDefinitionList/YDBDefinitionList.scss | Removed max-width constraint from properties list |
| src/components/VDiskPopup/VDiskPopup.tsx | Changed label logic to show donor OR replication label (not both) |
| src/components/VDisk/VDisk.tsx | Updated donor display logic and added highlighted state |
| src/components/DiskStateProgressBar/DiskStateProgressBar.tsx | Added highlighted prop to component interface |
| src/components/DiskStateProgressBar/DiskStateProgressBar.scss | Added opacity states, hover effects, overflow handling, and updated typography |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24 files reviewed, 1 comment
src/containers/Storage/PaginatedStorageNodesTable/columns/StorageNodesColumns.scss
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24 files reviewed, no comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
24 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
src/containers/Storage/VDisks/VDisks.scss, line 10-12 (link)style: hardcoded
12pxshould use design token
24 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
28 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
28 files reviewed, 1 comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
29 files reviewed, no comments
| return <div className={b('title')}>{`${Math.floor(diskAllocatedPercent)}%`}</div>; | ||
| } | ||
|
|
||
| if (!compact && !(diskAllocatedPercent >= 0) && noDataPlaceholder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!(diskAllocatedPercent >= 0) is hard to read
wouldnt it be better to use diskAllocatedPercent < 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diskAllocatedPercent can be NaN and both of these checks (diskAllocatedPercent < 0 and diskAllocatedPercent >= 0) would be wrong(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I wanted to use this check when diskAllocatedPercent < 0 or NaN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is even more implicit(
I suggest to make code more readable
like isNumerical(diskAllocatedPercent) && diskAllocatedPercent < 0
| () => debounce(hidePopup, delayClose), | ||
| () => | ||
| debounce(() => { | ||
| hidePopup(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like we could just debounce(hidePopup)
| NodeId: 224, | ||
|
|
||
| Severity: 0, | ||
| Severity: 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will Severity 0 be displayed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Severity 0 - is "grey" disks (disks with no available information).
We decided to show and consider Vdisk with no state information like error Vdisk, so I want to color such disks in red. (But this requirement is only about Vdisk, Pdisk with no state information should be grey)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant we changed tests here - but do we test Severity: 0 case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest, for vdisk - no. I can explain:
If we don't have VDiskState - we consider VDisk as a wrong disk and color it in red (tests: Should display as unavailable when no VDiskState is provided)
Cases with Severity: 0 (grey) are possible for VDisks only as a fallback for unsupported vDiskState - I don't know, should we really test this behaviour (or maybe I didn't understand the question)))?
P.S. For PDisk tests include case with Severity: 0 case
| } | ||
| }; | ||
|
|
||
| const handleHidePopup = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: would be good to wrap functions in useCallback to not be recreated on every render (and not cause possible rerenders in child components that depend on these recreated functions)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
29 files reviewed, no comments

Stand: https://nda.ya.ru/t/msLLFRr57PEHGX
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔺
Current: 62.50 MB | Main: 62.48 MB
Diff: +0.01 MB (0.02%)
ℹ️ CI Information
var(--g-border-radius-m),var(--g-spacing-*), etc.)HoverPopupcomponent to useuseEffectfor callback timing, preventing stale closures and improving debounce behavior when moving between adjacent disksuseStorageGroupsSelectedColumns,useStorageNodesSelectedColumns) enabling coordinated highlighting across disk collectionsisTopLevelStorageContexthelper to only enable hover highlighting on storage overview pages, not on detail pages focused on specific nodes/diskshover-dim-column-classmixin that uses:has()selector to dim non-highlighted disks when any disk in the column is hoveredDesign System Improvements
The PR consistently migrates from hardcoded values to design tokens, improving maintainability and visual consistency across the console.
Confidence Score: 5/5
HoverPopupto useuseEffectfor callbacks is a correct fix for potential stale closure issues. Code follows existing patterns and uses TypeScript properly throughout.VDisks.scssabout using design tokens consistentlyImportant Files Changed
File Analysis
entity-state-colorsmixin to support highlighted states with hover colors, addedhover-dim-column-classmixin for hover interactionshighlightedanddarkenedprops for interactive hover statesdarkenedstate for hover effectsSequence Diagram
sequenceDiagram participant User participant Table as Storage Table participant Column as Column Component participant Disks as Disks/VDisks Component participant Item as VDisk/PDisk Item participant HoverPopup participant ProgressBar as DiskStateProgressBar User->>Item: Hover over disk Item->>HoverPopup: onMouseEnter HoverPopup->>HoverPopup: debouncedHandleShowPopup Note over HoverPopup: Delay (delayOpen) HoverPopup->>HoverPopup: setIsPopupVisible(true) HoverPopup->>HoverPopup: useEffect detects internalOpen change HoverPopup->>Item: onShowPopup callback Item->>Column: setHighlightedVDisk(id) Column->>Disks: highlightedVDisk={id} Disks->>Item: highlighted={true} Disks->>Item: darkened={false} (for current disk) Disks->>Item: darkened={true} (for other disks) Item->>ProgressBar: highlighted={true/false}, darkened={true/false} ProgressBar->>ProgressBar: Apply CSS classes with hover colors Note over Table,ProgressBar: CSS hover-dim-column-class mixin<br/>dims non-highlighted disks HoverPopup->>User: Show popup content User->>Item: Mouse leaves disk Item->>HoverPopup: onMouseLeave HoverPopup->>HoverPopup: debouncedHandleHidePopup Note over HoverPopup: Delay (delayClose) HoverPopup->>HoverPopup: setIsPopupVisible(false) HoverPopup->>HoverPopup: useEffect detects internalOpen change HoverPopup->>Item: onHidePopup callback Item->>Column: setHighlightedVDisk(undefined) Column->>Disks: highlightedVDisk={undefined} Disks->>Item: highlighted={false}, darkened={false} Item->>ProgressBar: highlighted={false}, darkened={false} ProgressBar->>ProgressBar: Remove highlight CSS classesGreptile Overview
Greptile Summary
This PR updates the visual design of disk indicators across the storage console with modern design tokens and implements an interactive hover system for better user experience.
Key Improvements:
var(--g-border-radius-m),var(--g-spacing-*), etc.) for better maintainabilityisTopLevelStorageContexthelper to only enable hover effects on overview pages, not on detail pagesuseEffect, preventing timing bugs when moving between adjacent disksTechnical Architecture:
The hover system uses a
hover-dim-column-classmixin with CSS:has()selector to dim non-highlighted disks when any disk in the column is hovered. State is managed at the column level and propagated down to individual disk components.Confidence Score: 5/5
HoverPopupto useuseEffectfor callbacks is a correct fix for potential stale closure issues. Code follows existing patterns, uses TypeScript properly throughout, and all tests have been updated to reflect the new behavior.Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant User participant Table as Storage Table participant Column as Column Component participant Disks as Disks/VDisks Component participant Item as VDisk/PDisk Item participant HoverPopup participant ProgressBar as DiskStateProgressBar User->>Item: Hover over disk Item->>HoverPopup: onMouseEnter HoverPopup->>HoverPopup: debouncedHandleShowPopup Note over HoverPopup: Delay (delayOpen) HoverPopup->>HoverPopup: setIsPopupVisible(true) HoverPopup->>HoverPopup: useEffect detects internalOpen change HoverPopup->>Item: onShowPopup callback Item->>Column: setHighlightedVDisk(id) Column->>Disks: highlightedVDisk={id} Disks->>Item: highlighted={true} Disks->>Item: darkened={false} (for current disk) Disks->>Item: darkened={true} (for other disks) Item->>ProgressBar: highlighted={true/false}, darkened={true/false} ProgressBar->>ProgressBar: Apply CSS classes with hover colors Note over Table,ProgressBar: CSS hover-dim-column-class mixin<br/>dims non-highlighted disks HoverPopup->>User: Show popup content User->>Item: Mouse leaves disk Item->>HoverPopup: onMouseLeave HoverPopup->>HoverPopup: debouncedHandleHidePopup Note over HoverPopup: Delay (delayClose) HoverPopup->>HoverPopup: setIsPopupVisible(false) HoverPopup->>HoverPopup: useEffect detects internalOpen change HoverPopup->>Item: onHidePopup callback Item->>Column: setHighlightedVDisk(undefined) Column->>Disks: highlightedVDisk={undefined} Disks->>Item: highlighted={false}, darkened={false} Item->>ProgressBar: highlighted={false}, darkened={false} ProgressBar->>ProgressBar: Remove highlight CSS classes