fix(proxmox): address second PR review pass — menu positioning + code clarity
Some checks failed
Test / frontend-typecheck (pull_request) Successful in 2m22s
Test / frontend-tests (pull_request) Successful in 1m50s
PR Review Automation / review (pull_request) Successful in 7m30s
Test / rust-fmt-check (pull_request) Failing after 14m24s
Test / rust-clippy (pull_request) Successful in 15m51s
Test / rust-tests (pull_request) Has been cancelled

- VMList VMActionMenu: restore viewport-aware positioning using useEffect +
  ref (reads menuContentRef.current after render, avoiding the react-hooks/refs
  ESLint violation that blocked the previous ref-during-render approach);
  menu flips upward when less than 20px remain below the viewport bottom
- VMList MigrationDialog: extract disabled condition to canSubmitMigration()
  helper for clarity; removes the inline comment in favour of readable code
- proxmox.rs list_proxmox_datastores: add tracing::debug! for generated
  storage IDs and tracing::warn! + early return for entries with empty
  storage names (catches unexpected API edge cases)
This commit is contained in:
Shaun Arman 2026-06-21 16:02:26 -05:00
parent 58b4d59e6d
commit 627373f433
2 changed files with 33 additions and 7 deletions

View File

@ -715,6 +715,11 @@ pub async fn list_proxmox_datastores(
} else { } else {
format!("storage/{}/{}", node_name, storage_name) format!("storage/{}/{}", node_name, storage_name)
}; };
if storage_name.is_empty() {
tracing::warn!(node = node_name, "storage entry has empty storage name — skipping");
return None;
}
tracing::debug!(storage_id = %storage_id, "generated storage ID");
normalized.insert("id".to_string(), serde_json::Value::String(storage_id)); normalized.insert("id".to_string(), serde_json::Value::String(storage_id));
normalized.insert( normalized.insert(
"storage".to_string(), "storage".to_string(),

View File

@ -1,4 +1,5 @@
import React, { useState, useEffect, useCallback, useRef } from 'react'; import React, { useState, useEffect, useCallback, useRef } from 'react';
import { clsx } from 'clsx';
import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/index'; import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/index';
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/index'; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/index';
import { Button } from '@/components/ui/index'; import { Button } from '@/components/ui/index';
@ -461,11 +462,13 @@ function VMActionMenu({
onDelete, onDelete,
}: VMActionMenuProps) { }: VMActionMenuProps) {
const [isOpen, setIsOpen] = useState(false); const [isOpen, setIsOpen] = useState(false);
const menuRef = useRef<HTMLDivElement>(null); const [flipUpward, setFlipUpward] = useState(false);
const containerRef = useRef<HTMLDivElement>(null);
const menuContentRef = useRef<HTMLDivElement>(null);
useEffect(() => { useEffect(() => {
function handleClickOutside(event: MouseEvent) { function handleClickOutside(event: MouseEvent) {
if (menuRef.current && !menuRef.current.contains(event.target as Node)) { if (containerRef.current && !containerRef.current.contains(event.target as Node)) {
setIsOpen(false); setIsOpen(false);
} }
} }
@ -479,6 +482,14 @@ function VMActionMenu({
}; };
}, [isOpen]); }, [isOpen]);
// After the menu renders, check whether it overflows the viewport bottom and flip if needed.
// Done in useEffect (not during render) to avoid the react-hooks/refs ESLint violation.
useEffect(() => {
if (!isOpen || !menuContentRef.current) return;
const rect = menuContentRef.current.getBoundingClientRect();
setFlipUpward(window.innerHeight - rect.bottom < 20);
}, [isOpen]);
const toggleMenu = (e: React.MouseEvent) => { const toggleMenu = (e: React.MouseEvent) => {
e.stopPropagation(); e.stopPropagation();
setIsOpen(!isOpen); setIsOpen(!isOpen);
@ -491,7 +502,7 @@ function VMActionMenu({
}; };
return ( return (
<div className="relative" ref={menuRef}> <div className="relative" ref={containerRef}>
<Button <Button
variant="ghost" variant="ghost"
size="sm" size="sm"
@ -501,7 +512,13 @@ function VMActionMenu({
<MoreHorizontal className="h-4 w-4" /> <MoreHorizontal className="h-4 w-4" />
</Button> </Button>
{isOpen && ( {isOpen && (
<div className="absolute right-0 top-full z-50 mt-2 w-48 rounded-md border bg-background shadow-md"> <div
ref={menuContentRef}
className={clsx(
'absolute right-0 z-50 w-48 rounded-md border bg-background shadow-md',
flipUpward ? 'bottom-full mb-2' : 'top-full mt-2',
)}
>
<div className="space-y-1 p-1"> <div className="space-y-1 p-1">
{vm.status === 'stopped' && ( {vm.status === 'stopped' && (
<button <button
@ -639,6 +656,12 @@ function MigrationDialog({
.map((v) => v.node) .map((v) => v.node)
.filter((node, index, self) => self.indexOf(node) === index && node !== vm.node); .filter((node, index, self) => self.indexOf(node) === index && node !== vm.node);
const canSubmitMigration = () => {
if (!targetNode) return false;
if (isCrossCluster) return true;
return availableTargets.length > 0;
};
return ( return (
<Dialog open={isOpen} onOpenChange={onClose}> <Dialog open={isOpen} onOpenChange={onClose}>
<DialogContent> <DialogContent>
@ -748,11 +771,9 @@ function MigrationDialog({
<Button variant="outline" onClick={onClose}> <Button variant="outline" onClick={onClose}>
Cancel Cancel
</Button> </Button>
{/* Disabled when: no target node typed/selected,
OR same-cluster migration with no enumerated nodes to choose from */}
<Button <Button
onClick={onSubmit} onClick={onSubmit}
disabled={!targetNode || (!isCrossCluster && availableTargets.length === 0)} disabled={!canSubmitMigration()}
> >
Start Migration Start Migration
</Button> </Button>