-
Notifications
You must be signed in to change notification settings - Fork 432
feat: add macOS-style traffic light buttons for Linux #1988
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
Conversation
On Linux, window decorations are removed, leaving no way to close/minimize/maximize windows. This adds macOS-style traffic light buttons to the settings window header for Linux users. Changes: - Add usePlatform hook for platform detection using @tauri-apps/plugin-os - Add TrafficLights component with close/minimize/maximize buttons - Integrate TrafficLights into settings layout for Linux only Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a platform detection hook and a Linux-only TrafficLights React component, integrates it into desktop header and settings layout, and enables minimizable/maximizable flags for the Settings window in the native window builder. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/window/traffic-lights.tsx (1)
5-10: Consider caching the window instance.
getCurrentWebviewWindow()is called on every render. If this function isn't internally memoized, consider extracting it withuseMemo:+import { useMemo } from "react"; import { getCurrentWebviewWindow } from "@tauri-apps/api/webviewWindow"; import { cn } from "@hypr/utils"; export function TrafficLights({ className }: { className?: string }) { - const win = getCurrentWebviewWindow(); + const win = useMemo(() => getCurrentWebviewWindow(), []); const onClose = () => win.close(); const onMinimize = () => win.minimize(); const onMaximize = () => win.toggleMaximize();This prevents unnecessary calls if the Tauri API doesn't cache the result internally.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/window/traffic-lights.tsx(1 hunks)apps/desktop/src/hooks/usePlatform.ts(1 hunks)apps/desktop/src/routes/app/settings/_layout.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/routes/app/settings/_layout.tsxapps/desktop/src/components/window/traffic-lights.tsxapps/desktop/src/hooks/usePlatform.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/hooks/usePlatform.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/routes/app/settings/_layout.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/routes/app/settings/_layout.tsx
🧬 Code graph analysis (2)
apps/desktop/src/routes/app/settings/_layout.tsx (2)
apps/desktop/src/hooks/usePlatform.ts (1)
useIsLinux(9-11)apps/desktop/src/components/window/traffic-lights.tsx (1)
TrafficLights(5-34)
apps/desktop/src/components/window/traffic-lights.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
⏰ 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). (10)
- GitHub Check: Redirect rules - hyprnote-storybook
- GitHub Check: Header rules - hyprnote-storybook
- GitHub Check: Pages changed - hyprnote-storybook
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: fmt
🔇 Additional comments (4)
apps/desktop/src/components/window/traffic-lights.tsx (1)
13-32: LGTM: Clean implementation of traffic light buttons.The component correctly:
- Uses inline props per guidelines (not shared)
- Applies
cnwith an array for className composition- Sets
data-tauri-drag-region="false"to prevent drag interference- Uses proper button types and semantic HTML
The macOS-style colors (#ff5f57 for close, #ffbd2e for minimize, #28c840 for maximize) accurately replicate the native appearance.
apps/desktop/src/routes/app/settings/_layout.tsx (2)
26-27: LGTM: Appropriate imports for platform-specific UI.The imports are correctly scoped to enable Linux-specific traffic light rendering.
Note: Ensure the critical issue in
apps/desktop/src/hooks/usePlatform.ts(invalid React hooks) is addressed first.
190-194: LGTM: Correct conditional rendering for Linux traffic lights.The implementation properly:
- Wraps the traffic lights in a drag-region container with consistent height (
h-9)- Only renders
<TrafficLights />on Linux where native window controls aren't available- Maintains proper spacing with
px-3This approach makes sense since macOS already has native traffic light buttons, so they're only needed on Linux.
The logic depends on the platform detection hooks being fixed (see critical issue in
apps/desktop/src/hooks/usePlatform.ts).apps/desktop/src/hooks/usePlatform.ts (1)
5-19: These are not valid React hooks; cache the platform detection.These functions use the "use" prefix but don't call any React hooks internally, violating React's Rules of Hooks. Additionally,
platform()is invoked on every render, which is inefficient since the platform never changes at runtime.Recommended solution: Cache the platform at module level since it's constant:
-export function usePlatform(): Platform { - return platform(); -} - -export function useIsLinux(): boolean { - return usePlatform() === "linux"; -} - -export function useIsMacos(): boolean { - return usePlatform() === "macos"; -} - -export function useIsWindows(): boolean { - return usePlatform() === "windows"; -} +// Cache at module level since platform never changes at runtime +const currentPlatform = platform(); + +export function usePlatform(): Platform { + return currentPlatform; +} + +export function useIsLinux(): boolean { + return currentPlatform === "linux"; +} + +export function useIsMacos(): boolean { + return currentPlatform === "macos"; +} + +export function useIsWindows(): boolean { + return currentPlatform === "windows"; +}Alternative solution: If you need these to be true React hooks (e.g., for future reactivity), use
useMemo:import { useMemo } from "react"; export function usePlatform(): Platform { return useMemo(() => platform(), []); } export function useIsLinux(): boolean { const p = usePlatform(); return useMemo(() => p === "linux", [p]); } // ... similar for other helpers⛔ Skipped due to learnings
Learnt from: CR Repo: fastrepl/hyprnote PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-24T16:32:23.055Z Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.Learnt from: CR Repo: fastrepl/hyprnote PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-24T16:32:19.706Z Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
…hts to main window Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
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: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/index.tsx (1)
16-24: Linux header integration looks good; consider tightening padding logic and verifying target windowThe Linux‑specific header adjustments are wired up cleanly: using
useIsLinux()to gate<TrafficLights />and keeping the drag region handling viadata-tauri-drag-regionplusdata-tauri-drag-region="false"on the buttons is a solid approach. Use ofcn([...])also matches the project guidelines. Based on learnings, …Two small follow‑ups:
- Avoid conflicting padding utilities on Linux
Right now the header classes combine:
className={cn([ "w-full h-9 flex items-center", !leftsidebar.expanded && "pl-[72px]", isLinux && "pl-3", ])}On Linux with a collapsed sidebar both
pl-[72px]andpl-3will be present, which makes the effective padding a bit opaque and potentially order‑dependent. You can make the intent explicit by choosing onepl-*based on conditions, e.g.:function Header({ tabs }: { tabs: Tab[] }) { const { leftsidebar } = useShell(); const isLinux = useIsLinux(); + const headerPadding = + isLinux ? "pl-3" : !leftsidebar.expanded ? "pl-[72px]" : undefined; … <div data-tauri-drag-region className={cn([ "w-full h-9 flex items-center", - !leftsidebar.expanded && "pl-[72px]", - isLinux && "pl-3", + headerPadding, ])} >Adjust the exact values as needed for your layout.
- Confirm this Body header is intended to show TrafficLights
Bodylives undercomponents/main/body, so it’s presumably used by the main app window. The PR text emphasizes adding traffic lights for the Settings window specifically, while other undecorated windows might be updated later. Please double‑check that this header is only used on windows where the Linux traffic lights should appear; if not, you may want to gate it further or rely on the Settings‑specific layout instead.Also applies to: 61-103
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/index.tsx(3 hunks)plugins/windows/src/window/v1.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`), similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/main/body/index.tsx
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : If there are many classNames with conditional logic, use `cn` (import from `hypr/utils`). It is similar to `clsx`. Always pass an array and split by logical grouping.
Applied to files:
apps/desktop/src/components/main/body/index.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/main/body/index.tsx (2)
apps/desktop/src/hooks/usePlatform.ts (1)
useIsLinux(9-11)apps/desktop/src/components/window/traffic-lights.tsx (1)
TrafficLights(5-34)
⏰ 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). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
🔇 Additional comments (1)
plugins/windows/src/window/v1.rs (1)
131-138: Enabling minimize/maximize for Settings looks consistent and necessaryMaking the Settings window minimizable and maximizable aligns it with the Main window and is required for the Linux TrafficLights minimize/maximize actions to work as expected. I don’t see issues with this change; just ensure the new controls are manually exercised on Linux as you noted in the PR.
Summary
On Linux, window decorations are removed (
decorations(false)inplugins/windows/src/window/v1.rs), leaving no way to close, minimize, or maximize windows. This PR adds macOS-style traffic light buttons for Linux users.Changes:
usePlatformhook for platform detection using@tauri-apps/plugin-osTrafficLightscomponent with close (red), minimize (yellow), maximize (green) buttons.minimizable(true)and.maximizable(true)to Settings window builder (was missing, which prevented minimize/maximize from working)Updates since last revision
.minimizable(true)and.maximizable(true)flags to enable those window operationsTesting performed on Linux:
Review & Testing Checklist for Human
decorations(false)- decide if these buttons should show there too (currently Linux-only).Recommended test plan:
Notes
The Settings window traffic lights are rendered inside a sidebar Group component with a nested
data-tauri-drag-regiondiv. If clicks aren't registering, this may be a hit-testing issue with the drag region. The Main window uses a simpler structure that works correctly.Link to Devin run: https://app.devin.ai/sessions/6d971ace7142469990f8bf522babf7d1
Requested by: yujonglee (@yujonglee)