🎨 Palette: Add keyboard shortcut for search input - #183
Conversation
Co-authored-by: corebrimtech <175357468+corebrimtech@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe changes implement OS-specific keyboard shortcut hints for a search input component. A global Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/page.tsx (1)
61-66: Consider a more robust OS detection approach.
navigator.platformis deprecated. While it still works in most browsers, consider using a fallback pattern that checksnavigator.userAgentData?.platformfirst (modern Chromium browsers) and falls back tonavigator.platform:♻️ Suggested improvement
useEffect(() => { setIsMounted(true) if (typeof window !== 'undefined') { - setIsMac(navigator.platform.toUpperCase().indexOf('MAC') >= 0) + const platform = navigator.userAgentData?.platform ?? navigator.platform + setIsMac(platform.toUpperCase().includes('MAC')) } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/page.tsx` around lines 61 - 66, The OS detection using deprecated navigator.platform should be made more robust: in the useEffect callback that calls setIsMounted and setIsMac, derive a platform string by preferring navigator.userAgentData?.platform (if available), falling back to navigator.platform and then navigator.userAgent (to cover older browsers), normalize to upper/lower case, and setIsMac based on whether that platform string contains "MAC" (or "MACINTOSH") to be case-insensitive and resilient to missing fields; update the useEffect that currently references setIsMounted and setIsMac to use this fallback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/page.tsx`:
- Around line 68-78: The custom Input component does not forward refs so
searchInputRef passed from page.tsx never reaches the underlying <input> and
focus() silently fails; change the Input component to use React.forwardRef with
a signature like forwardRef<HTMLInputElement, InputProps>((props, ref) => { ...
}) and attach the forwarded ref to the actual input element (e.g., <input
ref={ref} ... />), keep existing props spread, update the component export to
the forwardRef result and adjust the InputProps type to extend
React.InputHTMLAttributes<HTMLInputElement> so ref usage (searchInputRef) from
the page will work and focus() will succeed.
---
Nitpick comments:
In `@src/app/page.tsx`:
- Around line 61-66: The OS detection using deprecated navigator.platform should
be made more robust: in the useEffect callback that calls setIsMounted and
setIsMac, derive a platform string by preferring
navigator.userAgentData?.platform (if available), falling back to
navigator.platform and then navigator.userAgent (to cover older browsers),
normalize to upper/lower case, and setIsMac based on whether that platform
string contains "MAC" (or "MACINTOSH") to be case-insensitive and resilient to
missing fields; update the useEffect that currently references setIsMounted and
setIsMac to use this fallback logic.
🪄 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: b9bfe8bc-d1d4-435d-921e-28236bb3a071
📒 Files selected for processing (2)
.Jules/palette.mdsrc/app/page.tsx
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'k') { | ||
| e.preventDefault() | ||
| searchInputRef.current?.focus() | ||
| } | ||
| } | ||
|
|
||
| document.addEventListener('keydown', handleKeyDown) | ||
| return () => document.removeEventListener('keydown', handleKeyDown) | ||
| }, []) |
There was a problem hiding this comment.
Keyboard shortcut will silently fail — Input component lacks forwardRef.
The Input component in src/components/ui/input.tsx is a plain function component that does not use React.forwardRef. When you pass ref={searchInputRef} on line 372, React cannot forward it to the underlying <input> element, so searchInputRef.current will always be null. The focus() call on line 72 will do nothing.
You need to update the Input component to use forwardRef:
🔧 Proposed fix for `src/components/ui/input.tsx`
+import * as React from "react"
+
-function Input({ className, type, ...props }: React.ComponentProps<"input">) {
+const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>(
+ ({ className, type, ...props }, ref) => {
return (
<input
+ ref={ref}
type={type}
data-slot="input"
className={cn(
"file:text-foreground placeholder:text-muted-foreground selection:bg-primary selection:text-primary-foreground dark:bg-input/30 border-input flex h-9 w-full min-w-0 rounded-md border bg-transparent px-3 py-1 text-base shadow-xs transition-[color,box-shadow] outline-none file:inline-flex file:h-7 file:border-0 file:bg-transparent file:text-sm file:font-medium disabled:pointer-events-none disabled:cursor-not-allowed disabled:opacity-50 md:text-sm",
"focus-visible:border-ring focus-visible:ring-ring/50 focus-visible:ring-[3px]",
"aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive",
className
)}
{...props}
/>
)
-}
+ }
+)
+Input.displayName = "Input"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/page.tsx` around lines 68 - 78, The custom Input component does not
forward refs so searchInputRef passed from page.tsx never reaches the underlying
<input> and focus() silently fails; change the Input component to use
React.forwardRef with a signature like forwardRef<HTMLInputElement,
InputProps>((props, ref) => { ... }) and attach the forwarded ref to the actual
input element (e.g., <input ref={ref} ... />), keep existing props spread,
update the component export to the forwardRef result and adjust the InputProps
type to extend React.InputHTMLAttributes<HTMLInputElement> so ref usage
(searchInputRef) from the page will work and focus() will succeed.
1 participant
💡 What: Added a global keyboard shortcut (
Cmd+Kfor Mac,Ctrl+Kfor Windows) to quickly focus the main search input field.🎯 Why: Searching is a primary action on the Threat Monitor dashboard. Adding a standard keyboard shortcut allows power users to navigate the app much faster without reaching for the mouse, significantly improving the data exploration experience.
📸 Before/After: The search input now displays a subtle, OS-specific visual hint (
⌘KorCtrl K) when the input is empty and not focused.♿ Accessibility:
<Input />component now dynamically receives thearia-keyshortcutsattribute (e.g.,Meta+KorControl+K) depending on the user's OS, ensuring screen readers can announce the shortcut properly.<kbd>tags.isMountedstate check.PR created automatically by Jules for task 7944634498791727082 started by @corebrimtech
Summary by CodeRabbit