Enhance sidebar navigation with icons and mobile support by etsraphael · Pull Request #24 · etsraphael/Forge

etsraphael · 2026-03-24T02:29:14Z

Improve the sidebar navigation by adding icons to each link and implementing mobile support for better accessibility and user experience. The sidebar now features a responsive design that adapts to different screen sizes.

Copilot

Pull request overview

This PR updates the Forge FE app shell to replace the top navigation with a sidebar-based layout that includes per-route icons and a responsive mobile drawer experience.

Changes:

Add icon-backed sidebar navigation items for primary routes.
Implement a responsive layout with a fixed desktop sidebar and a mobile slide-in drawer + overlay.
Refactor nav link rendering into a reusable SidebarLink component (with badge support).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T02:31:54Z

+      {/* Mobile overlay */}
+      {sidebarOpen && (
+        <div
+          className="fixed inset-0 z-40 bg-black/50 lg:hidden animate-in fade-in duration-200"
+          onClick={closeSidebar}
+        />
+      )}


When the mobile sidebar is open, background scrolling isn’t prevented. On touch devices this often causes the main content to scroll behind the overlay. Consider toggling document.body.style.overflow = 'hidden' (and restoring it) while sidebarOpen is true, or using an existing modal/sheet primitive that handles scroll lock.

Copilot · 2026-03-24T02:31:55Z

+    function onKeyDown(e: KeyboardEvent) {
+      if (e.key === 'Escape') setSidebarOpen(false)
+    }
+    document.addEventListener('keydown', onKeyDown)
+    return () => document.removeEventListener('keydown', onKeyDown)
+  }, [])


The Escape key listener is registered for the lifetime of the AppShell, even when the sidebar is closed. Consider only attaching the listener while sidebarOpen is true to reduce global event handling and avoid potential interaction with other Escape-driven UI (dialogs, menus).

Suggested change

function onKeyDown(e: KeyboardEvent) {

if (e.key === 'Escape') setSidebarOpen(false)

}

document.addEventListener('keydown', onKeyDown)

return () => document.removeEventListener('keydown', onKeyDown)

}, [])

if (!sidebarOpen) return

function onKeyDown(e: KeyboardEvent) {

if (e.key === 'Escape') setSidebarOpen(false)

}

document.addEventListener('keydown', onKeyDown)

return () => document.removeEventListener('keydown', onKeyDown)

}, [sidebarOpen])

Copilot · 2026-03-24T02:31:55Z

+          <button
+            onClick={() => setSidebarOpen(true)}
+            className="rounded-lg p-1 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground"
+          >
+            <Menu className="size-5" />
+          </button>


Icon-only buttons (mobile menu toggle and close drawer) are missing an accessible name. Please add an aria-label (and/or title) so screen readers can identify the action (e.g., “Open navigation” / “Close navigation”).

Copilot · 2026-03-24T02:31:55Z

          </div>
+          <button
+            onClick={closeSidebar}
+            className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"


The close button in the mobile drawer is icon-only and currently has no accessible name. Add aria-label (and/or title) to ensure it’s discoverable to assistive tech.

Suggested change

className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"

className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"

aria-label="Close navigation menu"

title="Close navigation menu"

Copilot · 2026-03-24T02:31:55Z

+      <aside
+        className={cn(
+          'fixed inset-y-0 left-0 z-50 flex w-64 flex-col bg-sidebar border-r border-sidebar-border',
+          'transition-transform duration-300 ease-in-out lg:hidden',
+          sidebarOpen ? 'translate-x-0' : '-translate-x-full',
+        )}
+      >
+        {/* Drawer header */}
+        <div className="flex h-14 items-center justify-between px-5">
+          <div className="flex items-center gap-2.5">
+            <Anvil className="size-5 text-sidebar-primary" />
+            <span className="text-lg font-semibold text-sidebar-foreground">
+              Forge
+            </span>
          </div>
+          <button
+            onClick={closeSidebar}
+            className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"
+          >
+            <X className="size-5" />
+          </button>
+        </div>
+        <div className="px-4 pb-3">
+          <ProjectSwitcher />
        </div>
+        <div className="mx-3 border-t border-sidebar-border" />
+        {sidebarContent}
+      </aside>


The mobile sidebar drawer is always mounted and only moved off-screen via translate-x-*. When closed, its links/buttons can still be reachable via keyboard navigation and some assistive tech. Consider conditionally rendering it only when sidebarOpen, or apply inert/aria-hidden/tabIndex management (or a Dialog/Sheet component) so hidden content isn’t focusable.

Suggested change

<aside

className={cn(

'fixed inset-y-0 left-0 z-50 flex w-64 flex-col bg-sidebar border-r border-sidebar-border',

'transition-transform duration-300 ease-in-out lg:hidden',

sidebarOpen ? 'translate-x-0' : '-translate-x-full',

)}

>

{/* Drawer header */}

<div className="flex h-14 items-center justify-between px-5">

<div className="flex items-center gap-2.5">

