feat: generalize command palette entity shortcuts #7792
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two-key command sequences and multi-step command-palette flows for projects, cycles, and issues, with new selectors/lists, per-page data handling, store APIs to activate/clear entities, and assorted exports/hooks to wire sequence-driven navigation in the palette. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CP as Global Key Handler
participant Store as BaseCommandPaletteStore
participant Modal as CommandModal
participant PSel as ProjectSelector
participant CSel as CycleSelector
participant ISel as IssueList/Search
participant Nav as Router
U->>CP: Type two-key sequence (e.g., "o" then "p")
CP->>Store: activateEntity("project")
Store-->>Modal: open palette, activeEntity="project"
Modal->>PSel: Render project selector
U->>PSel: Select project (action: navigate|cycle)
alt action = cycle
PSel->>Modal: selectedProjectId + action=cycle
Modal->>CSel: Render cycles for project
U->>CSel: Select cycle
CSel->>Nav: Navigate to cycle
else action = navigate
PSel->>Modal: selectedProjectId + action=navigate
Modal->>ISel: Render recent/search issues for project
U->>ISel: Select issue
ISel->>Nav: Navigate to issue
end
Modal->>Store: clearActiveEntity()
sequenceDiagram
autonumber
participant U as User
participant Hook as useKeySequence
participant Modal as CommandModal
Note over U,Hook: When modal has no active page
U->>Modal: Press non-mod keys
Modal->>Hook: onKeyDown events
Hook->>Hook: Build 2-char sequence (500ms window)
Hook->>Modal: If matches COMMAND_CONFIG, invoke handler
Modal->>Modal: Open corresponding entity page
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a command registry system to make command palette shortcuts configurable and extensible. It refactors the existing command palette to use a shared registry with enable hooks and dynamic navigation lists.
- Introduces
CommandPaletteEntitytype and command registry system with configurable shortcuts - Refactors command palette to support entity-specific selectors (projects, cycles, issues) with dynamic navigation
- Adds key sequence handling for multi-key shortcuts like "OP" for projects, "OC" for cycles, and "OI" for issues
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
apps/web/core/store/base-command-palette.store.ts |
Adds entity activation state management for command palette |
apps/web/core/components/command-palette/use-key-sequence.ts |
Implements reusable key sequence detection hook |
apps/web/core/components/command-palette/project-selector.tsx |
Creates project selection component using entity list |
apps/web/core/components/command-palette/index.ts |
Exports new command palette components |
apps/web/core/components/command-palette/entity-list.tsx |
Generic entity list component for command palette items |
apps/web/core/components/command-palette/cycle-selector.tsx |
Creates cycle selection component using entity list |
apps/web/core/components/command-palette/commands.ts |
Defines command configuration registry with shortcuts |
apps/web/core/components/command-palette/command-palette.tsx |
Integrates key sequence handling with command activation |
apps/web/core/components/command-palette/command-modal.tsx |
Major refactor to support entity-specific navigation and selectors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .then((res) => | ||
| setRecentIssues(res.map((r: TActivityEntityData) => r.entity_data as TIssueEntityData).slice(0, 10)) | ||
| ) |
There was a problem hiding this comment.
The type assertion as TIssueEntityData could fail if the entity_data doesn't match the expected structure. Consider adding type guards or proper validation before the assertion.
Copilot uses AI. Check for mistakes.
| workspaceService | ||
| .fetchWorkspaceRecents(workspaceSlug.toString(), "issue") | ||
| .then((res) => | ||
| setRecentIssues(res.map((r: TActivityEntityData) => r.entity_data as TIssueEntityData).slice(0, 10)) |
There was a problem hiding this comment.
The magic number 10 should be defined as a named constant (e.g., MAX_RECENT_ISSUES = 10) for better maintainability.
Copilot uses AI. Check for mistakes.
| const cycle = getCycleById(cid); | ||
| const status = cycle?.status ? cycle.status.toLowerCase() : ""; | ||
| if (cycle && ["current", "upcoming"].includes(status)) cycles.push(cycle); |
There was a problem hiding this comment.
The hardcoded cycle statuses ['current', 'upcoming'] should be defined as constants or imported from a constants file for better maintainability.
Copilot uses AI. Check for mistakes.
| if (page === "open-issue") { | ||
| workspaceService | ||
| .searchEntity(workspaceSlug.toString(), { | ||
| count: 10, |
There was a problem hiding this comment.
The magic number 10 should be defined as a named constant (e.g., MAX_SEARCH_RESULTS = 10) for consistency with other search limits in the codebase.
Copilot uses AI. Check for mistakes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/command-palette/command-modal.tsx (1)
286-291: Eliminateas anycast when counting results.Use
Object.valueswith an array guard to avoid explicitany.- const count = Object.keys(results.results).reduce( - (accumulator, key) => (results.results as any)[key].length + accumulator, - 0 - ); + const count = Object.values(results.results).reduce( + (acc, arr) => acc + (Array.isArray(arr) ? arr.length : 0), + 0 + );
🧹 Nitpick comments (12)
apps/web/core/store/base-command-palette.store.ts (2)
88-89: Use ref-tracking for primitive observable.activeEntity is a primitive union; ref-tracking is cheaper and consistent with other booleans above.
- activeEntity: observable, + activeEntity: observable.ref,
160-166: Clear stale activeEntity when palette closes.Prevents reopening with an unintended pre-selected entity.
toggleCommandPaletteModal = (value?: boolean) => { - if (value !== undefined) { - this.isCommandPaletteOpen = value; - } else { - this.isCommandPaletteOpen = !this.isCommandPaletteOpen; - } + if (value !== undefined) this.isCommandPaletteOpen = value; + else this.isCommandPaletteOpen = !this.isCommandPaletteOpen; + if (!this.isCommandPaletteOpen) this.activeEntity = null; };apps/web/core/components/command-palette/entity-list.tsx (3)
7-15: Require getKey to avoid unstable/duplicate keys.Using label as a fallback key risks duplicates; make getKey mandatory for stability.
-interface CommandPaletteEntityListProps<T> { +interface CommandPaletteEntityListProps<T> { heading: string; items: T[]; onSelect: (item: T) => void; - getKey?: (item: T) => string; + getKey: (item: T) => string; getLabel: (item: T) => string; renderItem?: (item: T) => React.ReactNode; emptyText?: string; }
31-37: Key derivation and minor typing nit.Use the now-required getKey and avoid extra getLabel call. Optional: bind item type explicitly in map for clarity.
- {items.map((item) => ( + {items.map((item: T) => ( <Command.Item - key={getKey ? getKey(item) : getLabel(item)} + key={getKey(item)} value={getLabel(item)} onSelect={() => onSelect(item)} className={cn("focus:outline-none")} >
17-25: Alternative: switch to function declaration for better TSX generic inference.Optional but often reduces generic JSX inference edge-cases.
-export const CommandPaletteEntityList = <T,>({ - heading, - items, - onSelect, - getKey, - getLabel, - renderItem, - emptyText = "No results found", -}: CommandPaletteEntityListProps<T>) => { +export function CommandPaletteEntityList<T>({ + heading, + items, + onSelect, + getKey, + getLabel, + renderItem, + emptyText = "No results found", +}: CommandPaletteEntityListProps<T>) { if (items.length === 0) return <div className="px-3 py-8 text-center text-sm text-custom-text-300">{emptyText}</div>; return ( <Command.Group heading={heading}> {items.map((item) => ( <Command.Item key={getKey ? getKey(item) : getLabel(item)} value={getLabel(item)} onSelect={() => onSelect(item)} className={cn("focus:outline-none")} > {renderItem ? renderItem(item) : getLabel(item)} </Command.Item> ))} </Command.Group> ); -}; +}apps/web/core/components/command-palette/commands.ts (1)
30-52: Optional: freeze config and keep literal types.Helps prevent accidental mutation and preserves literal types for
id/sequence.-export const COMMAND_CONFIG: CommandConfig[] = [ +export const COMMAND_CONFIG = [ { id: "open-project", sequence: "op", title: "Open project...", keys: ["O", "P"], entity: "project", }, { id: "open-cycle", sequence: "oc", title: "Open cycle...", keys: ["O", "C"], entity: "cycle", }, { id: "open-issue", sequence: "oi", title: "Open issue...", keys: ["O", "I"], entity: "issue", }, -]; +] as const satisfies readonly CommandConfig[];apps/web/core/components/command-palette/command-palette.tsx (2)
203-216: Harden 2-key handler and clear timer on match; consider reusing the new hook.Prevents repeat key churn and stray timeout; also reduces drift vs use-key-sequence.ts.
- if (!cmdClicked && !altKey && !shiftKey && !isAnyModalOpen) { + if (!cmdClicked && !altKey && !shiftKey && !isAnyModalOpen && !e.repeat) { keySequence.current = (keySequence.current + keyPressed).slice(-2); if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current); sequenceTimeout.current = setTimeout(() => { keySequence.current = ""; }, 500); const cmd = commandSequenceMap[keySequence.current]; if (cmd && (!cmd.enabled || cmd.enabled())) { e.preventDefault(); activateEntity(cmd.entity); keySequence.current = ""; + if (sequenceTimeout.current) { + clearTimeout(sequenceTimeout.current); + sequenceTimeout.current = null; + } return; } }Follow-up: Replace this block with the shared
useKeySequencehook to centralize behavior.
167-174: Map build nit: note future dynamic config.If commands become dynamic, include
COMMAND_CONFIGin deps to rebuild safely. No change required now since it’s static.apps/web/core/components/command-palette/command-modal.tsx (4)
255-304: Guard against stale async updates in debounced search.Rapid term changes can interleave responses and set stale state. Gate updates with an
isActiveflag.useEffect(() => { - if (!workspaceSlug) return; - - setIsLoading(true); + if (!workspaceSlug) return; + let isActive = true; + setIsLoading(true); @@ - .then((res) => { - setIssueResults(res.issue || []); - setResultsCount(res.issue?.length || 0); - }) - .finally(() => { - setIsLoading(false); - setIsSearching(false); - }); + .then((res) => { + if (!isActive) return; + setIssueResults(res.issue || []); + setResultsCount(res.issue?.length || 0); + }) + .finally(() => { + if (!isActive) return; + setIsLoading(false); + setIsSearching(false); + }); @@ - .then((results) => { - setResults(results); - const count = Object.keys(results.results).reduce( - (accumulator, key) => (results.results as any)[key].length + accumulator, - 0 - ); - setResultsCount(count); - }) - .finally(() => { - setIsLoading(false); - setIsSearching(false); - }); + .then((results) => { + if (!isActive) return; + setResults(results); + const count = Object.values(results.results).reduce( + (acc, arr) => acc + (Array.isArray(arr) ? arr.length : 0), + 0 + ); + setResultsCount(count); + }) + .finally(() => { + if (!isActive) return; + setIsLoading(false); + setIsSearching(false); + }); @@ - setResults(WORKSPACE_DEFAULT_SEARCH_RESULT); - setIssueResults([]); - setIsLoading(false); - setIsSearching(false); + setResults(WORKSPACE_DEFAULT_SEARCH_RESULT); + setIssueResults([]); + setIsLoading(false); + setIsSearching(false); } - }, [debouncedSearchTerm, isWorkspaceLevel, projectId, workspaceSlug, page]); + return () => { + isActive = false; + }; + }, [debouncedSearchTerm, isWorkspaceLevel, projectId, workspaceSlug, page]);
81-87: Strongly typepagesto prevent string-typo bugs.Use a union for known pages to get compiler help across the file.
- const [pages, setPages] = useState<string[]>([]); + type Page = + | "open-project" + | "open-cycle" + | "open-issue" + | "settings" + | "change-issue-state" + | "change-issue-priority" + | "change-issue-assignee" + | "change-interface-theme"; + const [pages, setPages] = useState<Page[]>([]);
312-317: Redundant state write on close.
if (searchInIssue) setSearchInIssue(true);is a no-op. Remove to avoid confusing state churn.onClose={() => { closePalette(); - if (searchInIssue) { - setSearchInIssue(true); - } }}
635-718: Consistent linking for issues (recent vs search).Good handling of
project_identifiervsproject__identifierand epics. Consider passingisArchivedwhen source data indicates archived to avoid wrong destinations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/core/components/command-palette/command-modal.tsx(12 hunks)apps/web/core/components/command-palette/command-palette.tsx(6 hunks)apps/web/core/components/command-palette/commands.ts(1 hunks)apps/web/core/components/command-palette/cycle-selector.tsx(1 hunks)apps/web/core/components/command-palette/entity-list.tsx(1 hunks)apps/web/core/components/command-palette/index.ts(1 hunks)apps/web/core/components/command-palette/project-selector.tsx(1 hunks)apps/web/core/components/command-palette/use-key-sequence.ts(1 hunks)apps/web/core/store/base-command-palette.store.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
apps/web/core/components/command-palette/commands.ts (1)
apps/web/core/store/base-command-palette.store.ts (1)
CommandPaletteEntity(11-11)
apps/web/core/components/command-palette/cycle-selector.tsx (1)
apps/web/core/components/command-palette/entity-list.tsx (1)
CommandPaletteEntityList(17-42)
apps/web/core/components/command-palette/project-selector.tsx (1)
apps/web/core/components/command-palette/entity-list.tsx (1)
CommandPaletteEntityList(17-42)
apps/web/core/components/command-palette/command-modal.tsx (8)
apps/space/core/hooks/store/use-cycle.ts (1)
useCycle(7-11)apps/web/core/components/command-palette/commands.ts (1)
COMMAND_CONFIG(30-52)apps/web/core/components/command-palette/use-key-sequence.ts (1)
useKeySequence(5-25)apps/web/core/store/project/project.store.ts (1)
joinedProjectIds(225-236)apps/web/core/components/command-palette/project-selector.tsx (1)
CommandPaletteProjectSelector(12-21)apps/web/core/components/command-palette/cycle-selector.tsx (1)
CommandPaletteCycleSelector(12-21)apps/web/core/components/command-palette/entity-list.tsx (1)
CommandPaletteEntityList(17-42)packages/utils/src/work-item/base.ts (1)
generateWorkItemLink(335-357)
apps/web/core/components/command-palette/command-palette.tsx (1)
apps/web/core/components/command-palette/commands.ts (2)
CommandConfig(3-28)COMMAND_CONFIG(30-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
apps/web/core/components/command-palette/cycle-selector.tsx (1)
12-21: LGTM — simple, typed wrapper looks good.No issues spotted. Mirrors project selector consistently.
apps/web/core/components/command-palette/index.ts (1)
5-9: LGTM — re-exports are cohesive.Surface area matches the new modules.
apps/web/core/components/command-palette/project-selector.tsx (1)
12-21: LGTM — mirrors cycle selector and typed correctly.Consistent props and empty state.
apps/web/core/components/command-palette/command-modal.tsx (2)
171-189: Nice abstraction: registry-driven sequence handlers.Mapping config to handlers with
useMemokeeps the palette extensible and side-effect free at render.
606-624: Confirm navigation target for project route.Using
/${workspaceSlug}/projects/${project.id}/issuesassumes the issues index route exists and is canonical. Verify or adjust to the current project default tab.
| onKeyDown={(e: any) => { | ||
| if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === "k") { | ||
| const key = e.key.toLowerCase(); | ||
| if (!e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey && !page && searchTerm === "") { | ||
| handleKeySequence(e); | ||
| } | ||
| if ((e.metaKey || e.ctrlKey) && key === "k") { |
There was a problem hiding this comment.
Fix lint failure: remove explicit any on keyboard event.
Type the event as React.KeyboardEvent to satisfy no-explicit-any and align with useKeySequence.
Apply:
- onKeyDown={(e: any) => {
+ onKeyDown={(e: React.KeyboardEvent) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onKeyDown={(e: any) => { | |
| if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === "k") { | |
| const key = e.key.toLowerCase(); | |
| if (!e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey && !page && searchTerm === "") { | |
| handleKeySequence(e); | |
| } | |
| if ((e.metaKey || e.ctrlKey) && key === "k") { | |
| onKeyDown={(e: React.KeyboardEvent) => { | |
| const key = e.key.toLowerCase(); | |
| if (!e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey && !page && searchTerm === "") { | |
| handleKeySequence(e); | |
| } | |
| if ((e.metaKey || e.ctrlKey) && key === "k") { |
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/command-modal.tsx around lines 349
to 354, the onKeyDown handler types the event as `any`; replace that with
`React.KeyboardEvent` (or `React.KeyboardEvent<HTMLElement>` if you want a
specific element) to satisfy the no-explicit-any lint rule and match
useKeySequence typing, and ensure React types are available (import React or
import type { KeyboardEvent } from 'react' / use the appropriate local React
import if required).
| const key = e.key.toLowerCase(); | ||
| if (!e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey && !page && searchTerm === "") { | ||
| handleKeySequence(e); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Quick check script to ensure no any remains.
Run to confirm the lint error is resolved in this file.
Also applies to: 286-291
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP ':\s*any\b|as any\b' apps/web/core/components/command-palette/command-modal.tsx || echo "No explicit any found in command-modal.tsx"Length of output: 140
Remove remaining any usages in command-modal.tsx
rg shows two explicit any usages that still cause lint errors — fix them:
- apps/web/core/components/command-palette/command-modal.tsx:288 — replace
(results.results as any)[key]with a properly typed access (e.g., typeresults.resultsasRecord<string, SearchResult[]>or iterateObject.values(results.results)), avoidas any. - apps/web/core/components/command-palette/command-modal.tsx:349 — replace
onKeyDown={(e: any) => ...}with a correctly typed event parameter (e.g.,React.KeyboardEvent<HTMLElement>or the specific element type).
Re-run the quick check: rg -nP ':\s*any\b|as any\b' apps/web/core/components/command-palette/command-modal.tsx.
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/command-modal.tsx around lines 288
and 349, remove the remaining any usages: replace the cast (results.results as
any)[key] by giving results.results a proper type (e.g., declare results.results
as Record<string, SearchResult[]> or iterate with Object.values(results.results)
to avoid indexing with any), and change onKeyDown={(e: any) => ...} to use the
correct React event type (e.g., onKeyDown={(e: React.KeyboardEvent<HTMLElement>)
=> ...} or the specific element type like React.KeyboardEvent<HTMLDivElement>);
after making these changes re-run the quick check rg -nP ':\s*any\b|as any\b'
apps/web/core/components/command-palette/command-modal.tsx to ensure no any
remains.
| import type { CommandPaletteEntity } from "@/store/base-command-palette.store"; | ||
| import { useIssueDetail } from "@/hooks/store/use-issue-detail"; |
There was a problem hiding this comment.
Remove unused type import.
Avoids lint noise.
-import type { CommandPaletteEntity } from "@/store/base-command-palette.store";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { CommandPaletteEntity } from "@/store/base-command-palette.store"; | |
| import { useIssueDetail } from "@/hooks/store/use-issue-detail"; | |
| import { useIssueDetail } from "@/hooks/store/use-issue-detail"; |
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/command-palette.tsx around lines 19
to 20, the imported type CommandPaletteEntity is not used and triggers lint
noise; remove the unused type-only import (or delete it from the import list) so
the file no longer imports CommandPaletteEntity, then run lint/TS check to
confirm no remaining references.
| const keySequence = useRef(""); | ||
| const sequenceTimeout = useRef<NodeJS.Timeout | null>(null); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use browser-safe timer type.
Aligns with DOM setTimeout return type.
- const sequenceTimeout = useRef<NodeJS.Timeout | null>(null);
+ const sequenceTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const keySequence = useRef(""); | |
| const sequenceTimeout = useRef<NodeJS.Timeout | null>(null); | |
| const keySequence = useRef(""); | |
| const sequenceTimeout = useRef<ReturnType<typeof setTimeout> | null>(null); |
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/command-palette.tsx around lines 164
to 166, the ref sequenceTimeout is typed as NodeJS.Timeout which is a
Node-specific type; change it to the browser-safe timer type (e.g. number | null
or ReturnType<typeof setTimeout>) so it matches DOM setTimeout return value.
Update the ref declaration to use that type and ensure any clearTimeout calls
remain compatible (no runtime changes needed), and remove any Node-specific type
imports if present.
| renderItem, | ||
| emptyText = "No results found", | ||
| }: CommandPaletteEntityListProps<T>) => { | ||
| if (items.length === 0) return <div className="px-3 py-8 text-center text-sm text-custom-text-300">{emptyText}</div>; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Locate the lint “Unexpected any” offender.
If ESLint is flagging this area, it’s likely from generic inference at JSX call sites. Let’s confirm the exact file/line.
🏁 Script executed:
#!/bin/bash
# Find explicit 'any' in the affected surfaces
rg -nP '\b(any|as any|<any>)\b' apps/web/core/components/command-palette apps/web/core/store | sed 's/^/ANY: /'
# Also flag NodeJS.Timeout usages in web code
rg -nP 'NodeJS\.Timeout' apps/web/core/components/command-palette apps/web/core/store | sed 's/^/TIMEOUT: /'Length of output: 5539
'Unexpected any' isn't coming from entity-list.tsx — explicit any found elsewhere
No any in apps/web/core/components/command-palette/entity-list.tsx; search found explicit any and NodeJS.Timeout usages you must fix:
- apps/web/core/components/command-palette/command-modal.tsx:288 —
(results.results as any)[key]... - apps/web/core/components/command-palette/command-modal.tsx:349 —
onKeyDown={(e: any) => ...} - apps/web/core/components/command-palette/actions/search-results.tsx:32 —
(results.results as any)[key] - apps/web/core/components/command-palette/actions/search-results.tsx:38 —
section.map((item: any) => ...) - apps/web/core/components/command-palette/use-key-sequence.ts:7 —
useRef<NodeJS.Timeout | null> - apps/web/core/components/command-palette/command-palette.tsx:165 —
useRef<NodeJS.Timeout | null>
Fixes: define proper types for search results/items (avoid (results as any)), type event handlers (e.g., React.KeyboardEvent), and replace NodeJS.Timeout with number or ReturnType<typeof setTimeout> for browser code.
| export const useKeySequence = (handlers: Record<string, () => void>, timeout = 500) => { | ||
| const sequence = useRef(""); | ||
| const sequenceTimeout = useRef<NodeJS.Timeout | null>(null); | ||
|
|
||
| return (e: React.KeyboardEvent) => { | ||
| const key = e.key.toLowerCase(); | ||
| sequence.current = (sequence.current + key).slice(-2); | ||
|
|
||
| if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current); | ||
| sequenceTimeout.current = setTimeout(() => { | ||
| sequence.current = ""; | ||
| }, timeout); | ||
|
|
||
| const action = handlers[sequence.current]; | ||
| if (action) { | ||
| e.preventDefault(); | ||
| action(); | ||
| sequence.current = ""; | ||
| } | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix browser timer type, ignore modifier/non-char keys, and clear timer on hit.
Prevents type noise in web envs, avoids accidental matches (e.g., Arrow keys), and stops a dangling timeout after a match.
-import { useRef } from "react";
+import { useRef } from "react";
+import type React from "react";
export const useKeySequence = (handlers: Record<string, () => void>, timeout = 500) => {
const sequence = useRef("");
- const sequenceTimeout = useRef<NodeJS.Timeout | null>(null);
+ const sequenceTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);
- return (e: React.KeyboardEvent) => {
- const key = e.key.toLowerCase();
+ return (e: React.KeyboardEvent) => {
+ if (e.metaKey || e.ctrlKey || e.altKey || e.shiftKey || e.repeat) return;
+ const keyRaw = e.key;
+ if (!keyRaw || keyRaw.length !== 1) return; // ignore non-character keys
+ const key = keyRaw.toLowerCase();
sequence.current = (sequence.current + key).slice(-2);
- if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current);
+ if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current);
sequenceTimeout.current = setTimeout(() => {
sequence.current = "";
}, timeout);
const action = handlers[sequence.current];
if (action) {
e.preventDefault();
action();
sequence.current = "";
+ if (sequenceTimeout.current) {
+ clearTimeout(sequenceTimeout.current);
+ sequenceTimeout.current = null;
+ }
}
};
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useKeySequence = (handlers: Record<string, () => void>, timeout = 500) => { | |
| const sequence = useRef(""); | |
| const sequenceTimeout = useRef<NodeJS.Timeout | null>(null); | |
| return (e: React.KeyboardEvent) => { | |
| const key = e.key.toLowerCase(); | |
| sequence.current = (sequence.current + key).slice(-2); | |
| if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current); | |
| sequenceTimeout.current = setTimeout(() => { | |
| sequence.current = ""; | |
| }, timeout); | |
| const action = handlers[sequence.current]; | |
| if (action) { | |
| e.preventDefault(); | |
| action(); | |
| sequence.current = ""; | |
| } | |
| }; | |
| }; | |
| import { useRef } from "react"; | |
| import type React from "react"; | |
| export const useKeySequence = (handlers: Record<string, () => void>, timeout = 500) => { | |
| const sequence = useRef(""); | |
| const sequenceTimeout = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| return (e: React.KeyboardEvent) => { | |
| if (e.metaKey || e.ctrlKey || e.altKey || e.shiftKey || e.repeat) return; | |
| const keyRaw = e.key; | |
| if (!keyRaw || keyRaw.length !== 1) return; // ignore non-character keys | |
| const key = keyRaw.toLowerCase(); | |
| sequence.current = (sequence.current + key).slice(-2); | |
| if (sequenceTimeout.current) clearTimeout(sequenceTimeout.current); | |
| sequenceTimeout.current = setTimeout(() => { | |
| sequence.current = ""; | |
| }, timeout); | |
| const action = handlers[sequence.current]; | |
| if (action) { | |
| e.preventDefault(); | |
| action(); | |
| sequence.current = ""; | |
| if (sequenceTimeout.current) { | |
| clearTimeout(sequenceTimeout.current); | |
| sequenceTimeout.current = null; | |
| } | |
| } | |
| }; | |
| }; |
🤖 Prompt for AI Agents
In apps/web/core/components/command-palette/use-key-sequence.ts around lines 5
to 25, the hook currently uses NodeJS.Timeout (server type), listens to all keys
(including modifiers and non-character keys like Arrow/Shift), and leaves a
pending timeout after a match; change the timeout ref type to number | null for
browser setTimeout, ignore keys that are modifiers or non-printable (e.g., check
e.key length > 1 or match against a whitelist/regex of single-char keys and
return early for others), convert the key to lower-case only for printable
chars, and when a handler matches clear the active timeout (clearTimeout and set
ref to null) before invoking the action and resetting sequence.current to "" to
avoid dangling timers and accidental matches.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/command-palette/command-modal.tsx (1)
31-41: Fix duplicate imports from '@/components/command-palette' (lint blocker).Combine into a single import to satisfy no-duplicate-imports and unblock CI.
Apply:
import { ChangeIssueAssignee, ChangeIssuePriority, ChangeIssueState, CommandPaletteHelpActions, CommandPaletteIssueActions, CommandPaletteProjectActions, CommandPaletteSearchResults, CommandPaletteThemeActions, CommandPaletteWorkspaceSettingsActions, -} from "@/components/command-palette"; + CommandPaletteProjectSelector, + CommandPaletteCycleSelector, + CommandPaletteEntityList, + useKeySequence, + COMMAND_CONFIG, +} from "@/components/command-palette"; @@ -import { - CommandPaletteProjectSelector, - CommandPaletteCycleSelector, - CommandPaletteEntityList, - useKeySequence, -} from "@/components/command-palette"; - -import { COMMAND_CONFIG } from "@/components/command-palette";Also applies to: 43-49
♻️ Duplicate comments (4)
apps/web/core/components/command-palette/command-modal.tsx (4)
289-295: Eliminateas anywhen counting results (lint blocker).Use
Object.valueswith an array guard.Apply:
- const count = Object.keys(results.results).reduce( - (accumulator, key) => (results.results as any)[key].length + accumulator, - 0 - ); + const count = Object.values(results.results).reduce<number>( + (acc, value) => acc + (Array.isArray(value) ? value.length : 0), + 0 + );
219-221: Avoid hardcoded cycle statuses. Extract a constant.Apply:
- if (cycle && ["current", "upcoming"].includes(status)) cycles.push(cycle); + const VISIBLE_CYCLE_STATUSES = ["current", "upcoming"] as const; + if (cycle && VISIBLE_CYCLE_STATUSES.includes(status as (typeof VISIBLE_CYCLE_STATUSES)[number])) { + cycles.push(cycle); + }
68-70: Replace magic numbers with named constants.Apply:
const workspaceService = new WorkspaceService(); +const MAX_RECENT_ISSUES = 10; +const MAX_SEARCH_RESULTS = 10;- setRecentIssues(res.map((r: TActivityEntityData) => r.entity_data as TIssueEntityData).slice(0, 10)) + setRecentIssues(res.map((r: TActivityEntityData) => r.entity_data as TIssueEntityData).slice(0, MAX_RECENT_ISSUES))- count: 10, + count: MAX_SEARCH_RESULTS,Also applies to: 167-169, 268-269
352-357: Type the keyboard event to remove explicitany(lint blocker).Apply:
- onKeyDown={(e: any) => { + onKeyDown={(e: React.KeyboardEvent<HTMLElement>) => { const key = e.key.toLowerCase(); if (!e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey && !page && searchTerm === "") { handleKeySequence(e); }
🧹 Nitpick comments (2)
apps/web/core/components/command-palette/cycle-selector.tsx (1)
16-18: Guard label against nullable names.If
cycle.namecan be empty/undefined, Command.Itemvalueshould still be a string.Apply:
- getLabel={(cycle) => cycle.name} + getLabel={(cycle) => cycle.name ?? "Untitled cycle"}apps/web/core/components/command-palette/command-modal.tsx (1)
3-3: Remove unuseduseRefimport (lint).Apply:
-import React, { useEffect, useState, useRef, useMemo, useCallback } from "react"; +import React, { useEffect, useState, useMemo, useCallback } from "react";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/command-palette/command-modal.tsx(13 hunks)apps/web/core/components/command-palette/cycle-selector.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/core/components/command-palette/command-modal.tsx (9)
apps/space/core/hooks/store/use-cycle.ts (1)
useCycle(7-11)apps/web/core/store/base-command-palette.store.ts (1)
CommandPaletteEntity(11-11)apps/web/core/components/command-palette/commands.ts (1)
COMMAND_CONFIG(30-52)apps/web/core/components/command-palette/use-key-sequence.ts (1)
useKeySequence(5-25)apps/web/core/store/project/project.store.ts (1)
joinedProjectIds(225-236)apps/web/core/components/command-palette/project-selector.tsx (1)
CommandPaletteProjectSelector(12-21)apps/web/core/components/command-palette/cycle-selector.tsx (1)
CommandPaletteCycleSelector(12-21)apps/web/core/components/command-palette/entity-list.tsx (1)
CommandPaletteEntityList(17-42)packages/utils/src/work-item/base.ts (1)
generateWorkItemLink(335-357)
apps/web/core/components/command-palette/cycle-selector.tsx (1)
apps/web/core/components/command-palette/entity-list.tsx (1)
CommandPaletteEntityList(17-42)
🪛 GitHub Check: Build and lint web apps
apps/web/core/components/command-palette/command-modal.tsx
[failure] 49-49:
'@/components/command-palette' import is duplicated
[failure] 43-43:
'@/components/command-palette' import is duplicated
🪛 GitHub Actions: Build and lint web apps
apps/web/core/components/command-palette/command-modal.tsx
[error] 43-43: ESLint: no-duplicate-imports. Duplicate import from '@/components/command-palette' in command-modal.tsx. Failing step: eslint . --max-warnings 821 during web:check:lint.
[warning] 3-3: React: 'useRef' is defined but never used.
🔇 Additional comments (1)
apps/web/core/components/command-palette/cycle-selector.tsx (1)
12-21: LGTM — simple, typed wrapper over EntityList looks correct.
Summary
Testing
pnpm check(fails: web#check:lint: Unexpected any. Specify a different type)https://chatgpt.com/codex/tasks/task_e_68c0a5dc9080832a93d2b2f97dbe79f3
Summary by CodeRabbit