docs: Add comprehensive code review for readability and maintainability

src/App.test.tsx

@@ -47,7 +47,6 @@ vi.mock('./hooks/useGitHubFormState', () => ({
     setEndDate: vi.fn(),
     setGithubToken: vi.fn(),
     setApiMode: vi.fn(),
-    handleUsernameBlur: vi.fn(),
     validateUsernameFormat: vi.fn(),
     error: null,
     setError: vi.fn(),

src/App.tsx

@@ -1,6 +1,6 @@
 import {
   useState,
-  useEffect,  
+  useEffect,
   useCallback,
   useMemo,
   createContext,
@@ -15,6 +15,7 @@ import { useGitHubDataFetching } from './hooks/useGitHubDataFetching';
 import { useGitHubDataProcessing } from './hooks/useGitHubDataProcessing';
 import { useIndexedDBStorage } from './hooks/useIndexedDBStorage';
 import { GitHubItem } from './types';
+import { isTestEnvironment } from './utils/environment';
 
 import SearchForm from './components/SearchForm';
 import IssuesAndPRsList from './views/IssuesAndPRsList';
@@ -42,12 +43,9 @@ interface FormContextType {
   setStartDate: (date: string) => void;
   setEndDate: (date: string) => void;
   setGithubToken: (token: string) => void;
-  setApiMode: (
-    mode: 'search' | 'events' | 'summary'
-  ) => void;
+  setApiMode: (mode: 'search' | 'events' | 'summary') => void;
   setSearchText: (searchText: string) => void;
   handleSearch: () => void;
-  handleUsernameBlur: () => void;
   validateUsernameFormat: (username: string) => void;
   addAvatarsToCache: (avatarUrls: { [username: string]: string }) => void;
   loading: boolean;
@@ -69,15 +67,9 @@ export function useFormContext() {
   return context;
 }
 
-// eslint-disable-next-line react-refresh/only-export-components
-export const buttonStyles = {
-  height: 28,
-  minWidth: 0,
-  display: 'inline-flex',
-  alignItems: 'center',
-  justifyContent: 'center',
-  gap: 2,
-};
+// Pure comparator for case-insensitive string sorting (outside component to avoid re-creation)
+const caseInsensitiveCompare = (a: string, b: string): number =>
+  a.toLowerCase().localeCompare(b.toLowerCase());
 
 function App() {
   const [isSettingsOpen, setIsSettingsOpen] = useState(false);
@@ -97,7 +89,6 @@ function App() {
     setEndDate,
     setGithubToken,
     setApiMode,
-    handleUsernameBlur,
     validateUsernameFormat,
     addAvatarsToCache,
     error,
@@ -154,19 +145,6 @@ function App() {
     clearSearchItems,
   });
 
-  const avatarUrls = useMemo(() => {
-    // Prioritize cached avatar URLs if they exist, otherwise return empty array
-    // Only return non-empty arrays to prevent unnecessary re-renders
-    if (cachedAvatarUrls && cachedAvatarUrls.length > 0) {
-      return cachedAvatarUrls;
-    }
-    return [];
-  }, [cachedAvatarUrls]);
-
-  // Pure comparator for case-insensitive string sorting
-  const caseInsensitiveCompare = (a: string, b: string): number =>
-    a.toLowerCase().localeCompare(b.toLowerCase());
-
   // Extract unique users from results for search suggestions (with avatars)
   const availableUsers = useMemo(() => {
     const seen = new Set<string>();
@@ -185,15 +163,13 @@ function App() {
 
   // Extract labels from results sorted by frequency (most used first)
   const availableLabels = useMemo(() => {
-    // Build frequency map using reduce (functional approach)
     const labelCounts = results
       .flatMap(item => item.labels?.map(label => label.name) ?? [])
       .reduce((acc, label) => {
         acc.set(label, (acc.get(label) || 0) + 1);
         return acc;
       }, new Map<string, number>());
 
-    // Sort by frequency (descending), then alphabetically for ties
     return Array.from(labelCounts.entries())
       .sort((a, b) => b[1] - a[1] || caseInsensitiveCompare(a[0], b[0]))
       .map(([label]) => label);
@@ -216,27 +192,18 @@ function App() {
 
   useEffect(() => {
     if (initialLoadingCount === 1) {
-      // Skip initial loading in test environment
-      const isTestEnvironment = typeof window !== 'undefined' && 
-        (window.navigator?.userAgent?.includes('jsdom') || 
-         process.env.NODE_ENV === 'test' ||
-         import.meta.env?.MODE === 'test');
-
-      if (isTestEnvironment) {
-        // In tests, immediately exit loading mode
+      if (isTestEnvironment()) {
         setInitialLoadingCount(0);
         setIsDataLoadingComplete(false);
         return;
       }
-      
+
       const startTime = Date.now();
-      const minLoadingTime = 3000; // 3 seconds minimum for better UX
-      
+      const minLoadingTime = 3000;
+
       handleSearch().then(() => {
         const elapsedTime = Date.now() - startTime;
         const remainingTime = Math.max(0, minLoadingTime - elapsedTime);
-
-        // Instead of automatically switching, mark data loading as complete
         setTimeout(() => {
           setIsDataLoadingComplete(true);
         }, remainingTime);
@@ -266,7 +233,6 @@ function App() {
           },
         }}
       >
-        {/* GitVegas Branding - Vertical Layout */}
         <Box
           sx={{
             display: 'flex',
@@ -276,29 +242,19 @@ function App() {
             mb: 4,
           }}
         >
-          {/* GitVegas Title */}
           <Box sx={{ fontSize: [30, 100], fontWeight: 'bold', color: 'fg.default', mb: 4 }}>
             GitVegas
           </Box>
-
-          {/* Slot Machine */}
-          <Box
-            sx={{
-              display: 'flex',
-              alignItems: 'center',
-              justifyContent: 'center',
-              mb: 4,
-            }}
-          >
+
+          <Box sx={{ display: 'flex', alignItems: 'center', justifyContent: 'center', mb: 4 }}>
             <SlotMachineLoader
-              avatarUrls={avatarUrls}
+              avatarUrls={cachedAvatarUrls}
               isLoading={loading}
               isManuallySpinning={isManuallySpinning}
               size="huge"
             />
           </Box>
-
-          {/* Start Button */}
+
           <Box
             sx={{
               display: 'flex',
@@ -352,14 +308,13 @@ function App() {
               🕹️ {isDataLoadingComplete && !loading ? 'Start' : 'Loading...'}
             </Button>
           </Box>
-
-          {/* Messages - Fixed height to prevent layout jumping */}
-          <Box sx={{ 
-            minHeight: '120px', // Fixed height to prevent jumping
+
+          <Box sx={{
+            minHeight: '120px',
             display: 'flex',
             flexDirection: 'column',
             alignItems: 'center',
-            justifyContent: 'center'
+            justifyContent: 'center',
           }}>
             <LoadingIndicator
               loadingProgress={loadingProgress}
@@ -408,57 +363,47 @@ function App() {
             <PageHeader.Title>GitVegas</PageHeader.Title>
           </PageHeader.TitleArea>
           <PageHeader.Actions>
-            {/* Loading indicator - always visible */}
             <LoadingIndicator
               loadingProgress={isEnriching ? 'Enriching PR details...' : loadingProgress}
               isLoading={loading || isEnriching}
               currentUsername={currentUsername}
             />
-
-            {/* Header search */}
+
             <HeaderSearch
               searchText={searchText}
               onSearchChange={setSearchText}
               availableUsers={availableUsers}
               availableLabels={availableLabels}
               availableRepos={availableRepos}
             />
-
-            {/* Mobile-optimized actions */}
+
             <Box
               sx={{
                 display: 'flex',
                 alignItems: 'center',
                 gap: 2,
-                // Hide share button on mobile to save space
                 '@media (max-width: 767px)': {
                   '& > :first-child': { display: 'none' },
                 },
               }}
             >
-              <ShareButton
-                formSettings={formSettings}
-                size="medium"
-              />
+              <ShareButton formSettings={formSettings} size="medium" />
               <IconButton
                 icon={GearIcon}
                 aria-label="Settings"
                 onClick={() => setIsSettingsOpen(true)}
               />
             </Box>
-
-            {/* Slot machine - smaller on mobile */}
+
             <Box
               sx={{
                 '@media (max-width: 767px)': {
-                  '& .slot-machine': {
-                    transform: 'scale(0.8)',
-                  },
+                  '& .slot-machine': { transform: 'scale(0.8)' },
                 },
               }}
             >
               <SlotMachineLoader
-                avatarUrls={avatarUrls}
+                avatarUrls={cachedAvatarUrls}
                 isLoading={loading}
                 isManuallySpinning={isManuallySpinning}
                 size="small"
@@ -468,13 +413,11 @@ function App() {
         </PageHeader>
       </PageLayout.Header>
 
-      <PageLayout.Content 
-        sx={{ 
+      <PageLayout.Content
+        sx={{
           px: 3,
           py: 1,
-          '@media (max-width: 767px)': {
-            px: 2,
-          },
+          '@media (max-width: 767px)': { px: 2 },
         }}
       >
         <FormContext.Provider
@@ -492,7 +435,6 @@ function App() {
             setApiMode,
             setSearchText,
             handleSearch,
-            handleUsernameBlur,
             validateUsernameFormat,
             addAvatarsToCache,
             loading,
@@ -510,12 +452,10 @@ function App() {
             <SummaryView
               items={results}
               rawEvents={indexedDBEvents}
-              indexedDBSearchItems={
-                indexedDBSearchItems as unknown as GitHubItem[]
-              }
+              indexedDBSearchItems={indexedDBSearchItems as unknown as GitHubItem[]}
             />
           ) : (
-            <IssuesAndPRsList results={results} buttonStyles={buttonStyles} />
+            <IssuesAndPRsList results={results} />
           )}
 
           <SettingsDialog
@@ -547,7 +487,6 @@ function App() {
         </Box>
       </PageLayout.Footer>
 
-      {/* PWA Update Notification */}
       <PWAUpdateNotification />
     </PageLayout>
   );

src/components/CopyToClipboardButton.tsx

@@ -5,7 +5,7 @@ import {
   CheckIcon,
 } from '@primer/octicons-react';
 import { useCopyFeedback } from '../hooks/useCopyFeedback';
-import { GitHubItem } from '../types';
+import { GitHubItem, getItemId } from '../types';
 
 interface CopyToClipboardButtonProps {
   item: GitHubItem;
@@ -27,15 +27,15 @@ const CopyToClipboardButton = memo(function CopyToClipboardButton({
   const handleCopy = useCallback(async () => {
     try {
       await navigator.clipboard.writeText(item.html_url);
-      triggerCopy(item.event_id || item.id);
+      triggerCopy(getItemId(item));
       onSuccess?.();
     } catch (error) {
       console.error('Failed to copy link:', error);
       onError?.(error as Error);
     }
   }, [item, triggerCopy, onSuccess, onError]);
 
-  const isItemCopied = isCopied(item.event_id || item.id);
+  const isItemCopied = isCopied(getItemId(item));
 
   return (
     <IconButton

src/components/ItemRow.tsx

@@ -6,7 +6,7 @@ import {
   IssueOpenedIcon,
   RepoIcon,
 } from '@primer/octicons-react';
-import { GitHubItem } from '../types';
+import { GitHubItem, getItemId } from '../types';
 import ActionButtonsRow from './ActionButtonsRow';
 import { getActionVariant } from '../utils/actionUtils';
 
@@ -25,7 +25,6 @@ interface ItemRowProps {
   onSelect?: (id: string | number) => void;
   showCheckbox?: boolean;
   showRepo?: boolean;
-  showUser?: boolean;
   showTime?: boolean;
   size?: 'small' | 'medium';
   groupCount?: number;
@@ -78,7 +77,7 @@ const ItemRow = ({
           {showCheckbox && onSelect && (
             <Checkbox
               checked={selected}
-              onChange={() => onSelect(item.event_id || item.id)}
+              onChange={() => onSelect(getItemId(item))}
               sx={{ flexShrink: 0 }}
             />
           )}
@@ -275,7 +274,7 @@ const ItemRow = ({
             {showCheckbox && onSelect && (
               <Checkbox
                 checked={selected}
-                onChange={() => onSelect(item.event_id || item.id)}
+                onChange={() => onSelect(getItemId(item))}
                 sx={{ flexShrink: 0 }}
               />
             )}

src/components/SearchForm.test.tsx

@@ -24,7 +24,6 @@ const mockSetStartDate = vi.fn();
 const mockSetEndDate = vi.fn();
 const mockSetApiMode = vi.fn();
 const mockHandleSearch = vi.fn();
-const mockHandleUsernameBlur = vi.fn();
 const mockValidateUsernameFormat = vi.fn();
 const mockAddAvatarsToCache = vi.fn();
 
@@ -39,7 +38,6 @@ vi.mock('../App', () => ({
     apiMode: 'events' as const,
     setApiMode: mockSetApiMode,
     handleSearch: mockHandleSearch,
-    handleUsernameBlur: mockHandleUsernameBlur,
     validateUsernameFormat: mockValidateUsernameFormat,
     addAvatarsToCache: mockAddAvatarsToCache,
     loading: false,
@@ -125,15 +123,6 @@ describe('SearchForm', () => {
     });
   });
 
-  it('calls handleUsernameBlur on blur event', () => {
-    render(<SearchForm />);
-    const usernameInput = screen.getByPlaceholderText(/Enter usernames/);
-
-    fireEvent.blur(usernameInput);
-
-    expect(mockHandleUsernameBlur).toHaveBeenCalledTimes(1);
-  });
-
   it('calls handleSearch on form submit', () => {
     render(<SearchForm />);
     const updateButton = screen.getByText('Update');