From a4214276d7d90bb9ab254920de23309b79cd6ef2 Mon Sep 17 00:00:00 2001
From: Stefan de Vogelaere <email@stefandevogelaere.be>
Date: Fri, 23 Jan 2026 01:58:15 +0100
Subject: [PATCH] fix(ui): address PR review comments and fix E2E tests for
 unified sidebar

- Add try/catch for getElectronAPI() in sidebar-footer with window.open fallback
- Use formatShortcut() for OS-aware hotkey display in sidebar-header
- Remove unnecessary optional chaining on project.icon
- Remove redundant ternary in sidebar-navigation className
- Update E2E tests to use new project-dropdown-trigger data-testid

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 .../layout/sidebar/components/sidebar-footer.tsx      |  9 +++++++--
 .../layout/sidebar/components/sidebar-header.tsx      | 11 ++++++++---
 .../layout/sidebar/components/sidebar-navigation.tsx  |  8 +-------
 .../tests/features/feature-manual-review-flow.spec.ts |  9 ++++-----
 apps/ui/tests/projects/new-project-creation.spec.ts   |  9 ++++-----
 apps/ui/tests/projects/open-existing-project.spec.ts  |  9 ++++-----
 6 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx b/apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx
index b407e365..49f4eccf 100644
--- a/apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx
+++ b/apps/ui/src/components/layout/sidebar/components/sidebar-footer.tsx
@@ -51,8 +51,13 @@ export function SidebarFooter({
   }, [navigate]);
 
   const handleFeedbackClick = useCallback(() => {
-    const api = getElectronAPI();
-    api.openExternalLink('https://github.com/AutoMaker-Org/automaker/issues');
+    try {
+      const api = getElectronAPI();
+      api.openExternalLink('https://github.com/AutoMaker-Org/automaker/issues');
+    } catch {
+      // Fallback for non-Electron environments (SSR, web browser)
+      window.open('https://github.com/AutoMaker-Org/automaker/issues', '_blank');
+    }
   }, []);
 
   // Collapsed state
diff --git a/apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx b/apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx
index db4835dd..afca3e9c 100644
--- a/apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx
+++ b/apps/ui/src/components/layout/sidebar/components/sidebar-header.tsx
@@ -4,6 +4,7 @@ import { ChevronsUpDown, Folder, Plus, FolderOpen } from 'lucide-react';
 import * as LucideIcons from 'lucide-react';
 import type { LucideIcon } from 'lucide-react';
 import { cn, isMac } from '@/lib/utils';
+import { formatShortcut } from '@/store/app-store';
 import { isElectron, type Project } from '@/lib/electron';
 import { getAuthenticatedImageUrl } from '@/lib/api-fetch';
 import { useAppStore } from '@/store/app-store';
