feat(storage): update vdisk donor/replica visuals and tooltips#3110
Conversation
| background-image: repeating-linear-gradient( | ||
| 135deg, | ||
| transparent 0, | ||
| transparent 4px, |
There was a problem hiding this comment.
lest use variables like
--stripe-width: 4px;
--stripe-step: 8px;
| const color = severity !== undefined && getSeverityColor(severity); | ||
| if (color) { | ||
| mods[color.toLocaleLowerCase()] = true; | ||
| mods[color.toLowerCase()] = true; |
There was a problem hiding this comment.
why did you decide to change it?
| } | ||
|
|
||
| &_striped { | ||
| overflow: hidden; |
There was a problem hiding this comment.
does we really need this prop?
| [EFlag.Grey]: 'unknown', | ||
| [EFlag.Orange]: 'orange', | ||
| [EFlag.Yellow]: 'warning', | ||
| [EFlag.DarkGrey]: 'unknown', |
There was a problem hiding this comment.
but we never get such status from backend - it's not presented in proto https://github.com/ydb-platform/ydb/blob/b97b42caffd37e4a5462751d2336b9df9adc84ab/ydb/core/viewer/protos/viewer.proto#L312
| {headerLabels.map((label, index) => ( | ||
| <Label key={index} theme={label.theme}> | ||
| <Flex gap="1" alignItems="center"> | ||
| {label.icon && <Icon data={label.icon} size={12} />} |
There was a problem hiding this comment.
no need to do this layout. You may put icon as a property of Label component (take a look at its api https://preview.gravity-ui.com/uikit/?path=/docs/components-data-display-label--docs&globals=theme:dark)
| {vDiskPopupKeyset('label_vdisk')} {Recipient.StringifiedId} | ||
| </InternalLink> | ||
| ) : ( | ||
| <div>{Recipient.StringifiedId}</div> |
There was a problem hiding this comment.
do we really need this dummy wrapper?
| } | ||
|
|
||
| const stateTheme = | ||
| stateSeverity !== undefined ? NUMERIC_SEVERITY_LABEL_THEME[stateSeverity] : undefined; |
There was a problem hiding this comment.
isNil seems to fit here and farther is such situations
|
|
||
| const {VDiskState, DonorMode, Replicated} = data; | ||
|
|
||
| if (!VDiskState) { |
There was a problem hiding this comment.
are we sure we want to render stub? maybe show no labels at all? (discuss it with Alina pls)
| Orange: 2, | ||
| Red: 1, | ||
| Grey: 0, | ||
| DarkGrey: -1, |
| } from '../../../types/api/storage'; | ||
| import {EVDiskState} from '../../../types/api/vdisk'; | ||
| import type {TVDiskStateInfo} from '../../../types/api/vdisk'; | ||
| import {valueIsDefined} from '../../../utils'; |
There was a problem hiding this comment.
please use isNil from lodash instead
| name: vDiskPopupKeyset('label_donor'), | ||
| content: ( | ||
| <Flex direction="column"> | ||
| {Donors.map((donor) => { |
There was a problem hiding this comment.
let's move this representaion into separate component (in the same file VDiskPopup). It seems it has same logic for donor and for recipient
| margin-bottom: var(--g-spacing-3); | ||
| } | ||
|
|
||
| .ydb-definition-list__properties-list { |
There was a problem hiding this comment.
| } | ||
|
|
||
| &_compact { | ||
| .ydb-definition-list__title { |
There was a problem hiding this comment.
you may use here variable for ydb-definition-list. Search for $block in scss files!
| inactive, | ||
| setHighlightedVDisk, | ||
| unavailableVDiskWidth, | ||
| withIcon = false, |
There was a problem hiding this comment.
here and other places for default for withIcon. It seems it is redundant, cause undefined is falthy value
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -0,0 +1,14 @@ | |||
| { | |||
| "context_not-available": "not available", | |||
| "label_vdisk": "PDisk", | |||
There was a problem hiding this comment.
syntax: incorrect label key - should be label_pdisk not label_vdisk
| "label_vdisk": "PDisk", | |
| "label_pdisk": "PDisk", |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/PDiskPopup/i18n/en.json
Line: 3:3
Comment:
**syntax:** incorrect label key - should be `label_pdisk` not `label_vdisk`
```suggestion
"label_pdisk": "PDisk",
```
How can I resolve this? If you propose a fix, please make it concise.
| name: pDiskPopupKeyset('label_available'), | ||
| content: ( | ||
| <React.Fragment> | ||
| <Progress |
There was a problem hiding this comment.
discussed to remove progress here
| border-radius: 4px; // to match interactive area with disk shape | ||
| } | ||
|
|
||
| &__vdisk-icon { |
There was a problem hiding this comment.
it seems this class is not used
2 participants
Stand with mocked data: https://nda.ya.ru/t/7XDsYG1w7NHyC8
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔽
Current: 66.02 MB | Main: 66.10 MB
Diff: 0.07 MB (-0.11%)
✅ Bundle size decreased.
ℹ️ CI Information