feat/tab-close-button #2359
feat/tab-close-button #2359
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 read-only hook to detect when a session's enhanced notes are being generated and surfaces that state as a "finalizing" spinner in session tabs and timeline items by passing a new Changes
Sequence Diagram(s)mermaid SessionsIndex->>Hook: call useIsSessionEnhancing(sessionId) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/sessions/index.tsx (1)
39-43: Consider extracting spinner state logic to a shared hook.The pattern of computing
isEnhancing,isFinalizing, andshowSpinneris duplicated intimeline/item.tsx(lines 42-48). Extracting this to a reusable hook likeuseSessionSpinnerState(sessionId)would reduce duplication and improve maintainability.Example implementation:
// In a shared hooks file export function useSessionSpinnerState(sessionId: string | null) { const sessionMode = useListener((state) => sessionId ? state.getSessionMode(sessionId) : "inactive" ); const isEnhancing = useIsSessionEnhancing(sessionId ?? ""); const isFinalizing = sessionMode === "finalizing"; const showSpinner = isFinalizing || isEnhancing; return { isEnhancing, isFinalizing, showSpinner }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/main/body/sessions/index.tsx(2 hunks)apps/desktop/src/components/main/body/shared.tsx(4 hunks)apps/desktop/src/components/main/sidebar/timeline/item.tsx(3 hunks)apps/desktop/src/hooks/useEnhancedNotes.ts(2 hunks)
🧰 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/hooks/useEnhancedNotes.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/hooks/useEnhancedNotes.tsapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/main/body/sessions/index.tsxapps/desktop/src/components/main/sidebar/timeline/item.tsx
**/*.{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/hooks/useEnhancedNotes.tsapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/main/body/sessions/index.tsxapps/desktop/src/components/main/sidebar/timeline/item.tsx
**/*.{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/hooks/useEnhancedNotes.tsapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/main/body/sessions/index.tsxapps/desktop/src/components/main/sidebar/timeline/item.tsx
**/*.{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/hooks/useEnhancedNotes.tsapps/desktop/src/components/main/body/shared.tsxapps/desktop/src/components/main/body/sessions/index.tsxapps/desktop/src/components/main/sidebar/timeline/item.tsx
🧬 Code graph analysis (4)
apps/desktop/src/hooks/useEnhancedNotes.ts (2)
apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(60-65)apps/desktop/src/contexts/ai-task.tsx (1)
useAITask(49-60)
apps/desktop/src/components/main/body/shared.tsx (1)
extensions/shared/types/hypr-extension.d.ts (1)
Spinner(425-425)
apps/desktop/src/components/main/body/sessions/index.tsx (3)
apps/desktop/src/hooks/useEnhancedNotes.ts (1)
useIsSessionEnhancing(176-195)apps/desktop/src/components/main/body/shared.tsx (1)
TabItemBase(38-162)extensions/calendar/components/icons.tsx (1)
StickyNoteIcon(86-104)
apps/desktop/src/components/main/sidebar/timeline/item.tsx (3)
apps/desktop/src/contexts/listener.tsx (1)
useListener(37-49)apps/desktop/src/hooks/useEnhancedNotes.ts (1)
useIsSessionEnhancing(176-195)extensions/shared/types/hypr-extension.d.ts (1)
Spinner(425-425)
⏰ 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). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
🔇 Additional comments (3)
apps/desktop/src/hooks/useEnhancedNotes.ts (1)
176-195: LGTM! Clean implementation of enhancement status check.The function correctly aggregates AI task statuses for all enhanced notes in a session. The use of
useMemoto derive task IDs is appropriate for performance, and the logic properly handles edge cases (null/empty arrays).apps/desktop/src/components/main/body/shared.tsx (1)
110-112: LGTM! Clean conditional rendering for finalizing state.The three-way conditional properly handles finalizing, active, and default icon states. The spinner provides clear visual feedback during enhancement operations.
apps/desktop/src/components/main/sidebar/timeline/item.tsx (1)
185-197: LGTM! Layout properly accommodates the loading spinner.The horizontal flex layout with
flex-shrink-0on the spinner ensures proper spacing and alignment. Themin-w-0on the content container correctly enables text truncation when space is constrained.
ComputelessComputer
force-pushed
the
feat/tab-close-button
branch
from
73a2372 to
c9ab57d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/shared.tsx (1)
110-119: Consider: Close button behavior during finalization.The current implementation allows users to close tabs while they're finalizing (close button appears on hover even when
finalizing={true}). If this is intentional to give users control, the implementation is correct.However, if finalization should prevent closing, you may want to conditionally disable the close button:
<div className={cn([ "absolute inset-0 flex items-center justify-center transition-opacity duration-200", - isHovered ? "opacity-100" : "opacity-0", + isHovered && !finalizing ? "opacity-100" : "opacity-0", ])} >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/main/body/sessions/index.tsx(2 hunks)apps/desktop/src/components/main/body/shared.tsx(4 hunks)apps/desktop/src/components/main/sidebar/timeline/item.tsx(3 hunks)apps/desktop/src/hooks/useEnhancedNotes.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/components/main/sidebar/timeline/item.tsx
- apps/desktop/src/components/main/body/sessions/index.tsx
🧰 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/shared.tsxapps/desktop/src/hooks/useEnhancedNotes.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/shared.tsxapps/desktop/src/hooks/useEnhancedNotes.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/shared.tsxapps/desktop/src/hooks/useEnhancedNotes.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/shared.tsxapps/desktop/src/hooks/useEnhancedNotes.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/useEnhancedNotes.ts
🧬 Code graph analysis (2)
apps/desktop/src/components/main/body/shared.tsx (1)
extensions/shared/types/hypr-extension.d.ts (1)
Spinner(425-425)
apps/desktop/src/hooks/useEnhancedNotes.ts (2)
apps/desktop/src/store/zustand/ai-task/task-configs/index.ts (1)
createTaskId(60-65)apps/desktop/src/contexts/ai-task.tsx (1)
useAITask(49-60)
⏰ 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). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
🔇 Additional comments (1)
apps/desktop/src/hooks/useEnhancedNotes.ts (1)
176-195: LGTM! Clean implementation of the enhancing state hook.The hook correctly:
- Retrieves enhanced note IDs for the session
- Maps them to task IDs using the established pattern
- Uses
useMemoto optimize the task ID array computation- Leverages
useAITaskto check if any enhancement task is generating- Handles undefined
enhancedNoteIdswith the|| []fallback
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/body/shared.tsx (1)
60-72: Critical: Undefined variable reference.Line 61 references
isEmptyTab, but this prop was removed in the refactoring. This will cause aReferenceErrorat runtime and break the context menu functionality.🔎 Proposed fix
If the
isEmptyTabcheck is no longer needed, remove it:const contextMenu = !active - ? selected && !isEmptyTab + ? selected ? [{ id: "close-tab", text: "Close", action: handleCloseThis }] : [ { id: "close-tab", text: "Close", action: handleCloseThis },Alternatively, if different logic is needed based on the
finalizingstate or another condition, please clarify the intended behavior.
🧹 Nitpick comments (1)
apps/desktop/src/components/main/sidebar/timeline/item.tsx (1)
4-4: Remove unused import.
ContextMenuItemis imported but never used in this file.🔎 Proposed fix
-import { ContextMenuItem } from "@hypr/ui/components/ui/context-menu";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/shared.tsx(4 hunks)apps/desktop/src/components/main/sidebar/timeline/item.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Format using
dprint fmtfrom the root. Do not usecargo fmt.
Files:
apps/desktop/src/components/main/sidebar/timeline/item.tsxapps/desktop/src/components/main/body/shared.tsx
**/*.{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/sidebar/timeline/item.tsxapps/desktop/src/components/main/body/shared.tsx
**/*.{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/sidebar/timeline/item.tsxapps/desktop/src/components/main/body/shared.tsx
**/*.{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/sidebar/timeline/item.tsxapps/desktop/src/components/main/body/shared.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/main/body/shared.tsx (1)
extensions/shared/types/hypr-extension.d.ts (1)
Spinner(425-425)
⏰ 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 (3)
apps/desktop/src/components/main/sidebar/timeline/item.tsx (1)
177-189: LGTM!The layout refactoring correctly implements the spinner display with proper flex behaviors. The
min-w-0on the content wrapper enables text truncation within the flex container, andflex-shrink-0prevents the spinner from being compressed.apps/desktop/src/components/main/body/shared.tsx (2)
24-24: LGTM!The prop rename from
isEmptyTabtofinalizingis clear and aligns well with the new functionality.Also applies to: 42-42
114-123: LGTM!The rendering logic correctly prioritizes the
finalizingstate over theactivestate, and the spinner size matches the icon container dimensions.
Description
This is part 1 of 3 in a stack: