From 627373f4331f20aafe4587b0b8dab0ec10908df4 Mon Sep 17 00:00:00 2001
From: Shaun Arman <shaun.arman@motorolasolutions.com>
Date: Sun, 21 Jun 2026 16:02:26 -0500
Subject: [PATCH] =?UTF-8?q?fix(proxmox):=20address=20second=20PR=20review?=
 =?UTF-8?q?=20pass=20=E2=80=94=20menu=20positioning=20+=20code=20clarity?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- 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)
---
 src-tauri/src/commands/proxmox.rs |  5 +++++
 src/components/Proxmox/VMList.tsx | 35 ++++++++++++++++++++++++-------
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/src-tauri/src/commands/proxmox.rs b/src-tauri/src/commands/proxmox.rs
index 923e77d0..026125be 100644
--- a/src-tauri/src/commands/proxmox.rs
+++ b/src-tauri/src/commands/proxmox.rs
@@ -715,6 +715,11 @@ pub async fn list_proxmox_datastores(
             } else {
                 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(
                 "storage".to_string(),
diff --git a/src/components/Proxmox/VMList.tsx b/src/components/Proxmox/VMList.tsx
index 7eeb8b7b..9665b7b4 100644
--- a/src/components/Proxmox/VMList.tsx
+++ b/src/components/Proxmox/VMList.tsx
@@ -1,4 +1,5 @@
 import React, { useState, useEffect, useCallback, useRef } from 'react';
+import { clsx } from 'clsx';
 import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/index';
 import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/index';
 import { Button } from '@/components/ui/index';
@@ -461,11 +462,13 @@ function VMActionMenu({
   onDelete,
 }: VMActionMenuProps) {
   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(() => {
     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);
       }
     }
@@ -479,6 +482,14 @@ function VMActionMenu({
     };
   }, [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) => {
     e.stopPropagation();
     setIsOpen(!isOpen);
@@ -491,7 +502,7 @@ function VMActionMenu({
   };
 
   return (
-    <div className="relative" ref={menuRef}>
+    <div className="relative" ref={containerRef}>
       <Button
         variant="ghost"
         size="sm"
@@ -501,7 +512,13 @@ function VMActionMenu({
         <MoreHorizontal className="h-4 w-4" />
       </Button>
       {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">
             {vm.status === 'stopped' && (
               <button
@@ -639,6 +656,12 @@ function MigrationDialog({
     .map((v) => v.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 (
     <Dialog open={isOpen} onOpenChange={onClose}>
       <DialogContent>
@@ -748,11 +771,9 @@ function MigrationDialog({
           <Button variant="outline" onClick={onClose}>
             Cancel
           </Button>
-          {/* Disabled when: no target node typed/selected,
-              OR same-cluster migration with no enumerated nodes to choose from */}
           <Button
             onClick={onSubmit}
-            disabled={!targetNode || (!isCrossCluster && availableTargets.length === 0)}
+            disabled={!canSubmitMigration()}
           >
             Start Migration
           </Button>