Conversation
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a tab-restore feature: a new restore slice and middleware in the tabs Zustand store to track closed tabs (capped at 10) and reopen the last closed tab, plus a keyboard shortcut (mod+shift+t) wired to the restore action. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 2
🧹 Nitpick comments (1)
apps/desktop/src/store/zustand/tabs/restore.ts (1)
15-22: Remove the type assertion and useOmitfor type-safe property exclusion.The type assertion incorrectly asserts that
Tabhas apinnedproperty, which doesn't exist. Since onlyactiveandslotIdneed to be excluded, usetype TabInput = Omit<Tab, 'active' | 'slotId'>or refactor the function to rely on proper type inference instead of unsafe assertions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/main/body/index.tsx(2 hunks)apps/desktop/src/store/zustand/tabs/index.ts(1 hunks)apps/desktop/src/store/zustand/tabs/restore.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/store/zustand/tabs/index.tsapps/desktop/src/store/zustand/tabs/restore.ts
**/*.{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.
Never do manual state management for form/mutation. UseuseFormfrom tanstack-form anduseQuery/useMutationfrom tanstack-query for 99% cases.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/store/zustand/tabs/index.tsapps/desktop/src/store/zustand/tabs/restore.ts
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/store/zustand/tabs/index.tsapps/desktop/src/store/zustand/tabs/restore.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/main/body/index.tsxapps/desktop/src/store/zustand/tabs/index.tsapps/desktop/src/store/zustand/tabs/restore.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/store/zustand/tabs/index.tsapps/desktop/src/store/zustand/tabs/restore.ts
🧬 Code graph analysis (1)
apps/desktop/src/components/main/body/index.tsx (2)
apps/desktop/src/store/zustand/tabs/index.ts (1)
useTabs(36-48)extensions/shared/types/hypr-extension.d.ts (1)
useTabs(81-107)
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
apps/desktop/src/components/main/body/index.tsx
[error] 623-623: TS2339: Property 'selectNext' does not exist on type 'BasicState & NavigationState & LifecycleState & RestoreState & BasicActions & StateBasicActions & NavigationActions & LifecycleActions & RestoreActions'.
⏰ 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). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (2)
apps/desktop/src/store/zustand/tabs/index.ts (1)
17-47: LGTM! Clean integration of restore functionality.The restore slice is properly integrated with:
- Type extensions for
StateandActions- Correct middleware wrapping order (restore → lifecycle → navigation)
- Consistent logging patterns for the new slice
apps/desktop/src/components/main/body/index.tsx (1)
694-703: LGTM! Standard hotkey implementation.The keyboard shortcut
mod+shift+tfollows the browser convention for reopening closed tabs and is implemented consistently with other hotkeys in the file.
ComputelessComputer
force-pushed
the
feat/tab-restore-functionality
branch
from
1764664 to
7dd3c0a
Compare
ComputelessComputer
force-pushed
the
feat/tab-restore-functionality
branch
3 times, most recently
from
fe44195 to
fab426c
Compare
ComputelessComputer
force-pushed
the
feat/tab-restore-functionality
branch
from
fab426c to
3bcc180
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/store/zustand/tabs/restore.ts (1)
60-89: Address the double state update issue flagged in previous review.The middleware's double
set()call (lines 67 and 83) remains unaddressed. This causes two separate state updates and subscriber notifications, leading to potential race conditions and performance issues. The previous review comment provides a comprehensive solution to merge both updates into a single state change.
🧹 Nitpick comments (2)
apps/desktop/src/store/zustand/tabs/restore.ts (2)
15-22: Consider simplifying type assertions.The explicit type assertion on lines 16-20 and the cast on line 21 suggest potential type definition issues. If
Tabalready includesactive,slotId, andpinnedas required fields, the assertionas Tab & { ... }is redundant.🔎 Simplified version if Tab properties are required
const tabToTabInput = (tab: Tab): TabInput => { - const { active, slotId, pinned, ...rest } = tab as Tab & { - active: boolean; - slotId: string; - pinned: boolean; - }; - return rest as TabInput; + const { active, slotId, pinned, ...rest } = tab; + return rest; };
43-58: Consider inlining the RestoreMiddleware type.Based on the coding guideline to avoid creating types that aren't shared, this type definition could be inlined directly into the
restoreMiddlewareImpldeclaration, as it's only used once.🔎 Inlined version
-type RestoreMiddleware = < - T extends { - tabs: Tab[]; - closedTabs: Tab[]; - }, ->( - f: ( - set: StoreApi<T>["setState"], - get: StoreApi<T>["getState"], - api: StoreApi<T>, - ) => T, -) => ( - set: StoreApi<T>["setState"], - get: StoreApi<T>["getState"], - api: StoreApi<T>, -) => T; - -const restoreMiddlewareImpl: RestoreMiddleware = +const restoreMiddlewareImpl = < + T extends { + tabs: Tab[]; + closedTabs: Tab[]; + }, +>( + config: ( + set: StoreApi<T>["setState"], + get: StoreApi<T>["getState"], + api: StoreApi<T>, + ) => T, +): (( + set: StoreApi<T>["setState"], + get: StoreApi<T>["getState"], + api: StoreApi<T>, +) => T) => - (config) => (set, get, api) => { + (set, get, api) => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/main/body/index.tsx(2 hunks)apps/desktop/src/store/zustand/tabs/index.ts(1 hunks)apps/desktop/src/store/zustand/tabs/restore.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/store/zustand/tabs/index.ts
- apps/desktop/src/components/main/body/index.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/store/zustand/tabs/restore.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/store/zustand/tabs/restore.ts
**/*.{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.
Never do manual state management for form/mutation. UseuseFormfrom tanstack-form anduseQuery/useMutationfrom tanstack-query for 99% cases.
Files:
apps/desktop/src/store/zustand/tabs/restore.ts
**/*.{ts,tsx,rs,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
By default, avoid writing comments at all. If you write one, it should be about 'Why', not 'What'.
Files:
apps/desktop/src/store/zustand/tabs/restore.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: If there are many classNames with conditional logic, usecn(import from@hypr/utils). Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/store/zustand/tabs/restore.ts
⏰ 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: desktop_ci (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (3)
apps/desktop/src/store/zustand/tabs/restore.ts (3)
1-9: LGTM!The imports, constant definition, and
RestoreStatetype are well-structured. The 10-tab history limit is reasonable for balancing memory usage with user convenience.
11-13: LGTM!The
RestoreActionstype is concise and clearly defines the public API for tab restoration.
24-41: LGTM!The
createRestoreSliceimplementation correctly handles the LIFO restoration pattern. The early return prevents errors, and the immutable update usingsliceis appropriate for Zustand state management.
2 participants
Overview
This is part 1 of 3 in a stack made with GitButler: