Auto delete through tabs lifecycle hook#1572
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRefactors the monolithic tabs store into modular Zustand slices (schema, utils, basic, navigation, lifecycle, state), replaces the old store with a composed useTabs hook, adds tests and test utilities, updates app layout to register an on-close handler, refactors Devtool drawer, and tweaks timeline sidebar UI. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as App Layout
participant Store as useTabs (lifecycle/basic)
participant DB as persistedStore
Note over UI,Store: Layout registers on-close handler at mount
UI->>Store: registerOnClose(handler)
Store-->>UI: unsubscribe()
User->>Store: close(tab: sessions)
Store->>Store: invoke onClose handlers
Store->>UI: handler(tab)
UI->>DB: fetch row by tab.id
alt row exists && no metadata
UI->>DB: delete row
else
Note right of UI: no deletion
end
sequenceDiagram
autonumber
actor User
participant Store as useTabs (basic/navigation)
participant Utils as History Utils
User->>Store: openCurrent(tabA)
Store->>Utils: pushHistory(tabA)
Store-->>User: state updated (currentTab=tabA)
User->>Store: openNew(tabB)
Store->>Utils: pushHistory(tabB)
Store-->>User: state updated (currentTab=tabB)
User->>Store: goBack()
Store->>Utils: computeHistoryFlags & select previous
Store-->>User: state updated (currentTab=tabA, flags)
User->>Store: goNext()
Store->>Utils: computeHistoryFlags & select next
Store-->>User: state updated (currentTab=tabB, flags)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 3
🧹 Nitpick comments (2)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
76-85: Consider usingcninstead ofclsxfor consistency.According to the coding guidelines for this path pattern, the
cnutility should be used when composing Tailwind classNames with conditional logic. This button has conditional positioning logic, so switching fromclsxtocnwould align with the project's standards.Apply this diff:
<Button size="sm" variant="outline" onClick={scrollToToday} - className={clsx([ + className={cn([ "group", "relative", "absolute left-1/2 transform -translate-x-1/2",Based on coding guidelines.
apps/desktop2/src/devtool/index.tsx (1)
151-196: LGTM! Clean refactoring with improved modularity.The extraction of
DevtoolSection,SeedList,SeedButton, andNavigationListcomponents improves code organization and reusability. The implementation follows the coding guidelines correctly:
cnutility is used with arrays throughout- Tailwind classes are logically grouped
- No unused code or unnecessary comments
Optional: Consider adding aria-labels for better accessibility.
To improve screen reader support, you could add descriptive
aria-labelattributes:function SeedButton({ seed, onClick }: { seed: SeedDefinition; onClick: () => void }) { return ( <button type="button" onClick={onClick} + aria-label={`Apply ${seed.label} seed`} className={cn([Similarly, the navigation links could benefit from more descriptive labels if needed, though the current text content is already clear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx(2 hunks)apps/desktop2/src/devtool/index.tsx(4 hunks)apps/desktop2/src/routes/app/main/_layout.tsx(1 hunks)apps/desktop2/src/store/zustand/tabs.ts(0 hunks)apps/desktop2/src/store/zustand/tabs/basic.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/index.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/lifecycle.test.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/lifecycle.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/navigation.test.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/navigation.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/schema.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/state.ts(1 hunks)apps/desktop2/src/store/zustand/tabs/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop2/src/store/zustand/tabs.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop2/src/store/zustand/tabs/navigation.tsapps/desktop2/src/store/zustand/tabs/schema.tsapps/desktop2/src/store/zustand/tabs/state.tsapps/desktop2/src/routes/app/main/_layout.tsxapps/desktop2/src/store/zustand/tabs/lifecycle.test.tsapps/desktop2/src/store/zustand/tabs/basic.tsapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/store/zustand/tabs/utils.tsapps/desktop2/src/store/zustand/tabs/navigation.test.tsapps/desktop2/src/store/zustand/tabs/lifecycle.tsapps/desktop2/src/store/zustand/tabs/index.tsapps/desktop2/src/devtool/index.tsx
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/routes/app/main/_layout.tsxapps/desktop2/src/components/main/sidebar/timeline/index.tsxapps/desktop2/src/devtool/index.tsx
🔇 Additional comments (1)
apps/desktop2/src/components/main/sidebar/timeline/index.tsx (1)
87-97: LGTM! Nice directional indicator UX.The wrapper div cleanly separates the button content from the decorative underline, and the conditional positioning of the underline (opposite the button's own position) provides a clear visual cue pointing toward "Today."
1 participant
No description provided.