Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Dec 9, 2025

ctrl-shift-0 doesn't work in windows (eaten by IME system). so need to switch to something else... chose Alt-0 for windows.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

This pull request adds platform-specific keyboard shortcuts for focusing AI input. Windows users now use Alt+0 while non-Windows users use Ctrl+Shift+0. Changes include: updating a keyboard component to accept optional platform-specific override props (windows, mac, linux), modifying key binding logic to conditionally apply Windows shortcuts, updating UI text to display the correct shortcut based on platform detection, and updating documentation to reflect the Windows variant.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • kbd.tsx component changes: Verify that platform-specific overrides are correctly selected and applied before key parsing. Check that the PlatformContext integration works as intended and that existing BrowserOnly behavior is preserved.
  • Key binding consistency: Confirm that the key bindings in keymodel.ts (Alt+c{Digit0} and Alt+c{Numpad0} for Windows; Ctrl:Shift:c{Digit0} and Ctrl:Shift:c{Numpad0} for non-Windows) align with the UI text displayed in aipanel.tsx.
  • Platform detection utility: Verify that the isWindows() function correctly identifies Windows platforms across all usage locations.
  • Documentation accuracy: Ensure that the waveai.mdx documentation correctly reflects both the Windows and non-Windows keyboard shortcuts.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix windows shortcut for waveai focus' directly and clearly describes the main change—fixing a Windows-specific keyboard shortcut for the WaveAI focus feature. It is concise, specific, and accurately reflects the primary purpose of the changeset.
Description check ✅ Passed The description explains the technical motivation for the change (Ctrl+Shift+0 being intercepted by Windows IME) and identifies the chosen solution (Alt+0 for Windows), which directly relates to the modifications across all affected files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/waveai-win-keybinding

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
docs/src/components/kbd.tsx (1)

47-60: Platform-specific overrides for Kbd look correct; consider a shared props type.

The windows / mac / linux override selection logic matches the PlatformContext platform values and cleanly falls back to k when no override is provided. This will correctly render Alt:0 on Windows while preserving existing behavior elsewhere. To avoid duplication, you could introduce a KbdProps type and reuse it for both KbdInternal and Kbd instead of repeating the inline { k; windows?; mac?; linux? } shape.

Also applies to: 72-73

frontend/app/store/keymodel.ts (1)

26-26: Windows-specific Alt+0 binding for Wave AI focus is correctly wired.

Using isWindows() to switch between Alt:c{Digit0} / Alt:c{Numpad0} on Windows and the existing Ctrl:Shift:c{Digit0} / Ctrl:Shift:c{Numpad0} elsewhere keeps behavior consistent with the docs and UI while avoiding IME conflicts on Windows. If you want to de-duplicate slightly, you could factor a small helper like const focusWaveAIInput = () => { WaveAIModel.getInstance().focusInput(); return true; }; and reuse it across the four globalKeyMap.set calls.

Also applies to: 610-628

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 440be99 and 769d61e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • docs/docs/waveai.mdx (1 hunks)
  • docs/src/components/kbd.tsx (2 hunks)
  • frontend/app/aipanel/aipanel.tsx (2 hunks)
  • frontend/app/store/keymodel.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/app/aipanel/aipanel.tsx (1)
frontend/util/platformutil.ts (1)
  • isWindows (16-18)
docs/src/components/kbd.tsx (1)
docs/src/components/platformcontext.tsx (1)
  • PlatformContext (14-14)
⏰ 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: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build for TestDriver.ai
🔇 Additional comments (2)
docs/docs/waveai.mdx (1)

22-22: Docs shortcut now matches platform-specific behavior.

Using windows="Alt:0" here aligns the documentation with the new Windows keybinding while preserving Ctrl:Shift:0 elsewhere; looks good.

frontend/app/aipanel/aipanel.tsx (1)

10-10: AI welcome message shortcut text now matches platform-specific bindings.

Conditionally rendering Alt + 0 on Windows and Ctrl + Shift + 0 elsewhere keeps the in-panel instructions in sync with the new keybindings and the docs; this looks consistent across platforms.

Also applies to: 138-151

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants