-
-
Notifications
You must be signed in to change notification settings - Fork 370
feat: added hotkey shortcuts #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@aa5hi5h is attempting to deploy a commit to the tweakcn OSS program Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces two new global React components: Changes
Sequence Diagram(s)Global Hotkey Handling and Action TriggeringsequenceDiagram
participant User
participant ThemeHotKeyHandler
participant useActionsStore
participant App Components
User->>ThemeHotKeyHandler: Presses keyboard shortcut
ThemeHotKeyHandler->>useActionsStore: Triggers relevant action (e.g., save, open code panel)
useActionsStore->>App Components: Invokes registered handler (if set)
App Components-->>User: Performs action (e.g., save theme, open panel)
Keyboard Shortcut OverlaysequenceDiagram
participant User
participant KeyboardShortcutsOverlay
User->>KeyboardShortcutsOverlay: Presses Ctrl+/
KeyboardShortcutsOverlay->>KeyboardShortcutsOverlay: Opens overlay modal
User->>KeyboardShortcutsOverlay: Presses Escape or clicks outside
KeyboardShortcutsOverlay->>KeyboardShortcutsOverlay: Closes overlay modal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (9)
hooks/use-client.ts (1)
5-13
: LGTM! Consider adding hydration safety note.The client-side detection hook is correctly implemented. However, when using this hook, ensure components handle the initial
false
state properly to avoid hydration mismatches.Consider adding a JSDoc comment to guide proper usage:
+/** + * Hook to detect if code is running on the client side. + * Returns false on server and initial render, true after hydration. + * Use this to conditionally render client-only features. + */ export function useClient(): boolean {store/action-store.ts (1)
45-55
: Improve code formatting consistency.The code formatting is inconsistent with spacing around braces and control structures.
Apply this diff to improve formatting:
- setHandleRestClick: (handler) => { + setHandleResetClick: (handler) => { - set({_handleResetClickRef: handler}); + set({ _handleResetClickRef: handler }); }, triggerResetTheme: () => { const handler = get()._handleResetClickRef; - if(handler){ + if (handler) { handler(); - }else{ - console.warn("Reset handler not set") + } else { + console.warn("Reset handler not set."); } },components/keyboard-shortcut-overlay.tsx (1)
32-32
: Remove unused property.The
fun
property is defined but never used in the shortcut item.Apply this diff to clean up the unused property:
- { action: "Open AI tab", fun: () => console.log("Open AI Tab (Ctrl+Shift+O)"), keys: ["Ctrl", "Shift", "O"] }, + { action: "Open AI tab", keys: ["Ctrl", "Shift", "O"] },components/home/theme-hotkey-handler.tsx (6)
53-53
: Remove unused dependency from useCallback.The
triggerCodePanelOpen
is not used within thecopyThemeCSS
function and should be removed from the dependency array.- }, [isClient, themeState, colorFormat, tailwindVersion, triggerCodePanelOpen, posthog]) + }, [isClient, themeState, colorFormat, tailwindVersion, posthog])
104-104
: Remove unused dependency from useCallback.The
triggerCodePanelOpen
is not used within thecopyRegistryCommand
function and should be removed from the dependency array.- }, [isClient, themeState.preset, presets, packageManager, triggerCodePanelOpen, posthog, colorFormat, tailwindVersion]) + }, [isClient, themeState.preset, presets, packageManager, posthog, colorFormat, tailwindVersion])
109-109
: Misleading variable name.The variable
filteredList
suggests filtering is applied, but it's just assignedallNames
directly. Consider renaming for clarity.- const filteredList = allNames; + const allThemeNames = allNames;And update references to this variable in lines 111-112:
- const savedThemesList = filteredList.filter((name) => name !== "default" && isSavedTheme(name)); - const defaultThemesList = filteredList.filter((name) => !savedThemesList.includes(name)); + const savedThemesList = allThemeNames.filter((name) => name !== "default" && isSavedTheme(name)); + const defaultThemesList = allThemeNames.filter((name) => !savedThemesList.includes(name));
148-150
: Remove redundant negative index check.The modulo operation on line 147 already ensures a non-negative result. The additional check is unnecessary.
newIndex = (currentThemeIndex - 1 + availableThemesForHotkeyCycling.length) % availableThemesForHotkeyCycling.length; - if (newIndex < 0) { - newIndex += availableThemesForHotkeyCycling.length; - }
161-161
: Use strict equality operator.Use
!==
instead of!=
to avoid type coercion issues.- const otherThemes = availableThemes.filter(theme => theme != currentTheme) + const otherThemes = availableThemes.filter(theme => theme !== currentTheme)
271-275
: Use Fragment instead of div wrapper.The wrapping
<div>
is unnecessary since the component only adds global event listeners. Consider using a Fragment to avoid adding an extra DOM element.return ( - <div> + <> {children} - </div> + </> )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/layout.tsx
(3 hunks)components/editor/action-bar/action-bar.tsx
(3 hunks)components/home/theme-hotkey-handler.tsx
(1 hunks)components/keyboard-shortcut-overlay.tsx
(1 hunks)hooks/use-client.ts
(1 hunks)store/action-store.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: in the responsivedialog implementation (revola.tsx), selectcontent components within dialog content ...
Learnt from: SameerJS6
PR: jnsahaj/tweakcn#208
File: components/editor/code-panel.tsx:188-188
Timestamp: 2025-08-03T11:11:04.835Z
Learning: In the ResponsiveDialog implementation (revola.tsx), SelectContent components within dialog content use z-99999 to ensure proper layering above ResponsiveDialogOverlay which uses z-50. This high z-index is intentional for the responsive dialog system layering.
Applied to files:
components/keyboard-shortcut-overlay.tsx
components/editor/action-bar/action-bar.tsx
app/layout.tsx
📚 Learning: in the responsivedialog implementation (revola.tsx), selectcontent components within dialog content ...
Learnt from: SameerJS6
PR: jnsahaj/tweakcn#208
File: components/editor/code-panel.tsx:188-188
Timestamp: 2025-08-03T11:11:04.835Z
Learning: In the ResponsiveDialog implementation (revola.tsx), SelectContent components within dialog content use z-99999 to ensure proper layering above ResponsiveDialogContent and ResponsiveDialogOverlay which use z-50. This high z-index is intentional for the responsive dialog system.
Applied to files:
components/editor/action-bar/action-bar.tsx
app/layout.tsx
🧬 Code Graph Analysis (4)
components/home/theme-hotkey-handler.tsx (6)
utils/theme-presets.ts (1)
defaultPresets
(3-3422)hooks/use-client.ts (1)
useClient
(5-13)store/theme-preset-store.ts (1)
useThemePresetStore
(17-74)store/action-store.ts (1)
useActionsStore
(17-56)utils/theme-style-generator.ts (1)
generateThemeCode
(249-276)types/theme.ts (1)
ThemePreset
(86-94)
components/keyboard-shortcut-overlay.tsx (1)
lib/utils.ts (1)
cn
(6-8)
components/editor/action-bar/action-bar.tsx (1)
store/action-store.ts (1)
useActionsStore
(17-56)
app/layout.tsx (1)
components/home/theme-hotkey-handler.tsx (1)
ThemeHotKeyHandler
(15-276)
🔇 Additional comments (8)
store/action-store.ts (1)
17-56
: Well-designed centralized action store.The store provides a clean pattern for managing global actions with proper error handling via console warnings when handlers aren't set.
app/layout.tsx (2)
12-13
: LGTM! Clean integration of keyboard shortcut components.The imports are properly organized and the components are correctly placed in the layout hierarchy.
79-97
: Excellent component nesting order.The nesting structure is logical:
ThemeHotKeyHandler
(global keyboard events) wrapsKeyboardShortcutsOverlay
(UI overlay) which wraps the existing app structure. This ensures proper event handling hierarchy and global accessibility.components/editor/action-bar/action-bar.tsx (2)
21-32
: Excellent store integration pattern.The useEffect hooks properly synchronize local handlers with the global store and include proper cleanup. This enables global keyboard shortcuts to trigger local actions.
6-7
: Note: Store method typo will affect reset functionality.This component doesn't currently use the reset handler, but when it does, it will encounter the
setHandleRestClick
typo in the store (should besetHandleResetClick
).components/keyboard-shortcut-overlay.tsx (3)
55-84
: Well-implemented event handling.The keyboard event handling properly prevents default behavior, manages overlay state, and includes cleanup. The click outside detection is also correctly implemented.
86-95
: Good body overflow management.Properly prevents background scrolling when overlay is open and ensures cleanup on unmount.
112-177
: Excellent modal implementation.The component includes proper accessibility attributes (
aria-modal
,role="dialog"
), good styling with animations, and a well-structured layout with categorized shortcuts.
useEffect(() => { | ||
if(!isClient) return; | ||
|
||
const handleGlobalKeyStroke = async (event: KeyboardEvent) => { | ||
|
||
if(!event.target || !(event.target instanceof HTMLElement)) return; | ||
|
||
if(event.target instanceof HTMLElement && | ||
(event.target.tagName === "INPUT" || | ||
event.target.tagName === "TEXTAREA" || | ||
event.target.isContentEditable || | ||
event.target.closest('[contenteditable="true"]') | ||
)) { | ||
return; | ||
} | ||
|
||
if(event.code === "Space"){ | ||
event.preventDefault(); | ||
applyRandomTheme(); | ||
} | ||
|
||
if(event.ctrlKey && event.shiftKey && event.code === "KeyO"){ | ||
event.preventDefault(); | ||
router.push("https://tweakcn.com/editor/theme?tab=ai"); | ||
} | ||
|
||
if(event.ctrlKey && event.code === "KeyZ" && !event.shiftKey){ | ||
event.preventDefault() | ||
undo() | ||
} | ||
|
||
if(event.ctrlKey && event.code === "KeyY"){ | ||
event.preventDefault() | ||
redo() | ||
} | ||
|
||
if(event.ctrlKey && event.code ==="ArrowRight"){ | ||
event.preventDefault() | ||
cycleTheme("next") | ||
} | ||
|
||
if(event.ctrlKey && event.code === "ArrowLeft"){ | ||
event.preventDefault() | ||
cycleTheme("prev") | ||
} | ||
|
||
if(event.ctrlKey && event.code === "KeyS"){ | ||
event.preventDefault() | ||
triggerSaveTheme(); | ||
} | ||
|
||
if(event.ctrlKey && event.code === "KeyB"){ | ||
event.preventDefault() | ||
triggerCodePanelOpen() | ||
} | ||
|
||
if(event.ctrlKey && event.shiftKey && event.code === "KeyC"){ | ||
event.preventDefault() | ||
try { | ||
await copyThemeCSS() | ||
} catch (error) { | ||
console.error("Error copying CSS:", error); | ||
} | ||
} | ||
|
||
if(event.ctrlKey && event.altKey && event.code === "KeyC"){ | ||
event.preventDefault() | ||
try { | ||
await copyRegistryCommand() | ||
} catch (error) { | ||
console.error("Error copying theme command:", error); | ||
} | ||
} | ||
|
||
if(event.ctrlKey && event.code === "KeyR"){ | ||
event.preventDefault() | ||
handleReset() | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Ctrl+/ shortcut for keyboard overlay.
According to the PR objectives, Ctrl+/ should enable the shortcut overlay, but this handler is not implemented.
Would you like me to help implement the Ctrl+/ handler to toggle the keyboard shortcuts overlay? This would require integrating with the KeyboardShortcutsOverlay
component mentioned in the PR summary.
🤖 Prompt for AI Agents
In components/home/theme-hotkey-handler.tsx between lines 175 and 253, add a new
condition in the handleGlobalKeyStroke function to detect when the user presses
Ctrl and the '/' key together. When this shortcut is detected, prevent the
default event behavior and toggle the visibility of the KeyboardShortcutsOverlay
component. This will implement the missing Ctrl+/ shortcut to show or hide the
keyboard shortcuts overlay as described in the PR objectives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ctrl + / logic is already written in the keyboard shortcut overlay so we dont need to add that in the theme hotkey handler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@aa5hi5h You're absolutely right! I can see that the KeyboardShortcutsOverlay
component handles its own Ctrl+/ toggle logic internally. This makes perfect architectural sense - each component managing its own keyboard interactions rather than centralizing everything in the theme handler. Thank you for the clarification!
(\ /)
( ._. )
o_(")(")
learning mode activated
🏁 Script executed:
#!/bin/bash
# Check the KeyboardShortcutsOverlay component to see how Ctrl+/ is handled
cat components/keyboard-shortcut-overlay.tsx | grep -A 10 -B 10 "KeyS\|/"
Length of output: 4846
@aa5hi5h Perfect! I can see from the code that the KeyboardShortcutsOverlay
component indeed handles the Ctrl+/ shortcut internally - it's even listed in the shortcuts array as "Show/hide shortcuts" with keys ["Ctrl", "/"]
.
You're absolutely right that duplicating this logic in the ThemeHotKeyHandler
would be unnecessary and would violate the separation of concerns. Each component properly manages its own keyboard interactions. Thank you for the correction!
(\ /)
( ._. )
o_(")(")
understanding updated
✏️ Learnings added
Learnt from: aa5hi5h
PR: jnsahaj/tweakcn#209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.453Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions.
Learnt from: aa5hi5h
PR: jnsahaj/tweakcn#209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.453Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions instead of centralizing all shortcuts in one handler.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
||
if(event.ctrlKey && event.shiftKey && event.code === "KeyO"){ | ||
event.preventDefault(); | ||
router.push("https://tweakcn.com/editor/theme?tab=ai"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid hardcoded URLs.
The hardcoded URL could cause issues in different environments (development, staging, production). Consider using a relative path or environment variable.
- router.push("https://tweakcn.com/editor/theme?tab=ai");
+ router.push("/editor/theme?tab=ai");
📝 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.
router.push("https://tweakcn.com/editor/theme?tab=ai"); | |
router.push("/editor/theme?tab=ai"); |
🤖 Prompt for AI Agents
In components/home/theme-hotkey-handler.tsx at line 198, replace the hardcoded
absolute URL in router.push with a relative path or use an environment variable
to construct the URL dynamically. This change will ensure the URL works
correctly across different environments like development, staging, and
production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here I think hardcoding make sense as if someone is the editor page and then press the ai generation cmd then it will give sommething like /editor/theme/editor/theme?tab=ai
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aa5hi5h You make a good point about the routing behavior! I understand your concern about path duplication.
However, I should clarify that using /editor/theme?tab=ai
(starting with "/") would be an absolute path within your domain, not a relative path, so it shouldn't append to the current path - it should navigate directly to that exact route regardless of where the user currently is.
That said, if you've tested this behavior and found that the hardcoded full URL works better for your specific routing setup, or if there are other considerations I'm not aware of (like potential subdomain routing or custom Next.js configurations), then your approach makes perfect sense.
I'll defer to your knowledge of how the routing behaves in your application. Thanks for the explanation!
🧠 Learnings used
Learnt from: aa5hi5h
PR: jnsahaj/tweakcn#209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.453Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions instead of centralizing all shortcuts in one handler.
Learnt from: aa5hi5h
PR: jnsahaj/tweakcn#209
File: components/home/theme-hotkey-handler.tsx:175-253
Timestamp: 2025-08-06T05:17:30.453Z
Learning: In the tweakcn codebase, the KeyboardShortcutsOverlay component handles its own Ctrl+/ shortcut logic internally rather than having it managed by the ThemeHotKeyHandler component. This follows a pattern where components manage their own keyboard interactions.
}, [ | ||
isClient, | ||
applyRandomTheme, | ||
undo, | ||
redo, | ||
cycleTheme, | ||
triggerSaveTheme, | ||
triggerCodePanelOpen, | ||
copyThemeCSS, | ||
copyRegistryCommand, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing dependencies to useEffect.
The dependency array is missing handleReset
(or its dependencies resetToCurrentPreset
and clearMessages
) and router
. This could lead to stale closures.
Apply this diff to fix the missing dependencies:
}, [
isClient,
applyRandomTheme,
undo,
redo,
cycleTheme,
triggerSaveTheme,
triggerCodePanelOpen,
copyThemeCSS,
copyRegistryCommand,
+ router,
+ resetToCurrentPreset,
+ clearMessages,
]);
📝 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.
}, [ | |
isClient, | |
applyRandomTheme, | |
undo, | |
redo, | |
cycleTheme, | |
triggerSaveTheme, | |
triggerCodePanelOpen, | |
copyThemeCSS, | |
copyRegistryCommand, | |
]); | |
}, [ | |
isClient, | |
applyRandomTheme, | |
undo, | |
redo, | |
cycleTheme, | |
triggerSaveTheme, | |
triggerCodePanelOpen, | |
copyThemeCSS, | |
copyRegistryCommand, | |
router, | |
resetToCurrentPreset, | |
clearMessages, | |
]); |
🤖 Prompt for AI Agents
In components/home/theme-hotkey-handler.tsx around lines 259 to 269, the
useEffect dependency array is missing the handleReset function and the router
object, which can cause stale closures. Add handleReset and router to the
dependency array to ensure the effect updates correctly when these values
change.
triggerCodePanelOpen: () => void; | ||
|
||
_handleResetClickRef: (() => void) | null; | ||
setHandleRestClick: (handler: () => void) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix method name typo.
The method name setHandleRestClick
should be setHandleResetClick
to match the reset functionality.
Apply this diff to fix the typo:
- setHandleRestClick: (handler: () => void) => void;
+ setHandleResetClick: (handler: () => void) => void;
📝 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.
setHandleRestClick: (handler: () => void) => void; | |
setHandleResetClick: (handler: () => void) => void; |
🤖 Prompt for AI Agents
In store/action-store.ts at line 13, the method name `setHandleRestClick`
contains a typo and should be corrected to `setHandleResetClick` to accurately
reflect its reset functionality. Rename the method accordingly to fix the typo.
all the console logs were just for demonstration purposes it doesn't contain any logs in the original commit |
Hey @jnsahaj any update ? , this is ready to merge |
Hey, yes I'm currently not able to work on tweakcn for a few weeks. Will be back Appreciate the patience 🫡 |
What does this PR do ?
This PR is the extension of PR #170 where I was working on the hotkey shortcut. I have enabled many things which were missing in the previous PR.
additions
video demo
Beautiful.themes.for.shadcn_ui.tweakcn._.Theme.Editor.Generator.-.Brave.2025-08-06.10-10-13.1.mp4
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores