"feat: add role-aware keyboard shortcuts (re-applied via patch)"#1691
Merged
ritesh-1918 merged 1 commit intoJun 5, 2026
Merged
Conversation
|
@isnotsafety is attempting to deploy a commit to the ritesh Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR implements a keyboard shortcuts system for rapid admin navigation. It adds a ChangesKeyboard Shortcuts System
App Routing Restructure
Sequence DiagramsequenceDiagram
participant User
participant Window as Window Keydown Listener
participant Hook as useKeyboardShortcuts Hook
participant Resolver as getShortcutAction
participant Navigator as React Router Navigate
participant UI as Help Modal / Search UI
User->>Window: Press 'g' then 'd'
Window->>Hook: keydown event (key='g')
Hook->>Hook: Check if typing target
Hook->>Hook: Store pending key='g', set timeout
User->>Window: Press 'd'
Window->>Hook: keydown event (key='d')
Hook->>Resolver: Resolve 'g+d' in shortcuts map
Resolver-->>Hook: { action: 'navigate', path: '/admin/dashboard' }
Hook->>Navigator: Navigate to /admin/dashboard
Hook->>Hook: Clear pending key
Navigator-->>UI: Route updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/App.jsx (1)
146-181:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRefresh the title mapping and mount
TitleUpdateron the main app router.The title table still points at pre-refactor paths like
/dashboard,/create-ticket,/admin/ticket/,/master-admin/all-admins, and/master-admin/admin-requests, while the current routes are/user/...,/admin/tickets/:id,/master-admin/admins, and/master-admin/pending-requests. Also,TitleUpdateris only mounted in the docs-subdomain branch, so the main app never updatesdocument.titleat all.Also applies to: 273-319, 338-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/App.jsx` around lines 146 - 181, The title mapping uses outdated route checks and TitleUpdater is not mounted for the main app, so document.title is never updated for current routes; update all path.startsWith and exact path comparisons in App.jsx to the new routes (e.g., replace '/dashboard' -> '/user/dashboard', '/create-ticket' -> '/user/create-ticket', '/admin/ticket/' -> '/admin/tickets/', '/master-admin/all-admins' -> '/master-admin/admins', '/master-admin/admin-requests' -> '/master-admin/pending-requests') and ensure the TitleUpdater component is mounted on the main router (not only the docs-subdomain branch) so TitleUpdater runs and sets document.title using the updated mapping and the existing document.title = `${title} | HELPDESK.AI`; reference the TitleUpdater mount point and the title-assignment logic in App.jsx when making changes.
🧹 Nitpick comments (5)
Frontend/src/hooks/useKeyboardShortcuts.js (2)
210-210: ⚡ Quick winReplace deprecated
navigator.platformwith modern API.
navigator.platformis deprecated. Consider usingnavigator.userAgentData?.platformwith a fallback, or detect macOS via user agent string.♻️ Suggested replacement
export const formatShortcut = (shortcut) => { - const isMac = navigator.platform.toUpperCase().indexOf('MAC') >= 0; + const isMac = navigator.userAgentData?.platform?.toUpperCase().includes('MAC') + ?? navigator.platform?.toUpperCase().includes('MAC') + ?? false; return shortcutAlternatively, for broader compatibility:
- const isMac = navigator.platform.toUpperCase().indexOf('MAC') >= 0; + const isMac = /Mac|iPhone|iPad|iPod/.test(navigator.userAgent);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/hooks/useKeyboardShortcuts.js` at line 210, The isMac detection uses the deprecated navigator.platform; update the logic in useKeyboardShortcuts.js to prefer navigator.userAgentData?.platform and fall back to checking navigator.userAgent for mac identifiers (e.g., "Mac", "Macintosh", "Mac OS") ensuring a boolean isMac result; locate the isMac constant and replace its assignment with a safe feature-detection sequence that handles undefined userAgentData and normalizes casing before testing.
59-62: ⚡ Quick winConsider removing unused
customShortcuts?.isAdminnormalization inuseKeyboardShortcuts.
useKeyboardShortcutscurrently overridesroleand dropscustomShortcutswhencustomShortcuts.isAdminis set, but all in-repo callers pass{}as the first argument and providerolevia theoptionsobject (App.jsx,AdminLayout.jsx, andkeyboard_shortcuts_hook.test.jsx). IfcustomShortcuts.isAdminisn’t part of the intended public API, removing these branches would simplify the hook’s surface area.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/hooks/useKeyboardShortcuts.js` around lines 59 - 62, Remove the unused admin-normalization branches in useKeyboardShortcuts: delete the conditional creation of normalizedOptions that overrides role via customShortcuts?.isAdmin and the normalizedCustomShortcuts assignment that empties customShortcuts when customShortcuts?.isAdmin is truthy; instead pass options and customShortcuts through directly (use options and customShortcuts as-is), update any references to normalizedOptions/normalizedCustomShortcuts inside useKeyboardShortcuts to use options/customShortcuts, and run/update the tests (keyboard_shortcuts_hook.test.jsx) to reflect that customShortcuts.isAdmin is not a supported input.Frontend/src/admin/layout/AdminLayout.jsx (1)
18-18: ⚖️ Poor tradeoffNote: Inconsistent state management pattern with App.jsx.
AdminLayout uses the hook's internal
showHelpstate, whileApp.jsx(lines 220-221) manages external state and passes anonShortcutsHelpcallback. Both patterns work, but the inconsistency may confuse future maintainers. Consider standardizing on one approach across the codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/admin/layout/AdminLayout.jsx` at line 18, AdminLayout currently uses internal hook state (const { showHelp, setShowHelp } = useKeyboardShortcuts({}, { role: 'admin' })) while App.jsx uses external state with an onShortcutsHelp callback; standardize to one pattern by refactoring AdminLayout to accept props for showHelp and onShortcutsHelp (or mirror App.jsx by lifting state into App and passing handlers down), update AdminLayout to call the passed onShortcutsHelp instead of setShowHelp and use the passed showHelp value, and ensure useKeyboardShortcuts is invoked only to register listeners (or returns callbacks) so state ownership is consistent with App.jsx (refer to useKeyboardShortcuts, showHelp, setShowHelp, AdminLayout and onShortcutsHelp).Frontend/src/hooks/keyboard_shortcuts_hook.test.jsx (1)
5-18: ⚖️ Poor tradeoffConsider expanding test coverage for the hook.
While the config module has comprehensive tests in
keyboard_shortcuts.test.js, this hook test only validates one scenario (adming+dnavigation). Consider adding tests for:
- User role shortcuts
- Quick actions (Ctrl+F, Ctrl+K, ?)
- Callback invocations (onSearch, onShortcutsHelp)
- Pending key timeout behavior
- Typing target suppression (shortcuts disabled in inputs)
- Escape key handling
This would improve confidence in the hook's runtime behavior and catch regressions in event handling logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/hooks/keyboard_shortcuts_hook.test.jsx` around lines 5 - 18, Add unit tests to expand coverage of useKeyboardShortcuts: create additional test cases that renderHook(() => useKeyboardShortcuts(...)) for different roles (e.g., user vs admin) and assert navigation via mockedNavigate for role-specific sequences; simulate quick actions by firing Ctrl+F, Ctrl+K and '?' and verify onSearch and onShortcutsHelp callbacks are invoked (pass spies as onSearch/onShortcutsHelp props); test pending key timeout by sending a leading key (e.g., 'g'), advancing timers, then the second key to ensure sequence expires; verify typing target suppression by focusing an input/textarea and firing shortcut keys to assert callbacks/navigations are not triggered; and assert Escape clears pending sequences by firing 'g' then 'Escape' then another key and confirming no navigation—use existing helpers like renderHook, fireEvent, mockedNavigate and jest timers/spies to implement each case.Frontend/src/hooks/keyboard_shortcuts_config.js (1)
75-78: ⚡ Quick winExtract the combo normalization logic into a shared helper.
The pattern
.join('+').replace('g+', 'g,')appears in bothgetShortcutAction(line 76) andgetShortcutDescription(lines 99-101), creating duplication. Consider extracting this into a helper function likenormalizeCombo(keys)to centralize the transformation logic and improve maintainability.♻️ Suggested refactor
+const normalizeCombo = (keys) => { + return keys.map(normalizeShortcutKey).join('+').replace('g+', 'g,'); +}; + export const getShortcutAction = (shortcuts, keys) => { - const combo = keys.map(normalizeShortcutKey).join('+').replace('g+', 'g,'); + const combo = normalizeCombo(keys); return shortcuts[combo] || null; }; export const getShortcutDescription = (shortcut) => { const normalizedShortcut = String(shortcut).toLowerCase(); const match = SHORTCUT_GROUPS .flatMap((group) => group.shortcuts) .find( (entry) => - entry.keys - .map(normalizeShortcutKey) - .join('+') - .replace('g+', 'g,') === normalizedShortcut + normalizeCombo(entry.keys) === normalizedShortcut ); return match?.description || shortcut; };Also applies to: 92-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/hooks/keyboard_shortcuts_config.js` around lines 75 - 78, The combo normalization logic (.map(normalizeShortcutKey).join('+').replace('g+', 'g,')) is duplicated in getShortcutAction and getShortcutDescription; extract it into a shared helper function (e.g., normalizeCombo(keys)) that takes the keys array, maps with normalizeShortcutKey, joins with '+', and applies the 'g+' → 'g,' replacement, then update getShortcutAction and getShortcutDescription to call normalizeCombo(keys) and return shortcuts[combo] || null (or the existing description logic) respectively to centralize transformation and remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Frontend/src/admin/components/ShortcutsHelpModal.jsx`:
- Around line 5-16: The shortcuts array in ShortcutsHelpModal.jsx is hardcoded
and duplicates the centralized SHORTCUT_GROUPS config; replace the local const
shortcuts with an import from keyboard_shortcuts_config.js and derive the list
used by this component (ShortcutsHelpModal) from SHORTCUT_GROUPS (filter/flatten
the groups to the same {keys,label} shape the modal expects), removing the
duplicated array and updating any references to use the imported data so all
shortcut definitions come from the single source of truth.
In `@Frontend/src/App.jsx`:
- Around line 359-389: There is an early return that renders the docs-only
router (the JSX starting with ThemeProvider/BrowserRouter/Routes that mounts
DocsPortal and wildcard Route) which short-circuits the later app tree (the
BrowserRouter/ErrorBoundary/Suspense/AppContent block); remove or conditionalize
that first return so the real application router (AppContent within
ErrorBoundary and Suspense) mounts on the main host—specifically, eliminate the
duplicate top-level return or move the Docs-only Routes behind a host check and
ensure only one BrowserRouter is rendered, keeping AppContent, ErrorBoundary,
Suspense, Toaster, BugReportWidget, ScrollToTopButton/BackToTop in the final
returned tree.
- Around line 213-217: The runtime ReferenceError is caused by missing
imports/definitions: add/import the hook useRealtimeNotifications and the
components ThemeProvider and BackToTopButton (or rename usages to the existing
ScrollToTopButton/BackToTop and correct names) in Frontend/src/App.jsx so the
identifiers used inside AppContent and the JSX (useRealtimeNotifications,
ThemeProvider, BackToTopButton) are defined; update the import block to import
useRealtimeNotifications from its hook module and ThemeProvider/BackToTopButton
from their component modules (or change the JSX to use the already-imported
ScrollToTopButton/BackToTop symbols) so references match available exports.
- Around line 80-86: The routes render lazy components (DocsPortal,
ApiReference, Changelog, StatusPage) directly without a React.Suspense boundary,
which will break on load; wrap the route branches that return these lazy
components (both docs route tables and the branches around where
DocsPortal/ApiReference/Changelog/StatusPage are used) in a Suspense with an
appropriate fallback so the lazy imports can load safely (import/use
React.Suspense or the Suspense component already in scope and place the fallback
around each branch or around the group of docs routes).
- Around line 97-138: There are duplicate lazy() declarations and missing
symbols causing compile/runtime errors; remove the duplicate block so each
component (e.g., UserLayout, AdminLayout, MasterAdminLayout, Dashboard,
CreateTicket, AIProcessing, MasterAdminLogin, etc.) is declared exactly once and
resolve the AdminScorecard conflict by keeping the correct path (choose either
AdminScorecard from ./admin/pages/AdminScorecard or AgentScorecard from
./admin/components/AgentScorecard) so the identifier is unique. Import or define
the missing symbols useRealtimeNotifications, ThemeProvider, and BackToTopButton
and call/use them where referenced (e.g., inside AppContent and the main App
wrapper) so they exist at runtime. Wrap all React.lazy-loaded route trees
(DocsPortal/ApiReference and the main AppContent routes) in a Suspense boundary
so lazy components can render, and fix the unreachable router by removing the
premature return that causes only docs routes to mount so AppContent is returned
when applicable. Finally move TitleUpdater out of the docs-only branch (mount
globally) and update its pathname predicates to match current nested routes
(e.g., /admin/tickets/:id, /tickets/:id, /user/dashboard or /dashboard) so
titles update correctly.
---
Outside diff comments:
In `@Frontend/src/App.jsx`:
- Around line 146-181: The title mapping uses outdated route checks and
TitleUpdater is not mounted for the main app, so document.title is never updated
for current routes; update all path.startsWith and exact path comparisons in
App.jsx to the new routes (e.g., replace '/dashboard' -> '/user/dashboard',
'/create-ticket' -> '/user/create-ticket', '/admin/ticket/' ->
'/admin/tickets/', '/master-admin/all-admins' -> '/master-admin/admins',
'/master-admin/admin-requests' -> '/master-admin/pending-requests') and ensure
the TitleUpdater component is mounted on the main router (not only the
docs-subdomain branch) so TitleUpdater runs and sets document.title using the
updated mapping and the existing document.title = `${title} | HELPDESK.AI`;
reference the TitleUpdater mount point and the title-assignment logic in App.jsx
when making changes.
---
Nitpick comments:
In `@Frontend/src/admin/layout/AdminLayout.jsx`:
- Line 18: AdminLayout currently uses internal hook state (const { showHelp,
setShowHelp } = useKeyboardShortcuts({}, { role: 'admin' })) while App.jsx uses
external state with an onShortcutsHelp callback; standardize to one pattern by
refactoring AdminLayout to accept props for showHelp and onShortcutsHelp (or
mirror App.jsx by lifting state into App and passing handlers down), update
AdminLayout to call the passed onShortcutsHelp instead of setShowHelp and use
the passed showHelp value, and ensure useKeyboardShortcuts is invoked only to
register listeners (or returns callbacks) so state ownership is consistent with
App.jsx (refer to useKeyboardShortcuts, showHelp, setShowHelp, AdminLayout and
onShortcutsHelp).
In `@Frontend/src/hooks/keyboard_shortcuts_config.js`:
- Around line 75-78: The combo normalization logic
(.map(normalizeShortcutKey).join('+').replace('g+', 'g,')) is duplicated in
getShortcutAction and getShortcutDescription; extract it into a shared helper
function (e.g., normalizeCombo(keys)) that takes the keys array, maps with
normalizeShortcutKey, joins with '+', and applies the 'g+' → 'g,' replacement,
then update getShortcutAction and getShortcutDescription to call
normalizeCombo(keys) and return shortcuts[combo] || null (or the existing
description logic) respectively to centralize transformation and remove
duplication.
In `@Frontend/src/hooks/keyboard_shortcuts_hook.test.jsx`:
- Around line 5-18: Add unit tests to expand coverage of useKeyboardShortcuts:
create additional test cases that renderHook(() => useKeyboardShortcuts(...))
for different roles (e.g., user vs admin) and assert navigation via
mockedNavigate for role-specific sequences; simulate quick actions by firing
Ctrl+F, Ctrl+K and '?' and verify onSearch and onShortcutsHelp callbacks are
invoked (pass spies as onSearch/onShortcutsHelp props); test pending key timeout
by sending a leading key (e.g., 'g'), advancing timers, then the second key to
ensure sequence expires; verify typing target suppression by focusing an
input/textarea and firing shortcut keys to assert callbacks/navigations are not
triggered; and assert Escape clears pending sequences by firing 'g' then
'Escape' then another key and confirming no navigation—use existing helpers like
renderHook, fireEvent, mockedNavigate and jest timers/spies to implement each
case.
In `@Frontend/src/hooks/useKeyboardShortcuts.js`:
- Line 210: The isMac detection uses the deprecated navigator.platform; update
the logic in useKeyboardShortcuts.js to prefer navigator.userAgentData?.platform
and fall back to checking navigator.userAgent for mac identifiers (e.g., "Mac",
"Macintosh", "Mac OS") ensuring a boolean isMac result; locate the isMac
constant and replace its assignment with a safe feature-detection sequence that
handles undefined userAgentData and normalizes casing before testing.
- Around line 59-62: Remove the unused admin-normalization branches in
useKeyboardShortcuts: delete the conditional creation of normalizedOptions that
overrides role via customShortcuts?.isAdmin and the normalizedCustomShortcuts
assignment that empties customShortcuts when customShortcuts?.isAdmin is truthy;
instead pass options and customShortcuts through directly (use options and
customShortcuts as-is), update any references to
normalizedOptions/normalizedCustomShortcuts inside useKeyboardShortcuts to use
options/customShortcuts, and run/update the tests
(keyboard_shortcuts_hook.test.jsx) to reflect that customShortcuts.isAdmin is
not a supported input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0aca1a74-14cc-4a84-8994-6a3d04b23df0
📒 Files selected for processing (8)
Frontend/src/App.jsxFrontend/src/admin/components/ShortcutsHelpModal.jsxFrontend/src/admin/layout/AdminLayout.jsxFrontend/src/components/shared/ShortcutsHelp.jsxFrontend/src/hooks/keyboard_shortcuts_config.jsFrontend/src/hooks/keyboard_shortcuts_hook.test.jsxFrontend/src/hooks/useKeyboardShortcuts.jsFrontend/tests/keyboard_shortcuts.test.js
Comment on lines
+5
to
+16
| const shortcuts = [ | ||
| { keys: 'G + D', label: 'Go to Dashboard' }, | ||
| { keys: 'G + T', label: 'Go to Tickets' }, | ||
| { keys: 'G + S', label: 'Go to Settings' }, | ||
| { keys: 'G + P', label: 'Go to Profile' }, | ||
| { keys: 'G + H', label: 'Go to Help' }, | ||
| { keys: 'G + U', label: 'Go to Users (admin)' }, | ||
| { keys: 'G + A', label: 'Go to Analytics (admin)' }, | ||
| { keys: 'Ctrl + F', label: 'Focus search bar' }, | ||
| { keys: '?', label: 'Toggle this help' }, | ||
| { keys: 'Esc', label: 'Close this help' }, | ||
| ]; |
There was a problem hiding this comment.
Eliminate hardcoded shortcuts duplication.
The shortcuts array duplicates configuration already defined in keyboard_shortcuts_config.js (lines 33-55 of that file define SHORTCUT_GROUPS). This creates a maintenance hazard—any shortcut changes must be synchronized across both files. ShortcutsHelp.jsx correctly imports and uses the centralized config; this component should do the same.
♻️ Proposed refactor to use shared config
import React, { useEffect } from 'react';
import { X, Keyboard } from 'lucide-react';
import { Card, CardHeader, CardTitle, CardContent } from '../../components/ui/card';
+import { SHORTCUT_GROUPS } from '../../hooks/keyboard_shortcuts_config';
-const shortcuts = [
- { keys: 'G + D', label: 'Go to Dashboard' },
- { keys: 'G + T', label: 'Go to Tickets' },
- { keys: 'G + S', label: 'Go to Settings' },
- { keys: 'G + P', label: 'Go to Profile' },
- { keys: 'G + H', label: 'Go to Help' },
- { keys: 'G + U', label: 'Go to Users (admin)' },
- { keys: 'G + A', label: 'Go to Analytics (admin)' },
- { keys: 'Ctrl + F', label: 'Focus search bar' },
- { keys: '?', label: 'Toggle this help' },
- { keys: 'Esc', label: 'Close this help' },
-];
+const shortcuts = SHORTCUT_GROUPS.flatMap(group =>
+ group.shortcuts.map(s => ({
+ keys: s.keys.join(' + '),
+ label: s.description
+ }))
+);
const ShortcutsHelpModal = ({ isOpen, onClose, isAdmin = false }) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/admin/components/ShortcutsHelpModal.jsx` around lines 5 - 16,
The shortcuts array in ShortcutsHelpModal.jsx is hardcoded and duplicates the
centralized SHORTCUT_GROUPS config; replace the local const shortcuts with an
import from keyboard_shortcuts_config.js and derive the list used by this
component (ShortcutsHelpModal) from SHORTCUT_GROUPS (filter/flatten the groups
to the same {keys,label} shape the modal expects), removing the duplicated array
and updating any references to use the imported data so all shortcut definitions
come from the single source of truth.
Comment on lines
+80
to
+86
| const ContactSales = lazy(() => import('./pages/ContactSales')); | ||
| const ApiReference = lazy(() => import('./pages/ApiReference')); | ||
| const Changelog = lazy(() => import('./pages/Changelog')); | ||
| const StatusPage = lazy(() => import('./pages/StatusPage')); | ||
| const AboutUs = lazy(() => import('./pages/AboutUs')); | ||
| const Careers = lazy(() => import('./pages/Careers')); | ||
| const DocsPortal = lazy(() => import('./docs/pages/DocsPortal')); |
There was a problem hiding this comment.
Wrap the docs route branches in Suspense before rendering lazy pages.
DocsPortal, ApiReference, Changelog, and StatusPage are declared with React.lazy, but both docs route tables render them directly. React 19 still requires a surrounding Suspense boundary for lazy components, so opening those routes will throw instead of showing a fallback.
#!/bin/bash
sed -n '80,86p' Frontend/src/App.jsx
printf '\n--- docs route branches ---\n'
sed -n '338,376p' Frontend/src/App.jsxAlso applies to: 338-355, 359-376
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/App.jsx` around lines 80 - 86, The routes render lazy components
(DocsPortal, ApiReference, Changelog, StatusPage) directly without a
React.Suspense boundary, which will break on load; wrap the route branches that
return these lazy components (both docs route tables and the branches around
where DocsPortal/ApiReference/Changelog/StatusPage are used) in a Suspense with
an appropriate fallback so the lazy imports can load safely (import/use
React.Suspense or the Suspense component already in scope and place the fallback
around each branch or around the group of docs routes).
Comment on lines
+97
to
+138
| // Tenant Core Workspace Layout Shells | ||
| const UserLayout = lazy(() => import('./user/UserLayout')); | ||
| const AdminLayout = lazy(() => import('./admin/layout/AdminLayout')); | ||
| const MasterAdminLayout = lazy(() => import('./master-admin/layout/MasterAdminLayout')); | ||
|
|
||
| // User Telemetry Target Node Matrix | ||
| const Dashboard = lazy(() => import('./user/pages/Dashboard')); | ||
| const CreateTicket = lazy(() => import('./user/pages/CreateTicket')); | ||
| const MyTickets = lazy(() => import('./user/pages/MyTickets')); | ||
| const TicketResult = lazy(() => import('./user/pages/TicketResult')); | ||
| const Profile = lazy(() => import('./user/pages/Profile')); | ||
| const TicketDetail = lazy(() => import('./user/pages/TicketDetail')); | ||
| const AIProcessing = lazy(() => import('./user/pages/AIProcessing')); | ||
| const AIUnderstanding = lazy(() => import('./user/pages/AIUnderstanding')); | ||
| const Notifications = lazy(() => import('./user/pages/Notifications')); | ||
| const Help = lazy(() => import('./user/pages/Help')); | ||
| const DuplicateDetection = lazy(() => import('./user/pages/DuplicateDetection')); | ||
| const AutoResolveChat = lazy(() => import('./user/pages/AutoResolveChat')); | ||
| const Resolved = lazy(() => import('./user/pages/Resolved')); | ||
| const TicketTracking = lazy(() => import('./user/pages/TicketTracking')); | ||
|
|
||
| // Operational Support Admin Nodes | ||
| const AdminDashboard = lazy(() => import('./admin/pages/AdminDashboard')); | ||
| const AdminTickets = lazy(() => import('./admin/pages/AdminTickets')); | ||
| const AdminTicketDetail = lazy(() => import('./admin/pages/AdminTicketDetail')); | ||
| const AdminUsers = lazy(() => import('./admin/pages/AdminUsers')); | ||
| const AdminAnalytics = lazy(() => import('./admin/pages/AdminAnalytics')); | ||
| const AdminProfile = lazy(() => import('./admin/pages/AdminProfile')); | ||
| const AdminSettings = lazy(() => import('./admin/pages/AdminSettings')); | ||
| const AdminScorecard = lazy(() => import('./admin/pages/AdminScorecard')); | ||
| const SLAPage = lazy(() => import('./admin/pages/SLAPage')); | ||
|
|
||
| // Master Root Governance Matrix | ||
| const MasterAdminLogin = lazy(() => import('./pages/MasterAdminLogin')); | ||
| const MasterAdminDashboard = lazy(() => import('./master-admin/pages/MasterAdminDashboard')); | ||
| const PendingAdminRequests = lazy(() => import('./master-admin/pages/PendingAdminRequests')); | ||
| const AllCompanies = lazy(() => import('./master-admin/pages/AllCompanies')); | ||
| const AllAdmins = lazy(() => import('./master-admin/pages/AllAdmins')); | ||
| const MasterBugReports = lazy(() => import('./master-admin/pages/MasterBugReports')); | ||
|
|
||
| // Dynamic Inline Fallbacks | ||
| const NotFoundPage = lazy(() => import('./components/ui/not-found-2').then((module) => ({ default: module.NotFound }))); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import re
from collections import Counter
from pathlib import Path
text = Path("Frontend/src/App.jsx").read_text()
names = re.findall(r'^const\s+([A-Za-z_$][\w$]*)\s*=\s*lazy\b', text, re.M)
for name, count in sorted(Counter(names).items()):
if count > 1:
print(f"{name}: declared {count} times")
PYRepository: ritesh-1918/HELPDESK.AI
Length of output: 1079
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Frontend/src/App.jsx"
echo "== Import lines (first ~120 lines) =="
sed -n '1,140p' "$FILE" | nl -ba | sed -n '1,140p'
echo
echo "== Occurrences: useRealtimeNotifications / ThemeProvider / BackToTopButton / TitleUpdater =="
for sym in useRealtimeNotifications ThemeProvider BackToTopButton TitleUpdater; do
echo "-- $sym --"
rg -n "$sym" "$FILE" || true
done
echo
echo "== Suspense boundaries around lazy route rendering =="
rg -n "Suspense" "$FILE" || true
echo
echo "== React.lazy declarations count (quick sanity) =="
rg -n "React\.lazy|lazy\(\(\)|lazy\s*\(\s*\(\s*=>|\blazy\s*\(" "$FILE" || true
echo
echo "== docs-subdomain / hostname branching =="
rg -n "docs|hostname|subdomain|window\.location|location\.hostname" "$FILE" || true
echo
echo "== Show likely render branches near non-docs / full app =="
# Print a larger window around the lines referenced in the original comment (approx).
sed -n '180,430p' "$FILE" | nl -ba | sed -n '1,260p'Repository: ritesh-1918/HELPDESK.AI
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Frontend/src/App.jsx"
python3 - <<'PY'
from pathlib import Path
p=Path("Frontend/src/App.jsx")
lines=p.read_text().splitlines()
for i,l in enumerate(lines[:140], start=1):
print(f"{i:4d}\t{l}")
PY
echo
echo "== Occurrences: useRealtimeNotifications / ThemeProvider / BackToTopButton / TitleUpdater =="
for sym in useRealtimeNotifications ThemeProvider BackToTopButton TitleUpdater; do
echo "-- $sym --"
rg -n "$sym" "$FILE" || true
done
echo
echo "== Suspense boundaries around lazy route rendering =="
rg -n "Suspense" "$FILE" || true
echo
echo "== React.lazy declarations count (quick sanity) =="
rg -n "React\.lazy|lazy\s*\(" "$FILE" || true
echo
echo "== docs-subdomain / hostname branching =="
rg -n "docs|hostname|subdomain|window\.location|location\.hostname" "$FILE" || true
echo
echo "== Render branches near the lines referenced in the original comment (180-430) =="
python3 - <<'PY'
from pathlib import Path
p=Path("Frontend/src/App.jsx")
lines=p.read_text().splitlines()
start,end=180,430
for i in range(start, min(end,len(lines))+1):
print(f"{i:4d}\t{lines[i-1]}")
PYRepository: ritesh-1918/HELPDESK.AI
Length of output: 29610
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Frontend/src/App.jsx"
echo "== Declarations/imports for ThemeProvider, useRealtimeNotifications, BackToTopButton =="
for sym in ThemeProvider useRealtimeNotifications BackToTopButton; do
echo "-- $sym --"
rg -n "(\bimport\b.*\\b$sym\\b|\bconst\\s+$sym\\b|\bfunction\\s+$sym\\b|\bclass\\s+$sym\\b|<$sym\\b)" "$FILE" || true
done
echo
echo "== Print TitleUpdater function (approx 130-240) =="
python3 - <<'PY'
from pathlib import Path
p=Path("Frontend/src/App.jsx")
lines=p.read_text().splitlines()
for i in range(120, 260):
if i < len(lines):
print(f"{i+1:4d}\t{lines[i]}")
PYRepository: ritesh-1918/HELPDESK.AI
Length of output: 7928
Fix Frontend/src/App.jsx so it can compile and render (remove duplicate lazy() declarations + missing symbols/Suspense).
- Remove the duplicated
const ... = lazy(...)block: many identifiers (e.g.,UserLayout,AdminLayout,Dashboard,MasterAdminLogin,AIProcessing) are declared twice, which will hard-fail withIdentifier ... has already been declared. AdminScorecardis imported from two different paths in the duplicate blocks (./admin/components/AgentScorecardvs./admin/pages/AdminScorecard).useRealtimeNotifications,ThemeProvider, andBackToTopButtonare referenced but not imported/declared in this file (e.g.,useRealtimeNotifications()inAppContent,ThemeProviderin both branches,BackToTopButtonrender).- The “docs portal” branches render
DocsPortal/ApiReference/etc (allReact.lazy) without aSuspenseboundary, and the mainAppContentrouter return is unreachable due to an earlierreturn(so only the docs routes mount). TitleUpdateris only mounted in the docs-subdomain branch, and its pathname predicates reference older patterns (e.g.,/admin/ticket/,/ticket/,/dashboard), so titles won’t match the current nested routes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/App.jsx` around lines 97 - 138, There are duplicate lazy()
declarations and missing symbols causing compile/runtime errors; remove the
duplicate block so each component (e.g., UserLayout, AdminLayout,
MasterAdminLayout, Dashboard, CreateTicket, AIProcessing, MasterAdminLogin,
etc.) is declared exactly once and resolve the AdminScorecard conflict by
keeping the correct path (choose either AdminScorecard from
./admin/pages/AdminScorecard or AgentScorecard from
./admin/components/AgentScorecard) so the identifier is unique. Import or define
the missing symbols useRealtimeNotifications, ThemeProvider, and BackToTopButton
and call/use them where referenced (e.g., inside AppContent and the main App
wrapper) so they exist at runtime. Wrap all React.lazy-loaded route trees
(DocsPortal/ApiReference and the main AppContent routes) in a Suspense boundary
so lazy components can render, and fix the unreachable router by removing the
premature return that causes only docs routes to mount so AppContent is returned
when applicable. Finally move TitleUpdater out of the docs-only branch (mount
globally) and update its pathname predicates to match current nested routes
(e.g., /admin/tickets/:id, /tickets/:id, /user/dashboard or /dashboard) so
titles update correctly.
Comment on lines
+213
to
217
| function AppContent() { | ||
| const { profile } = useAuthStore(); | ||
| const { pathname } = useLocation(); | ||
| const [showShortcuts, setShowShortcuts] = useState(false); | ||
| useRealtimeNotifications(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' Frontend/src/App.jsx
rg -nP '^(import .*\\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\\b|const\\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\\b|function\\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\\b|class\\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\\b)' Frontend/src/App.jsxRepository: ritesh-1918/HELPDESK.AI
Length of output: 210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Frontend/src/App.jsx"
echo "=== Import / top-of-file section ==="
nl -ba "$FILE" | sed -n '1,140p'
echo
echo "=== Around 213-217 ==="
nl -ba "$FILE" | sed -n '190,240p'
echo
echo "=== Around 340-365 ==="
nl -ba "$FILE" | sed -n '320,390p'
echo
echo "=== Declarations/definitions search (within this file) ==="
rg -nP '(^|\s)(import|export)\b.*\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '^(const|let|var)\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '^function\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '^class\s+(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '(useRealtimeNotifications|ThemeProvider|BackToTopButton)\s*=' "$FILE" || trueRepository: ritesh-1918/HELPDESK.AI
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Frontend/src/App.jsx"
echo "=== File head (imports area) ==="
cat -n "$FILE" | sed -n '1,140p'
echo
echo "=== Around 200-240 (includes 213-217) ==="
cat -n "$FILE" | sed -n '190,240p'
echo
echo "=== Around 320-390 (includes 340-365) ==="
cat -n "$FILE" | sed -n '320,390p'
echo
echo "=== Search for imports/definitions in this file ==="
rg -nP '^\s*import\s+.*\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '^\s*(export\s+)?(function|const|let|var|class)\s+\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE" || true
rg -nP '\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\s*=' "$FILE" || true
echo
echo "=== Search for components/usages in this file ==="
rg -nP '\b(useRealtimeNotifications|ThemeProvider|BackToTopButton)\b' "$FILE"Repository: ritesh-1918/HELPDESK.AI
Length of output: 14309
Fix undeclared identifiers in Frontend/src/App.jsx to prevent runtime ReferenceError.
useRealtimeNotifications() is called in AppContent (lines 213-217), but useRealtimeNotifications is not imported or defined in this module.
ThemeProvider (lines 340-355, 360-365) and BackToTopButton (line 365) are also referenced without being imported or defined in this module (only BackToTop/ScrollToTopButton are present in the imports).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/App.jsx` around lines 213 - 217, The runtime ReferenceError is
caused by missing imports/definitions: add/import the hook
useRealtimeNotifications and the components ThemeProvider and BackToTopButton
(or rename usages to the existing ScrollToTopButton/BackToTop and correct names)
in Frontend/src/App.jsx so the identifiers used inside AppContent and the JSX
(useRealtimeNotifications, ThemeProvider, BackToTopButton) are defined; update
the import block to import useRealtimeNotifications from its hook module and
ThemeProvider/BackToTopButton from their component modules (or change the JSX to
use the already-imported ScrollToTopButton/BackToTop symbols) so references
match available exports.
Comment on lines
+359
to
389
| return ( | ||
| <ThemeProvider> | ||
| <BrowserRouter> | ||
| <TitleUpdater /> | ||
| <ScrollToTop /> | ||
| <Toaster /> | ||
| <BugReportWidget /> | ||
| <BackToTopButton /> | ||
| <Routes> | ||
| <Route path="/" element={<DocsPortal />} /> | ||
| <Route path="/docs" element={<Navigate to="/" replace />} /> | ||
| <Route path="/api-reference" element={<ApiReference />} /> | ||
| <Route path="/changelog" element={<Changelog />} /> | ||
| <Route path="/status" element={<StatusPage />} /> | ||
| <Route path="*" element={<DocsPortal />} /> | ||
| <Route path='/' element={<DocsPortal />} /> | ||
| <Route path='/docs' element={<Navigate to='/' replace />} /> | ||
| <Route path='/api-reference' element={<ApiReference />} /> | ||
| <Route path='/changelog' element={<Changelog />} /> | ||
| <Route path='/status' element={<StatusPage />} /> | ||
| <Route path='*' element={<DocsPortal />} /> | ||
| </Routes> | ||
| </BrowserRouter> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <BrowserRouter> | ||
| <TitleUpdater /> | ||
| <ScrollToTop /> | ||
| <Toaster /> | ||
| <BugReportWidget /> | ||
| <Routes> | ||
| {/* Public */} | ||
| <Route path="/" element={<LandingPage />} /> | ||
| <Route path="/login" element={<Login />} /> | ||
| <Route path="/forgot-password" element={<ForgotPassword />} /> | ||
| <Route path="/reset-password" element={<ResetPassword />} /> | ||
| <Route path="/signup" element={<Signup />} /> | ||
| <Route path="/admin-signup" element={<AdminSignup />} /> | ||
| <Route path="/admin-lobby" element={<AdminLobby />} /> | ||
| <Route path="/user-lobby" element={<UserLobby />} /> | ||
| <Route path="/not-approved" element={<NotApproved />} /> | ||
| <Route path="/contact-sales" element={<ContactSales />} /> | ||
| <Route path="/docs" element={<DocsPortal />} /> | ||
| <Route path="/api-reference" element={<ApiReference />} /> | ||
| <Route path="/changelog" element={<Changelog />} /> | ||
| <Route path="/status" element={<StatusPage />} /> | ||
| <Route path="/about" element={<AboutUs />} /> | ||
| <Route path="/careers" element={<Careers />} /> | ||
| <Route path="/cookie-policy" element={<CookiePolicy />} /> | ||
|
|
||
| {/* Feature Pages */} | ||
| <Route path="/features/categorization" element={<AutoCategorizationFeature />} /> | ||
| <Route path="/features/priority" element={<PriorityDetectionFeature />} /> | ||
| <Route path="/features/resolution" element={<SmartResolutionFeature />} /> | ||
|
|
||
| {/* Legal Pages */} | ||
| <Route path="/terms" element={<TermsOfService />} /> | ||
| <Route path="/privacy" element={<PrivacyPolicy />} /> | ||
| <Route path="/security" element={<Security />} /> | ||
|
|
||
| {/* Master Admin Portal */} | ||
| <Route path="/master-admin-login" element={<MasterAdminLogin />} /> | ||
|
|
||
| <Route element={<MasterAdminProtectedRoute />}> | ||
| <Route element={<MasterAdminLayout />}> | ||
| <Route path="/master-admin/dashboard" element={<MasterAdminDashboard />} /> | ||
| <Route path="/master-admin/admin-requests" element={<PendingAdminRequests />} /> | ||
| <Route path="/master-admin/companies" element={<AllCompanies />} /> | ||
| <Route path="/master-admin/all-admins" element={<AllAdmins />} /> | ||
| <Route path="/master-admin/bug-reports" element={<MasterBugReports />} /> | ||
| </Route> | ||
| </Route> | ||
|
|
||
| {/* Protected */} | ||
| <Route element={<ProtectedRoute />}> | ||
| <Route path="/*" element={<AppLayout />} /> | ||
| </Route> | ||
| </Routes> | ||
| <ErrorBoundary> | ||
| <Suspense fallback={<PageSkeleton />}> | ||
| <AppContent /> | ||
| <Toaster /> | ||
| <BugReportWidget /> | ||
| <ScrollToTopButton /> | ||
| <BackToTop /> | ||
| </Suspense> | ||
| </ErrorBoundary> | ||
| </BrowserRouter> |
There was a problem hiding this comment.
The main hostname never mounts the real application router.
The return at Lines 359-376 short-circuits the component before the AppContent tree at Lines 377-389. On the non-docs host, /login, /user/..., /admin/..., and /master-admin/... can never reach their route definitions and instead fall into the docs-only table, with * routing everything to DocsPortal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/App.jsx` around lines 359 - 389, There is an early return that
renders the docs-only router (the JSX starting with
ThemeProvider/BrowserRouter/Routes that mounts DocsPortal and wildcard Route)
which short-circuits the later app tree (the
BrowserRouter/ErrorBoundary/Suspense/AppContent block); remove or conditionalize
that first return so the real application router (AppContent within
ErrorBoundary and Suspense) mounts on the main host—specifically, eliminate the
duplicate top-level return or move the Docs-only Routes behind a host check and
ensure only one BrowserRouter is rendered, keeping AppContent, ErrorBoundary,
Suspense, Toaster, BugReportWidget, ScrollToTopButton/BackToTop in the final
returned tree.
Owner
|
Hi @isnotsafety! 🙌 Thank you so much for your excellent contribution: ""feat: add role-aware keyboard shortcuts (re-applied via patch)""! We really appreciate the high-quality code and effort you have put into the platform. Just a quick, friendly heads-up as we prepare our manual merging and verification queues—please make sure to complete all the mandatory community steps listed below. Once those manual steps are verified, we'll get your PR officially merged into the Let's build something amazing together! 🚀🔥 🌟 Project Support & Developer Network (Show Some Love!)As we prepare our manual verification and merging queues, please take a quick moment to ensure you have completed all four community steps:
Note: Having all four steps completed manually is required before your PR points are officially cleared. |
ritesh-1918
added
gssoc
Owner
|
Hi @isnotsafety! 🙌 Thank you so much for your excellent contribution: ""feat: add role-aware keyboard shortcuts (re-applied via patch)""! We really appreciate the high-quality code and effort you have put into the platform. Just a quick, friendly heads-up as we prepare our manual merging and verification queues—please make sure to complete all the mandatory community steps listed below. Once those manual steps are verified, we'll get your PR officially merged into the Let's build something amazing together! 🚀🔥 🌟 Project Support & Developer Network (Show Some Love!)As we prepare our manual verification and merging queues, please take a quick moment to ensure you have completed all four community steps:
Note: Having all four steps completed manually is required before your PR points are officially cleared. |
Owner
|
Merged locally into gssoc branch. Thank you for your GSSoC contribution! 🚀🔥 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
"This PR re-applies the keyboard shortcuts feature from the original feature branch (f344b9e) after resolving conflicts with the current main branch.\n\nChanges:\n- Added keyboard shortcuts configuration and hooks\n- Updated App.jsx to register global shortcuts\n- Updated AdminLayout.jsx to include the ShortcutsHelp modal\n- Added ShortcutsHelpModal and ShortcutsHelp components\n- Added tests for the keyboard shortcuts functionality\n\nResolves #1172"
Summary by CodeRabbit