🎨 Palette: [UX improvement] Add Cmd/Ctrl+K shortcut to search - #277
🎨 Palette: [UX improvement] Add Cmd/Ctrl+K shortcut to search#277mkk2026 wants to merge 1 commit into
Conversation
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. |
📝 WalkthroughWalkthroughThis PR adds keyboard shortcut functionality (Cmd/Ctrl+K) to a search input component. A platform-detection effect registers a global keyboard listener that focuses the search input on Mac (⌘K) or Windows/Linux (Ctrl+K). A journal entry documents the SSR hydration considerations resolved through client-side state initialization. ChangesKeyboard Shortcut Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 746d82fdbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| setIsMac(typeof window !== 'undefined' && navigator.platform.toUpperCase().indexOf('MAC') >= 0) | ||
|
|
||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if ((e.metaKey || e.ctrlKey) && e.key === 'k') { |
There was a problem hiding this comment.
On macOS, the handler currently fires for both Meta+K and Control+K because the condition uses (e.metaKey || e.ctrlKey). That means pressing Ctrl+K (a common text-editing shortcut in Mac text fields) unexpectedly steals focus to the global search input, while aria-keyshortcuts advertises only Meta+K. Limiting the modifier to platform-specific behavior (Meta on Mac, Control elsewhere) avoids this shortcut conflict and keeps behavior aligned with the UI hint and accessibility metadata.
Useful? React with 👍 / 👎.
![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 (2)
src/app/page.tsx (2)
61-61: ⚡ Quick winReplace deprecated
navigator.platformwithnavigator.userAgentData+ fallback
navigator.platformis deprecating for all browsers. The safer pattern isnavigator?.userAgentData?.platform || navigator?.platform, sincenavigator.userAgentDatais not yet implemented in all browsers (notably Firefox and Safari historically).♻️ Proposed fix
- setIsMac(typeof window !== 'undefined' && navigator.platform.toUpperCase().indexOf('MAC') >= 0) + const platform = (navigator.userAgentData?.platform ?? navigator.platform ?? '').toUpperCase() + setIsMac(platform.includes('MAC'))The
typeof window !== 'undefined'guard is also redundant here —useEffectonly runs on the client.🤖 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` at line 61, In the useEffect that sets isMac (the setIsMac call), replace the deprecated navigator.platform usage with a modern check using navigator.userAgentData?.platform with a fallback to navigator.platform; e.g. read const platform = navigator?.userAgentData?.platform || navigator?.platform and setIsMac(platform?.toUpperCase().includes('MAC')); also remove the redundant typeof window !== 'undefined' guard since useEffect only runs client-side. Ensure you reference the setIsMac call and the enclosing useEffect when making the change.
374-380: ⚡ Quick winConsider hiding the kbd hint when the search input is focused
The
!searchQuerycondition alone keeps the⌘K/Ctrl+Khint visible even when the input is already focused (and empty). Many search UIs (e.g., VS Code, Linear) hide the hint on focus to avoid visual clutter.♻️ Suggested approach
+ const [searchFocused, setSearchFocused] = useState(false) ... <Input ref={searchInputRef} + onFocus={() => setSearchFocused(true)} + onBlur={() => setSearchFocused(false)} ... /> - {!searchQuery && ( + {!searchQuery && !searchFocused && ( <div ...> <kbd ...>...</kbd> </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 374 - 380, The hint visibility should also consider whether the search input is focused; update the render condition from checking only !searchQuery to also require the input not be focused (e.g., {!searchQuery && !isSearchFocused}). Add focus tracking for the search input (create a boolean state like isSearchFocused or use an input ref to compare to document.activeElement) and wire it to the input's onFocus/onBlur (or set when focus changes) so the kbd hint (the element using isMac and searchQuery) hides while the user is actively focused in the search field.
🤖 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 `@src/app/page.tsx`:
- Around line 365-366: The Input component is not forwarding refs so
searchInputRef.current stays null and focus() fails; update the Input component
(src/components/ui/input.tsx) to use React.forwardRef<HTMLInputElement,
React.ComponentProps<"input">> and accept (props, ref) then pass ref to the
underlying <input> element, set Input.displayName = "Input", and export the
forwarded component so searchInputRef in page.tsx can call focus() successfully.
---
Nitpick comments:
In `@src/app/page.tsx`:
- Line 61: In the useEffect that sets isMac (the setIsMac call), replace the
deprecated navigator.platform usage with a modern check using
navigator.userAgentData?.platform with a fallback to navigator.platform; e.g.
read const platform = navigator?.userAgentData?.platform || navigator?.platform
and setIsMac(platform?.toUpperCase().includes('MAC')); also remove the redundant
typeof window !== 'undefined' guard since useEffect only runs client-side.
Ensure you reference the setIsMac call and the enclosing useEffect when making
the change.
- Around line 374-380: The hint visibility should also consider whether the
search input is focused; update the render condition from checking only
!searchQuery to also require the input not be focused (e.g., {!searchQuery &&
!isSearchFocused}). Add focus tracking for the search input (create a boolean
state like isSearchFocused or use an input ref to compare to
document.activeElement) and wire it to the input's onFocus/onBlur (or set when
focus changes) so the kbd hint (the element using isMac and searchQuery) hides
while the user is actively focused in the search field.
🪄 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: 1df5e646-b1c4-4a65-aebf-f8aab8012d6c
📒 Files selected for processing (2)
.Jules/palette.mdsrc/app/page.tsx
| <Input | ||
| ref={searchInputRef} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm Input component uses React.forwardRef
rg -n "forwardRef" --type=tsx --type=ts -g "**/components/ui/input*"
# fallback if type=tsx not supported
fd -i "input" --extension tsx --extension ts -p "components/ui" | xargs grep -n "forwardRef"Repository: mkk2026/Security.News.Scraper
Length of output: 204
🏁 Script executed:
git ls-files --cached | head -30Repository: mkk2026/Security.News.Scraper
Length of output: 670
🏁 Script executed:
find . -type d -name "components" | head -10Repository: mkk2026/Security.News.Scraper
Length of output: 634
🏁 Script executed:
find ./src/components -type f -name "*.tsx" -o -name "*.ts" | grep -i inputRepository: mkk2026/Security.News.Scraper
Length of output: 137
🏁 Script executed:
cat ./src/components/ui/input.tsxRepository: mkk2026/Security.News.Scraper
Length of output: 1040
🏁 Script executed:
sed -n '360,385p' ./src/app/page.tsxRepository: mkk2026/Security.News.Scraper
Length of output: 2143
🏁 Script executed:
grep -n "searchInputRef" ./src/app/page.tsx | head -20Repository: mkk2026/Security.News.Scraper
Length of output: 224
🏁 Script executed:
sed -n '55,75p' ./src/app/page.tsxRepository: mkk2026/Security.News.Scraper
Length of output: 728
Wrap the Input component with React.forwardRef to enable ref forwarding
The Input component at src/components/ui/input.tsx does not use React.forwardRef, so searchInputRef will always be null. This breaks the Cmd/Ctrl+K keyboard shortcut at line 66, which attempts to call searchInputRef.current?.focus() — the focus() call silently fails because the ref is never forwarded to the underlying input element.
Fix the Input component:
const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>(
({ className, type, ...props }, ref) => (
<input
ref={ref}
type={type}
// ... rest of implementation
/>
)
)
Input.displayName = "Input"
export { Input }🤖 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 365 - 366, The Input component is not
forwarding refs so searchInputRef.current stays null and focus() fails; update
the Input component (src/components/ui/input.tsx) to use
React.forwardRef<HTMLInputElement, React.ComponentProps<"input">> and accept
(props, ref) then pass ref to the underlying <input> element, set
Input.displayName = "Input", and export the forwarded component so
searchInputRef in page.tsx can call focus() successfully.
1 participant
💡 What: Added a
Cmd/Ctrl+Kkeyboard shortcut to immediately focus the search input, and a visual<kbd>hint that displays when the search is empty.🎯 Why: Searching is the primary action on this dashboard. Power users and keyboard navigators need a quick way to jump to search without clicking, improving efficiency and accessibility.
📸 Before/After: Added a screenshot demonstrating the
Ctrl K/⌘ Kvisual hint inside the input.♿ Accessibility: Added
aria-keyshortcutsto the search input, securely derived from the OS platform, and safely mounted in auseEffectto prevent SSR hydration mismatches. Recorded this learning in.Jules/palette.md.PR created automatically by Jules for task 8498046087493958684 started by @corebrimtech
Summary by CodeRabbit
Release Notes