🎨 Palette: Add global search keyboard shortcut (Cmd+K/Ctrl+K) - #357
🎨 Palette: Add global search keyboard shortcut (Cmd+K/Ctrl+K)#357mkk2026 wants to merge 1 commit into
Conversation
- Implemented `Cmd+K` / `Ctrl+K` keyboard shortcut to focus the global search input. - Added visual `<kbd>` hint inside the search bar, with dynamic OS detection to avoid SSR hydration mismatches. - Updated `aria-keyshortcuts` for screen reader accessibility. - Enhanced `Input` component to properly accept `ref` via props in React 19. Co-authored-by: corebrimtech <175357468+corebrimtech@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe PR adds a keyboard shortcut (Ctrl/Cmd+k) to focus the search input. The ChangesKeyboard Shortcut to Search Input Focus
Sequence DiagramsequenceDiagram
participant User
participant Document
participant Handler
participant SearchInput
User->>Document: Press Ctrl/Cmd + k
Document->>Handler: keydown event fired
Handler->>SearchInput: focus()
SearchInput->>SearchInput: receives focus
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/app/page.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/components/ui/input.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/page.tsx (2)
377-392: ⚡ Quick winSimplify positioning structure for kbd hint and clear button.
The wrapper
<div>at line 377 and the<button>at line 384 both use identical absolute positioning (absolute right-4 top-1/2 transform -translate-y-1/2). This creates redundant and confusing structure:
- The kbd element (lines 379-381) is positioned relative to the wrapper div.
- The button (lines 384-390) has its own absolute positioning, making the wrapper positioning ineffective for it.
Since both elements need the same positioning, they should share the wrapper's positioning rather than duplicating it.
♻️ Proposed simplification
<div className="absolute right-4 top-1/2 transform -translate-y-1/2 flex items-center gap-2"> {!searchQuery && ( <kbd className="hidden sm:inline-flex h-6 items-center gap-1 rounded border border-slate-200 dark:border-slate-700 bg-slate-100 dark:bg-slate-800 px-1.5 font-mono text-[10px] font-medium text-slate-500 dark:text-slate-400 opacity-100 select-none pointer-events-none"> <span className="text-xs">{modifierKey === 'Cmd' ? '⌘' : 'Ctrl'}</span>K </kbd> )} {searchQuery && ( - <button - onClick={() => setSearchQuery('')} - className="absolute right-4 top-1/2 transform -translate-y-1/2 text-slate-400 hover:text-slate-600 dark:hover:text-slate-300 transition-colors focus:outline-none focus:ring-2 focus:ring-primary/20 rounded-full p-1" - aria-label="Clear search" - > - <X className="h-5 w-5" /> - </button> + <button + onClick={() => setSearchQuery('')} + className="text-slate-400 hover:text-slate-600 dark:hover:text-slate-300 transition-colors focus:outline-none focus:ring-2 focus:ring-primary/20 rounded-full p-1" + aria-label="Clear search" + > + <X className="h-5 w-5" /> + </button> )} </div>🤖 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 `@src/app/page.tsx` around lines 377 - 392, The wrapper div and the clear button duplicate the same absolute positioning; remove the absolute positioning classes from the button so it sits inside the already-positioned wrapper. Concretely, keep the wrapper with "absolute right-4 top-1/2 transform -translate-y-1/2" and simplify the button inside (used when searchQuery is truthy) by removing "absolute right-4 top-1/2 transform -translate-y-1/2" but preserving classes for colors, focus, rounded/full, padding, aria-label and the onClick handler (setSearchQuery('')) and the X icon usage; this ensures both the kbd hint and the clear button share the wrapper positioning and avoids redundancy.
61-63: 💤 Low valueConsider replacing
navigator.platformwithnavigator.userAgentData?.platformfor platform detection.
navigator.platformis not recommended/reliable due to user-agent reduction; there isn’t a single universal “deprecated replacement,” but User-Agent Client Hints is the modern API for platform/OS info (with fallback when unavailable).♻️ Proposed refactor using userAgentData
- if (navigator.platform.toLowerCase().includes('mac')) { + const isMac = navigator.userAgentData?.platform?.toLowerCase().includes('mac') + ?? navigator.platform?.toLowerCase().includes('mac') + if (isMac) { setModifierKey('Cmd') }🤖 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 `@src/app/page.tsx` around lines 61 - 63, The code currently uses navigator.platform in the platform check inside the component (the block that calls setModifierKey('Cmd')), which is unreliable; update the detection to prefer navigator.userAgentData?.platform (falling back to navigator.platform or a userAgent string) and use that value to decide when to call setModifierKey('Cmd'); ensure the logic remains the same (case-insensitive includes('mac')) and wraps access to navigator.userAgentData in a safe optional chain to avoid runtime errors in browsers that don't support Client Hints.
🤖 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 @.Jules/palette.md:
- Around line 27-29: Update the wording in the "Learning" section to clarify
that the aria-keyshortcuts attribute should remain static (e.g.,
aria-keyshortcuts="Control+K Meta+K") and is not derived from
navigator.platform, while only the visual <kbd> hint is set dynamically inside a
useEffect that reads navigator.platform; reference the static aria attribute and
the useEffect/navigator.platform pattern so future readers don’t convert
aria-keyshortcuts into a platform-dependent value and trigger SSR hydration
mismatches.
In `@src/app/page.tsx`:
- Around line 66-71: The handleKeyDown handler currently forces focus to
searchInputRef even if the user is typing elsewhere; update handleKeyDown to
first inspect document.activeElement and bail out if the active element is an
input, textarea, select, or has isContentEditable true (or other interactive
elements) so the Ctrl/Cmd+K shortcut does not steal focus from other controls;
keep the existing check for (e.ctrlKey || e.metaKey) && e.key === 'k' and only
call searchInputRef.current?.focus() when the active element is not a
user-editable or interactive element.
In `@src/components/ui/input.tsx`:
- Around line 5-9: The Input component currently types props as
React.ComponentProps<"input"> & { ref?: React.Ref<HTMLInputElement> }, which is
redundant in React 19; replace that intersection with
React.ComponentPropsWithRef<"input"> so the ref is typed using React's official
utility. Update the Input signature (function Input(...)) to accept props:
React.ComponentPropsWithRef<"input"> and remove the manual ref prop type,
keeping prop spread and usages intact to rely on the built-in ref typing.
---
Nitpick comments:
In `@src/app/page.tsx`:
- Around line 377-392: The wrapper div and the clear button duplicate the same
absolute positioning; remove the absolute positioning classes from the button so
it sits inside the already-positioned wrapper. Concretely, keep the wrapper with
"absolute right-4 top-1/2 transform -translate-y-1/2" and simplify the button
inside (used when searchQuery is truthy) by removing "absolute right-4 top-1/2
transform -translate-y-1/2" but preserving classes for colors, focus,
rounded/full, padding, aria-label and the onClick handler (setSearchQuery(''))
and the X icon usage; this ensures both the kbd hint and the clear button share
the wrapper positioning and avoids redundancy.
- Around line 61-63: The code currently uses navigator.platform in the platform
check inside the component (the block that calls setModifierKey('Cmd')), which
is unreliable; update the detection to prefer navigator.userAgentData?.platform
(falling back to navigator.platform or a userAgent string) and use that value to
decide when to call setModifierKey('Cmd'); ensure the logic remains the same
(case-insensitive includes('mac')) and wraps access to navigator.userAgentData
in a safe optional chain to avoid runtime errors in browsers that don't support
Client Hints.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ce6a638-b9ca-4e0e-bcfd-ceda59d8a5a7
📒 Files selected for processing (3)
.Jules/palette.mdsrc/app/page.tsxsrc/components/ui/input.tsx
| ## 2026-10-27 - Keyboard Shortcuts & SSR Hydration | ||
| **Learning:** To avoid Next.js Server-Side Rendering (SSR) hydration mismatches when displaying OS-specific UI elements or accessibility attributes (e.g., keyboard shortcut hints and `aria-keyshortcuts` derived from `navigator.platform`), the state must be initialized with a default value and dynamically updated inside a `useEffect` hook. Search inputs should ideally incorporate a global keyboard shortcut (e.g., `Ctrl+K` / `Cmd+K`) with a visual `<kbd>` hint inside the search bar to improve discoverability and usability for power users. | ||
| **Action:** Add `<kbd>` hints for discoverability, use `useEffect` for `navigator.platform` checks, and attach `aria-keyshortcuts`. |
There was a problem hiding this comment.
Clarify that aria-keyshortcuts is static, not platform-derived.
The Learning states that aria-keyshortcuts is "derived from navigator.platform," but the actual implementation uses a static aria-keyshortcuts="Control+K Meta+K" attribute. Only the visual <kbd> hint is dynamically set based on navigator.platform. The aria attribute correctly lists both modifiers to cover all platforms.
This phrasing could mislead future developers into making aria-keyshortcuts dynamic, which would introduce the very SSR hydration mismatch the pattern is designed to avoid.
📝 Suggested clarification
-**Learning:** To avoid Next.js Server-Side Rendering (SSR) hydration mismatches when displaying OS-specific UI elements or accessibility attributes (e.g., keyboard shortcut hints and `aria-keyshortcuts` derived from `navigator.platform`), the state must be initialized with a default value and dynamically updated inside a `useEffect` hook. Search inputs should ideally incorporate a global keyboard shortcut (e.g., `Ctrl+K` / `Cmd+K`) with a visual `<kbd>` hint inside the search bar to improve discoverability and usability for power users.
+**Learning:** To avoid Next.js Server-Side Rendering (SSR) hydration mismatches when displaying OS-specific UI elements (e.g., keyboard shortcut hints derived from `navigator.platform`), the state must be initialized with a default value and dynamically updated inside a `useEffect` hook. Search inputs should ideally incorporate a global keyboard shortcut (e.g., `Ctrl+K` / `Cmd+K`) with a visual `<kbd>` hint inside the search bar and a static `aria-keyshortcuts` attribute listing all supported modifiers to improve discoverability and usability for power users.📝 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.
| ## 2026-10-27 - Keyboard Shortcuts & SSR Hydration | |
| **Learning:** To avoid Next.js Server-Side Rendering (SSR) hydration mismatches when displaying OS-specific UI elements or accessibility attributes (e.g., keyboard shortcut hints and `aria-keyshortcuts` derived from `navigator.platform`), the state must be initialized with a default value and dynamically updated inside a `useEffect` hook. Search inputs should ideally incorporate a global keyboard shortcut (e.g., `Ctrl+K` / `Cmd+K`) with a visual `<kbd>` hint inside the search bar to improve discoverability and usability for power users. | |
| **Action:** Add `<kbd>` hints for discoverability, use `useEffect` for `navigator.platform` checks, and attach `aria-keyshortcuts`. | |
| ## 2026-10-27 - Keyboard Shortcuts & SSR Hydration | |
| **Learning:** To avoid Next.js Server-Side Rendering (SSR) hydration mismatches when displaying OS-specific UI elements (e.g., keyboard shortcut hints derived from `navigator.platform`), the state must be initialized with a default value and dynamically updated inside a `useEffect` hook. Search inputs should ideally incorporate a global keyboard shortcut (e.g., `Ctrl+K` / `Cmd+K`) with a visual `<kbd>` hint inside the search bar and a static `aria-keyshortcuts` attribute listing all supported modifiers to improve discoverability and usability for power users. | |
| **Action:** Add `<kbd>` hints for discoverability, use `useEffect` for `navigator.platform` checks, and attach `aria-keyshortcuts`. |
🤖 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 @.Jules/palette.md around lines 27 - 29, Update the wording in the "Learning"
section to clarify that the aria-keyshortcuts attribute should remain static
(e.g., aria-keyshortcuts="Control+K Meta+K") and is not derived from
navigator.platform, while only the visual <kbd> hint is set dynamically inside a
useEffect that reads navigator.platform; reference the static aria attribute and
the useEffect/navigator.platform pattern so future readers don’t convert
aria-keyshortcuts into a platform-dependent value and trigger SSR hydration
mismatches.
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.ctrlKey || e.metaKey) && e.key === 'k') { | ||
| e.preventDefault() | ||
| searchInputRef.current?.focus() | ||
| } | ||
| } |
There was a problem hiding this comment.
Prevent focus stealing from other input fields.
The keyboard shortcut handler will steal focus even when the user is actively typing in another input or textarea elsewhere on the page. This disrupts workflow when users are filling out forms or interacting with other controls.
🛡️ Proposed fix to check active element
const handleKeyDown = (e: KeyboardEvent) => {
if ((e.ctrlKey || e.metaKey) && e.key === 'k') {
e.preventDefault()
+ // Don't steal focus if user is typing in another input
+ const activeEl = document.activeElement as HTMLElement
+ if (activeEl?.tagName === 'INPUT' || activeEl?.tagName === 'TEXTAREA') {
+ return
+ }
searchInputRef.current?.focus()
}
}📝 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 handleKeyDown = (e: KeyboardEvent) => { | |
| if ((e.ctrlKey || e.metaKey) && e.key === 'k') { | |
| e.preventDefault() | |
| searchInputRef.current?.focus() | |
| } | |
| } | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| if ((e.ctrlKey || e.metaKey) && e.key === 'k') { | |
| e.preventDefault() | |
| // Don't steal focus if user is typing in another input | |
| const activeEl = document.activeElement as HTMLElement | |
| if (activeEl?.tagName === 'INPUT' || activeEl?.tagName === 'TEXTAREA') { | |
| return | |
| } | |
| searchInputRef.current?.focus() | |
| } | |
| } |
🤖 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 `@src/app/page.tsx` around lines 66 - 71, The handleKeyDown handler currently
forces focus to searchInputRef even if the user is typing elsewhere; update
handleKeyDown to first inspect document.activeElement and bail out if the active
element is an input, textarea, select, or has isContentEditable true (or other
interactive elements) so the Ctrl/Cmd+K shortcut does not steal focus from other
controls; keep the existing check for (e.ctrlKey || e.metaKey) && e.key === 'k'
and only call searchInputRef.current?.focus() when the active element is not a
user-editable or interactive element.
| function Input({ className, type, ref, ...props }: React.ComponentProps<"input"> & { ref?: React.Ref<HTMLInputElement> }) { | ||
| return ( | ||
| <input | ||
| type={type} | ||
| ref={ref} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React 19 ref as prop ComponentProps type definition
💡 Result:
In React 19, ref is now treated as a regular prop [1][2]. This change removes the need for forwardRef in new components, allowing function components to receive ref directly as a parameter [1][3][4]. To define the type for a ref prop in TypeScript, you should use the React.Ref<T> type [5][2]. ### Typing ref as a Prop If you are defining a component that accepts a ref as a prop, you should include it explicitly in your props interface [6][5][2]. It is recommended to mark it as optional (ref?: ...) to account for cases where the parent does not pass a ref [5][2]. tsx import type { Ref } from 'react'; interface MyComponentProps { label: string; ref?: Ref<HTMLInputElement>; } function MyComponent({ label, ref }: MyComponentProps) { return ( <label> {label} <input ref={ref} /> </label> ); } ### Using ComponentProps When creating a component that wraps a native element and accepts all standard props including ref, you can use ComponentPropsWithRef [6][7]. This utility automatically includes the ref prop in the resulting type definition [7]. tsx import { ComponentPropsWithRef } from "react"; function MyInput(props: ComponentPropsWithRef<"input">) { return <input {...props} />; } // Usage function Parent() { const inputRef = useRef<HTMLInputElement>(null); return <MyInput ref={inputRef} />; } ### Key Considerations * Deprecation of forwardRef: While forwardRef remains supported for backward compatibility, it is no longer necessary for new React 19 components and will be deprecated in future versions [1][3][2]. * ComponentProps: You can generally use ComponentProps<"element"> or ComponentPropsWithRef<"element"> to handle native element props seamlessly [6][5]. * Ref Cleanup: React 19 introduces support for cleanup functions in ref callbacks (returning a function to reset the ref) [1][8][4]. TypeScript will now reject any return values from ref callbacks other than null, undefined, or a cleanup function [4].
Citations:
- 1: https://react.dev/blog/2024/12/05/react-19
- 2: https://www.stanza.dev/courses/react-modern-patterns/syntax-changes/react-ref-as-prop
- 3: https://react.dev/reference/react/forwardRef
- 4: https://blog.saeloun.com/2025/03/24/react-19-ref-as-prop/
- 5: https://stackoverflow.com/questions/79300987/react-19-ref-as-prop-and-typescript
- 6: https://react-typescript-cheatsheet.netlify.app/docs/basic/getting-started/forward_and_create_ref
- 7: https://www.totaltypescript.com/react-component-props-type-helper
- 8: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
Use React.ComponentPropsWithRef for Input’s ref typing in React 19.
React 19 treats ref as a regular prop for function components, and the React.ComponentPropsWithRef<"input"> utility already includes the correct ref type—so the extra & { ref?: React.Ref<HTMLInputElement> } intersection is redundant and risks drifting from React’s official types.
🤖 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 `@src/components/ui/input.tsx` around lines 5 - 9, The Input component
currently types props as React.ComponentProps<"input"> & { ref?:
React.Ref<HTMLInputElement> }, which is redundant in React 19; replace that
intersection with React.ComponentPropsWithRef<"input"> so the ref is typed using
React's official utility. Update the Input signature (function Input(...)) to
accept props: React.ComponentPropsWithRef<"input"> and remove the manual ref
prop type, keeping prop spread and usages intact to rely on the built-in ref
typing.
1 participant
💡 What
Added a global keyboard shortcut (
Cmd+Kon Mac,Ctrl+Kon Windows/Linux) to instantly focus the main search bar from anywhere on the dashboard. Also added a visual<kbd>hint inside the input.🎯 Why
In a data-heavy dashboard (articles, CVEs), search is a primary action. Power users and keyboard-centric users often have to manually move their mouse to the search bar. This provides a standard, intuitive shortcut to improve workflow speed and discoverability.
📸 Before/After
(See attached screenshots in verification)
The search bar now displays a subtle
Ctrl KorCmd Kbadge on the right side when empty, cleanly fading out when text is entered to make room for the clear button.♿ Accessibility
aria-keyshortcuts="Control+K Meta+K"to the input so screen readers can announce the available shortcut.PR created automatically by Jules for task 2913467706412035892 started by @corebrimtech
Summary by CodeRabbit