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>
This commit is contained in:
Stefan de Vogelaere
2026-01-23 01:58:15 +01:00
parent d09da4af20
commit a4214276d7
6 changed files with 28 additions and 27 deletions

View File

@@ -51,8 +51,13 @@ export function SidebarFooter({
}, [navigate]); }, [navigate]);
const handleFeedbackClick = useCallback(() => { const handleFeedbackClick = useCallback(() => {
const api = getElectronAPI(); try {
api.openExternalLink('https://github.com/AutoMaker-Org/automaker/issues'); 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 // Collapsed state

View File

@@ -4,6 +4,7 @@ import { ChevronsUpDown, Folder, Plus, FolderOpen } from 'lucide-react';
import * as LucideIcons from 'lucide-react'; import * as LucideIcons from 'lucide-react';
import type { LucideIcon } from 'lucide-react'; import type { LucideIcon } from 'lucide-react';
import { cn, isMac } from '@/lib/utils'; import { cn, isMac } from '@/lib/utils';
import { formatShortcut } from '@/store/app-store';
import { isElectron, type Project } from '@/lib/electron'; import { isElectron, type Project } from '@/lib/electron';
import { getAuthenticatedImageUrl } from '@/lib/api-fetch'; import { getAuthenticatedImageUrl } from '@/lib/api-fetch';
import { useAppStore } from '@/store/app-store'; import { useAppStore } from '@/store/app-store';
@@ -49,7 +50,7 @@ export function SidebarHeader({
); );
const getIconComponent = (project: Project): LucideIcon => { 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 (LucideIcons as unknown as Record<string, LucideIcon>)[project.icon];
} }
return Folder; return Folder;
@@ -200,7 +201,9 @@ export function SidebarHeader({
{project.name} {project.name}
</span> </span>
{hotkeyLabel && ( {hotkeyLabel && (
<span className="text-xs text-muted-foreground">{hotkeyLabel}</span> <span className="text-xs text-muted-foreground">
{formatShortcut(`Cmd+${hotkeyLabel}`, true)}
</span>
)} )}
</DropdownMenuItem> </DropdownMenuItem>
); );
@@ -342,7 +345,9 @@ export function SidebarHeader({
{project.name} {project.name}
</span> </span>
{hotkeyLabel && ( {hotkeyLabel && (
<span className="text-xs text-muted-foreground">{hotkeyLabel}</span> <span className="text-xs text-muted-foreground">
{formatShortcut(`Cmd+${hotkeyLabel}`, true)}
</span>
)} )}
</DropdownMenuItem> </DropdownMenuItem>
); );

View File

@@ -97,13 +97,7 @@ export function SidebarNavigation({
}); });
return ( return (
<nav <nav ref={navRef} className={cn('flex-1 overflow-y-auto scrollbar-hide px-3 pb-2 mt-1')}>
ref={navRef}
className={cn(
'flex-1 overflow-y-auto scrollbar-hide px-3 pb-2',
sidebarOpen ? 'mt-1' : 'mt-1'
)}
>
{/* Navigation sections */} {/* Navigation sections */}
{visibleSections.map((section, sectionIdx) => { {visibleSections.map((section, sectionIdx) => {
const isCollapsed = section.label ? collapsedSections[section.label] : false; const isCollapsed = section.label ? collapsedSections[section.label] : false;

View File

@@ -21,7 +21,6 @@ import {
getKanbanColumn, getKanbanColumn,
authenticateForTests, authenticateForTests,
handleLoginScreenIfPresent, handleLoginScreenIfPresent,
sanitizeForTestId,
} from '../utils'; } from '../utils';
const TEST_TEMP_DIR = createTempDirPath('manual-review-test'); const TEST_TEMP_DIR = createTempDirPath('manual-review-test');
@@ -131,10 +130,10 @@ test.describe('Feature Manual Review Flow', () => {
await page.waitForTimeout(300); await page.waitForTimeout(300);
} }
// Verify we're on the correct project (project switcher button shows project name) // Verify we're on the correct project (project dropdown trigger shows project name)
// Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName} await expect(
const sanitizedProjectName = sanitizeForTestId(projectName); page.locator('[data-testid="project-dropdown-trigger"]').getByText(projectName)
await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({ ).toBeVisible({
timeout: 10000, timeout: 10000,
}); });

View File

@@ -14,7 +14,6 @@ import {
authenticateForTests, authenticateForTests,
handleLoginScreenIfPresent, handleLoginScreenIfPresent,
waitForNetworkIdle, waitForNetworkIdle,
sanitizeForTestId,
} from '../utils'; } from '../utils';
const TEST_TEMP_DIR = createTempDirPath('project-creation-test'); 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 // Wait for project to be set as current and visible on the page
// The project name appears in the project switcher button // The project name appears in the project dropdown trigger
// Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName} await expect(
const sanitizedProjectName = sanitizeForTestId(projectName); page.locator('[data-testid="project-dropdown-trigger"]').getByText(projectName)
await expect(page.locator(`[data-testid$="-${sanitizedProjectName}"]`)).toBeVisible({ ).toBeVisible({
timeout: 15000, timeout: 15000,
}); });

View File

@@ -18,7 +18,6 @@ import {
authenticateForTests, authenticateForTests,
handleLoginScreenIfPresent, handleLoginScreenIfPresent,
waitForNetworkIdle, waitForNetworkIdle,
sanitizeForTestId,
} from '../utils'; } from '../utils';
// Create unique temp dir for this test run // 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 // Wait for a project to be set as current and visible on the page
// The project name appears in the project switcher button // The project name appears in the project dropdown trigger
// Use ends-with selector since data-testid format is: project-switcher-{id}-{sanitizedName}
if (targetProjectName) { if (targetProjectName) {
const sanitizedName = sanitizeForTestId(targetProjectName); await expect(
await expect(page.locator(`[data-testid$="-${sanitizedName}"]`)).toBeVisible({ page.locator('[data-testid="project-dropdown-trigger"]').getByText(targetProjectName)
).toBeVisible({
timeout: 15000, timeout: 15000,
}); });
} }