Skip to content

fix(PDiskSpaceDistribution): slot height calculation #3897

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.

Sign up for GitHub

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

Open
Raubzeug wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(PDiskSpaceDistribution): slot height calculation #3897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/containers/PDiskPage/PDiskSpaceDistribution/PDiskSpaceDistribution.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,14 @@

--disk-border-radius: var(--g-border-radius-xs);

.storage-disk-progress-bar {
height: 100%;
}

&__pdisk-bar {
display: flex;
flex-grow: 1;
flex-direction: column;
gap: var(--g-spacing-2);

min-width: 500px;
max-width: 800px;
height: auto;
padding: var(--g-spacing-2);
}

Expand All @@ -37,6 +33,7 @@
display: flex;

width: 100%;
height: 100%;
}

&__slot-content {
Expand Down
50 changes: 39 additions & 11 deletions src/containers/PDiskPage/PDiskSpaceDistribution/PDiskSpaceDistribution.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import React from 'react';

import {DiskStateProgressBar} from '../../../components/DiskStateProgressBar/DiskStateProgressBar';
import {HoverPopup} from '../../../components/HoverPopup/HoverPopup';
import {InternalLink} from '../../../components/InternalLink';
Expand Down Expand Up @@ -26,7 +28,7 @@ import './PDiskSpaceDistribution.scss';

const b = cn('ydb-pdisk-space-distribution');

const SLOT_HEIGHT = 40;
const BASE_SLOT_HEIGHT = 40;

interface PDiskSpaceDistributionProps {
data: PDiskData;
Expand All @@ -38,13 +40,33 @@ export function PDiskSpaceDistribution({data}: PDiskSpaceDistributionProps) {

const {PDiskId, NodeId} = data;

const containerHeight = SLOT_HEIGHT * (SlotItems?.length || 1);
// Find the minimum Total among non-log slots to use as the base unit for height scaling
const minNonLogTotal = React.useMemo(() => {
if (!SlotItems?.length) {
return 1;
}

let minTotal = Infinity;

for (const item of SlotItems) {
if (item.SlotType === 'log') {
continue;
}
const value = Number(item.Total) || 1;
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent invalid totals from becoming a 1-byte baseline

When any non-log slot has a missing, zero, or NaN Total, this fallback makes minNonLogTotal equal to 1. The new height calculation then divides real byte-sized slots by that value, so a normal 20 GB empty slot renders as about 800,000,000,000 px tall. This can happen with partial/older VDisk data where SizeLimit is unavailable while ExpectedSlotCount still creates empty slots; the previous flex layout kept the total bar height bounded, but these fixed pixel heights can make the PDisk page unusable.

Useful? React with 👍 / 👎.

if (value < minTotal) {
minTotal = value;
}
}

return minTotal === Infinity ? 1 : minTotal;
}, [SlotItems]);

const renderSlots = () => {
return SlotItems?.map((item, index) => {
return (
<Slot
item={item}
minNonLogTotal={minNonLogTotal}
pDiskId={PDiskId}
nodeId={NodeId}
getVDiskPagePath={getVDiskPagePath}
Expand All @@ -59,13 +81,7 @@ export function PDiskSpaceDistribution({data}: PDiskSpaceDistributionProps) {
}

return (
<div
className={b(null)}
style={{
height: containerHeight,
minHeight: containerHeight,
}}
>
<div className={b(null)}>
<DiskStateProgressBar
className={b('pdisk-bar')}
severity={data.Severity}
Expand All @@ -80,6 +96,7 @@ export function PDiskSpaceDistribution({data}: PDiskSpaceDistributionProps) {

interface SlotProps<T extends SlotItemType> {
item: SlotItem<T>;
minNonLogTotal: number;

pDiskId?: string | number;
nodeId?: string | number;
Expand All @@ -89,7 +106,12 @@ interface SlotProps<T extends SlotItemType> {
) => string | undefined;
}

function Slot<T extends SlotItemType>({item, nodeId, getVDiskPagePath}: SlotProps<T>) {
function Slot<T extends SlotItemType>({
item,
minNonLogTotal,
nodeId,
getVDiskPagePath,
}: SlotProps<T>) {
const renderContent = () => {
if (isVDiskSlot(item)) {
const vDiskPagePath = getVDiskPagePath?.({
Expand Down Expand Up @@ -175,8 +197,14 @@ function Slot<T extends SlotItemType>({item, nodeId, getVDiskPagePath}: SlotProp
return null;
};

// Log slots get half the base height; others scale proportionally to the smallest non-log slot
const slotHeight =
item.SlotType === 'log'
? BASE_SLOT_HEIGHT / 2
: ((Number(item.Total) || 1) / minNonLogTotal) * BASE_SLOT_HEIGHT;
Comment on lines +200 to +204
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 When item.Total is undefined, null, or 0 for any non-log slot, Number(item.Total) || 1 normalises it to 1. That 1 then wins the minNonLogTotal search, so every other slot with a real capacity — say 100 GB = 107_374_182_400 — computes a height of (107_374_182_400 / 1) × 40 ≈ 4.3 × 10¹²px, effectively freezing the page. Adding a Math.min cap (e.g. 10 × BASE_SLOT_HEIGHT) keeps the layout sensible even when data is incomplete.

Suggested change
// Log slots get half the base height; others scale proportionally to the smallest non-log slot
const slotHeight =
item.SlotType === 'log'
? BASE_SLOT_HEIGHT / 2
: ((Number(item.Total) || 1) / minNonLogTotal) * BASE_SLOT_HEIGHT;
// Log slots get half the base height; others scale proportionally to the smallest non-log slot
const slotHeight =
item.SlotType === 'log'
? BASE_SLOT_HEIGHT / 2
: Math.min(
((Number(item.Total) || 1) / minNonLogTotal) * BASE_SLOT_HEIGHT,
10 * BASE_SLOT_HEIGHT,
);
Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/containers/PDiskPage/PDiskSpaceDistribution/PDiskSpaceDistribution.tsx
Line: 200-204

Comment:
When `item.Total` is `undefined`, `null`, or `0` for any non-log slot, `Number(item.Total) || 1` normalises it to `1`. That `1` then wins the `minNonLogTotal` search, so every other slot with a real capacity — say 100 GB = `107_374_182_400` — computes a height of `(107_374_182_400 / 1) × 40 ≈ 4.3 × 10¹²px`, effectively freezing the page. Adding a `Math.min` cap (e.g. `10 × BASE_SLOT_HEIGHT`) keeps the layout sensible even when data is incomplete.

```suggestion
    // Log slots get half the base height; others scale proportionally to the smallest non-log slot
    const slotHeight =
        item.SlotType === 'log'
            ? BASE_SLOT_HEIGHT / 2
            : Math.min(
                  ((Number(item.Total) || 1) / minNonLogTotal) * BASE_SLOT_HEIGHT,
                  10 * BASE_SLOT_HEIGHT,
              );
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex Fix in Claude Code


return (
<div className={b('slot-wrapper')} style={{flexGrow: Number(item.Total) || 1}}>
<div className={b('slot-wrapper')} style={{height: slotHeight}}>
{renderContent()}
</div>
);
Expand Down
Loading