Fix code review issues: improve clipboard API usage, optimize performance, and remove type assertions

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>

Author: Copilot

packages/ui/src/components/grid/DataTable.tsx

@@ -151,55 +151,47 @@ export function DataTable<TData, TValue>({
   }
 
   // Copy/Paste functionality
-  const handleCopy = React.useCallback((e: React.ClipboardEvent) => {
-    if (!enableCopyPaste) return
-    
-    const selection = window.getSelection()
-    if (selection && selection.toString()) {
-      // Let browser handle the copy
-      return
-    }
+  const handleCopy = React.useCallback(() => {
+    if (!enableCopyPaste || !hasSelection) return
 
-    // Copy selected rows as TSV
-    if (hasSelection) {
-      e.preventDefault()
-      const headers = enhancedColumns
+    const headers = enhancedColumns
+      .filter(col => col.id !== 'select')
+      .map(col => typeof col.header === 'string' ? col.header : col.id)
+      .join('\t')
+    
+    const rows = selectedRows.map(row => 
+      enhancedColumns
         .filter(col => col.id !== 'select')
-        .map(col => typeof col.header === 'string' ? col.header : col.id)
+        .map(col => {
+          const cell = row.getValue(col.id as string)
+          return cell !== null && cell !== undefined ? String(cell) : ''
+        })
         .join('\t')
-      
-      const rows = selectedRows.map(row => 
-        enhancedColumns
-          .filter(col => col.id !== 'select')
-          .map(col => {
-            const cell = row.getValue(col.id as string)
-            return cell !== null && cell !== undefined ? String(cell) : ''
-          })
-          .join('\t')
-      ).join('\n')
+    ).join('\n')
 
-      const tsv = headers + '\n' + rows
-      e.clipboardData.setData('text/plain', tsv)
-    }
-  }, [enableCopyPaste, hasSelection, selectedRows, enhancedColumns])
-
-  // Add keyboard shortcuts for copy
-  React.useEffect(() => {
-    if (!enableCopyPaste) return
-
-    const handleKeyDown = (e: KeyboardEvent) => {
-      if ((e.ctrlKey || e.metaKey) && e.key === 'c' && hasSelection) {
-        // Copy will be handled by the copy event
-        document.dispatchEvent(new ClipboardEvent('copy', {
-          clipboardData: new DataTransfer(),
-          bubbles: true
-        }))
+    const tsv = headers + '\n' + rows
+    
+    // Use modern clipboard API with error handling
+    if (navigator.clipboard && navigator.clipboard.writeText) {
+      navigator.clipboard.writeText(tsv).catch(err => {
+        console.error('Failed to copy to clipboard:', err)
+      })
+    } else {
+      // Fallback for older browsers
+      const textArea = document.createElement('textarea')
+      textArea.value = tsv
+      textArea.style.position = 'fixed'
+      textArea.style.left = '-999999px'
+      document.body.appendChild(textArea)
+      textArea.select()
+      try {
+        document.execCommand('copy')
+      } catch (err) {
+        console.error('Failed to copy to clipboard:', err)
       }
+      document.body.removeChild(textArea)
     }
-
-    document.addEventListener('keydown', handleKeyDown)
-    return () => document.removeEventListener('keydown', handleKeyDown)
-  }, [enableCopyPaste, hasSelection])
+  }, [enableCopyPaste, hasSelection, selectedRows, enhancedColumns])
 
   const renderFilters = () => {
     // If enableMultipleFilters is true and filterConfigs are provided
@@ -284,7 +276,7 @@ export function DataTable<TData, TValue>({
   }
 
   return (
-    <div onCopy={handleCopy}>
+    <div>
       {/* Bulk actions toolbar */}
       {enableRowSelection && hasSelection && (
         <div className="flex items-center gap-2 p-2 mb-2 bg-blue-50 border border-blue-200 rounded-md">
@@ -296,16 +288,7 @@ export function DataTable<TData, TValue>({
               <Button
                 variant="outline"
                 size="sm"
-                onClick={() => {
-                  const event = new ClipboardEvent('copy', {
-                    clipboardData: new DataTransfer(),
-                    bubbles: true
-                  })
-                  handleCopy(event as any)
-                  if (event.clipboardData) {
-                    navigator.clipboard.writeText(event.clipboardData.getData('text/plain'))
-                  }
-                }}
+                onClick={handleCopy}
                 className="h-8"
               >
                 <Copy className="w-4 h-4 mr-1" />

packages/ui/src/components/grid/GridView.tsx

@@ -63,9 +63,17 @@ export function GridView({
     setColumns(initialColumns)
   }, [initialColumns])
 
-  // Group data if grouping is enabled
-  const groupedData = React.useMemo(() => {
-    if (!enableGrouping || !groupByColumn) return { ungrouped: data }
+  // Group data if grouping is enabled and create index map for performance
+  const { groupedData, rowIndexMap } = React.useMemo(() => {
+    // Create a map for O(1) index lookups
+    const indexMap = new Map<any, number>()
+    data.forEach((row, index) => {
+      indexMap.set(row, index)
+    })
+
+    if (!enableGrouping || !groupByColumn) {
+      return { groupedData: { ungrouped: data }, rowIndexMap: indexMap }
+    }
 
     const groups: Record<string, any[]> = {}
     data.forEach(row => {
@@ -75,13 +83,18 @@ export function GridView({
       }
       groups[groupValue].push(row)
     })
-    return groups
+    return { groupedData: groups, rowIndexMap: indexMap }
   }, [data, enableGrouping, groupByColumn])
 
   const [expandedGroups, setExpandedGroups] = React.useState<Set<string>>(
     new Set(Object.keys(groupedData))
   )
 
+  // Update expanded groups when groupedData changes
+  React.useEffect(() => {
+    setExpandedGroups(new Set(Object.keys(groupedData)))
+  }, [groupedData])
+
   const toggleGroup = (groupKey: string) => {
     setExpandedGroups(prev => {
       const newSet = new Set(prev)
@@ -139,7 +152,28 @@ export function GridView({
       .join('\n')
 
     const tsv = headers + '\n' + rows
-    navigator.clipboard.writeText(tsv)
+    
+    // Use modern clipboard API with error handling
+    if (navigator.clipboard && navigator.clipboard.writeText) {
+      navigator.clipboard.writeText(tsv).catch(err => {
+        console.error('Failed to copy to clipboard:', err)
+        // Fallback: show user a message or use document.execCommand as last resort
+      })
+    } else {
+      // Fallback for older browsers
+      const textArea = document.createElement('textarea')
+      textArea.value = tsv
+      textArea.style.position = 'fixed'
+      textArea.style.left = '-999999px'
+      document.body.appendChild(textArea)
+      textArea.select()
+      try {
+        document.execCommand('copy')
+      } catch (err) {
+        console.error('Failed to copy to clipboard:', err)
+      }
+      document.body.removeChild(textArea)
+    }
   }
 
   // Column drag and drop handlers
@@ -404,8 +438,8 @@ export function GridView({
                     </td>
                   </tr>
                   {expandedGroups.has(groupKey) &&
-                    groupRows.map((row, rowIndex) => {
-                      const actualIndex = data.indexOf(row)
+                    groupRows.map((row) => {
+                      const actualIndex = rowIndexMap.get(row) ?? -1
                       return (
                         <tr
                           key={actualIndex}