@@ -49,7 +50,7 @@ export function SidebarHeader({
   );
 
   const getIconComponent = (project: Project): LucideIcon => {
-    if (project?.icon && project.icon in LucideIcons) {
+    if (project.icon && project.icon in LucideIcons) {
       return (LucideIcons as unknown as Record<string, LucideIcon>)[project.icon];
     }
     return Folder;
@@ -200,7 +201,9 @@ export function SidebarHeader({
                         {project.name}
                       </span>
                       {hotkeyLabel && (
-                        <span className="text-xs text-muted-foreground">⌘{hotkeyLabel}</span>
+                        <span className="text-xs text-muted-foreground">
+                          {formatShortcut(`Cmd+${hotkeyLabel}`, true)}
+                        </span>
                       )}
                     </DropdownMenuItem>
                   );
@@ -342,7 +345,9 @@ export function SidebarHeader({
                       {project.name}
                     </span>
                     {hotkeyLabel && (
-                      <span className="text-xs text-muted-foreground">⌘{hotkeyLabel}</span>
+                      <span className="text-xs text-muted-foreground">
+                        {formatShortcut(`Cmd+${hotkeyLabel}`, true)}
+                      </span>
                     )}
                   </DropdownMenuItem>
                 );
diff --git a/apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsx b/apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsx
index f303ad44..4a1ab1fc 100644
--- a/apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsx
+++ b/apps/ui/src/components/layout/sidebar/components/sidebar-navigation.tsx
@@ -97,13 +97,7 @@ export function SidebarNavigation({
   });
 
   return (
-    <nav
-      ref={navRef}
-      className={cn(
-        'flex-1 overflow-y-auto scrollbar-hide px-3 pb-2',
-        sidebarOpen ? 'mt-1' : 'mt-1'
-      )}
-    >
+    <nav ref={navRef} className={cn('flex-1 overflow-y-auto scrollbar-hide px-3 pb-2 mt-1')}>
       {/* Navigation sections */}
       {visibleSections.map((section, sectionIdx) => {
         const isCollapsed = section.label ? collapsedSections[section.label] : false;
diff --git a/apps/ui/tests/features/feature-manual-review-flow.spec.ts b/apps/ui/tests/features/feature-manual-review-flow.spec.ts
index ab8b077d..be776d60 100644
--- a/apps/ui/tests/features/feature-manual-review-flow.spec.ts
+++ b/apps/ui/tests/features/feature-manual-review-flow.spec.ts
@@ -21,7 +21,6 @@ import {
   getKanbanColumn,
   authenticateForTests,
   handleLoginScreenIfPresent,
-  sanitizeForTestId,
 } from '../utils';
 
 const TEST_TEMP_DIR = createTempDirPath('manual-review-test');
@@ -131,10 +130,10 @@ test.describe('Feature Manual Review Flow', () => {
       await page.waitForTimeout(300);
     }
 
-    // Verify we're on the correct project (project switcher button shows project name)
-    // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
-    const sanitizedProjectName = sanitizeForTestId(projectName);
-    await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({
+    // Verify we're on the correct project (project dropdown trigger shows project name)
+    await expect(
+      page.locator('[data-testid="project-dropdown-trigger"]').getByText(projectName)
+    ).toBeVisible({
       timeout: 10000,
     });
 
diff --git a/apps/ui/tests/projects/new-project-creation.spec.ts b/apps/ui/tests/projects/new-project-creation.spec.ts
index 4599e8fe..07d5bc3b 100644
--- a/apps/ui/tests/projects/new-project-creation.spec.ts
+++ b/apps/ui/tests/projects/new-project-creation.spec.ts
@@ -14,7 +14,6 @@ import {
   authenticateForTests,
   handleLoginScreenIfPresent,
   waitForNetworkIdle,
-  sanitizeForTestId,
 } from '../utils';
 
 const TEST_TEMP_DIR = createTempDirPath('project-creation-test');
@@ -78,10 +77,10 @@ test.describe('Project Creation', () => {
     }
 
     // Wait for project to be set as current and visible on the page
-    // The project name appears in the project switcher button
-    // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
-    const sanitizedProjectName = sanitizeForTestId(projectName);
-    await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({
+    // The project name appears in the project dropdown trigger
+    await expect(
+      page.locator('[data-testid="project-dropdown-trigger"]').getByText(projectName)
+    ).toBeVisible({
       timeout: 15000,
     });
 
diff --git a/apps/ui/tests/projects/open-existing-project.spec.ts b/apps/ui/tests/projects/open-existing-project.spec.ts
index 0e3cb789..4d8db61f 100644
--- a/apps/ui/tests/projects/open-existing-project.spec.ts
+++ b/apps/ui/tests/projects/open-existing-project.spec.ts
@@ -18,7 +18,6 @@ import {
   authenticateForTests,
   handleLoginScreenIfPresent,
   waitForNetworkIdle,
-  sanitizeForTestId,
 } from '../utils';
 
 // Create unique temp dir for this test run
@@ -169,11 +168,11 @@ test.describe('Open Project', () => {
     }
 
     // Wait for a project to be set as current and visible on the page
-    // The project name appears in the project switcher button
-    // Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
+    // The project name appears in the project dropdown trigger
     if (targetProjectName) {
-      const sanitizedName = sanitizeForTestId(targetProjectName);
-      await expect(page.locator(`[data-testid$="-${sanitizedName}"]`)).toBeVisible({
+      await expect(
+        page.locator('[data-testid="project-dropdown-trigger"]').getByText(targetProjectName)
+      ).toBeVisible({
         timeout: 15000,
       });
     }