<Anvil className="size-5 text-sidebar-primary" />

<span className="text-lg font-semibold text-sidebar-foreground">

Forge

</span>

</div>

<button

onClick={closeSidebar}

className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"

>

<X className="size-5" />

</button>

</div>

<div className="px-4 pb-3">

<ProjectSwitcher />

</div>

<div className="mx-3 border-t border-sidebar-border" />

{sidebarContent}

</aside>

{sidebarOpen && (

<aside

className={cn(

'fixed inset-y-0 left-0 z-50 flex w-64 flex-col bg-sidebar border-r border-sidebar-border',

'transition-transform duration-300 ease-in-out lg:hidden',

sidebarOpen ? 'translate-x-0' : '-translate-x-full',

)}

>

{/* Drawer header */}

<div className="flex h-14 items-center justify-between px-5">

<div className="flex items-center gap-2.5">

<Anvil className="size-5 text-sidebar-primary" />

<span className="text-lg font-semibold text-sidebar-foreground">

Forge

</span>

</div>

<button

onClick={closeSidebar}

className="rounded-lg p-1 text-sidebar-foreground/60 transition-colors hover:bg-sidebar-accent hover:text-sidebar-foreground"

>

<X className="size-5" />

</button>

</div>

<div className="px-4 pb-3">

<ProjectSwitcher />

</div>

<div className="mx-3 border-t border-sidebar-border" />

{sidebarContent}

</aside>

)}

claude · 2026-03-24T02:33:50Z

Frontend Code Review (forge-fe/)

claude · 2026-03-24T02:38:53Z

test

claude · 2026-03-24T02:39:11Z

Deleting test comments - this is a placeholder

claude · 2026-03-24T02:40:06Z

Frontend Code Review forge-fe/ - Full Report

SUMMARY TABLE:
Accessibility: High severity, 5 issues
React Best Practices: Medium, 3 issues
TypeScript: Medium, 3 issues
Component Architecture: Medium, 3 issues
Performance: Medium, 3 issues
Security: Medium, 1 issue
Code Quality: Low, 2 issues

SECURITY: XSS risk in markdown rendering at forge-fe/src/components/chat/markdown-content.tsx lines 36-41. react-markdown has no allowedElements prop restricting what HTML elements can render. Fix: add allowedElements prop or sanitize server-side.

ACCESSIBILITY (High Priority):

Icon-only buttons missing aria-label at Connectors.tsx lines 834/990/1000, kanban-column.tsx line 40, task-detail-modal.tsx line 242. Fix: add aria-label to all icon-only buttons.
Decorative icons not hidden: type-icon.tsx and priority-dot.tsx need aria-hidden=true on decorative icons.
Modal focus management: task-detail-modal.tsx and approval-detail-modal.tsx - verify base-ui Dialog handles focus trapping.

REACT BEST PRACTICES:

Duplicate relativeTime function: Chat.tsx lines 47-55 duplicates lib/format.ts lines 3-24. Remove local copy.
Silent error suppression: Chat.tsx lines 313/344/353 have empty catch blocks. Fix: surface errors via state.
Stale closure in streaming loop: Chat.tsx lines 410-430, activeSessionId captured in closure. Fix: use useRef.

TYPESCRIPT:

Untyped MessageEvent data: Connectors.tsx lines 195-206, add MessageEvent typing.
Untyped API responses: Chat.tsx lines 284-290, define interfaces for API responses.
Unexplained type assertions: kanban-board.tsx lines 36/37/94, add explanatory comments.

COMPONENT ARCHITECTURE:

Monolithic Connectors page 1100+ lines - extract github-oauth-form, add-form, category-section.
Inline components in Chat (lines 58-263) - extract SessionSidebar and ModelSelector.

PERFORMANCE:

Hardcoded API endpoints throughout - centralize in lib/api.ts with VITE_API_URL env var.
KanbanCard not memoized - wrap with memo().
Per-token state updates in streaming (lines 392-463) - batch with useTransition.

POSITIVE FINDINGS:

No dangerouslySetInnerHTML
No index-based keys
External links have rel=noopener noreferrer
No exposed secrets
No console.log debug statements
No explicit any types

Review generated by Claude Code

…component

feat: enhance sidebar navigation with icons and mobile support

9b189c9

Copilot AI review requested due to automatic review settings March 24, 2026 02:29

Copilot started reviewing on behalf of etsraphael March 24, 2026 02:29 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

fix: improve error handling for session loading and deletion in Chat …

129d845

…component

etsraphael merged commit 00a5c5f into main Mar 24, 2026
2 checks passed

etsraphael deleted the feature/sidebar branch March 24, 2026 03:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance sidebar navigation with icons and mobile support#24

Enhance sidebar navigation with icons and mobile support#24
etsraphael merged 2 commits intomainfrom
feature/sidebar

etsraphael commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

etsraphael commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Mar 24, 2026

Frontend Code Review (forge-fe/)

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

claude Bot commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants