feat/tab-aware-banner-conditions#2277
Conversation
feat(sidebar): add AI tab state checks for banner visibility
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughDetects AI-tab state in the sidebar, passes two boolean flags into the banner registry, and updates STT/LLM banner visibility, labels, and descriptions to account for whether the corresponding AI tab is active. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/components/main/sidebar/banner/registry.tsx (1)
31-65: Banner suppression conditions look correct; consider whether title-less banners need an a11y label.The updated conditions (
!isAiTranscriptionTabActive/!isAiIntelligenceTabActive) match the intent. Since these banners now omittitle, consider whether you want a consistent accessible “heading” (e.g., a visually-hidden title) for screen readers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/main/sidebar/banner/component.tsx(1 hunks)apps/desktop/src/components/main/sidebar/banner/index.tsx(3 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/main/sidebar/banner/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/components/main/sidebar/banner/types.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 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/sidebar/banner/types.tsapps/desktop/src/components/main/sidebar/banner/index.tsxapps/desktop/src/components/main/sidebar/banner/registry.tsxapps/desktop/src/components/main/sidebar/banner/component.tsx
⏰ 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: fmt
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-14)
🔇 Additional comments (4)
apps/desktop/src/components/main/sidebar/banner/index.tsx (2)
64-86: Memo dependencies/params update is correct.You’re correctly threading
isAiTranscriptionTabActive/isAiIntelligenceTabActiveintocreateBannerRegistryand including them in the memo deps, so registry recalculates when the tab changes.
32-54: AI tab state wiring looks good; please confirmupdateAiTabStatecall signature.The derived booleans and “update existing AI tab vs open new AI tab” behavior match the PR goal. Main thing to double-check is that
updateAiTabState(currentTab, { tab })matches the zustand store’s intended API (object vs id).#!/bin/bash # Locate the zustand tab store types and the updateAiTabState signature + callsites. rg -n --type=ts --type=tsx -C3 '\bupdateAiTabState\b' apps/desktop/src/components rg -n --type=ts --type=tsx -C3 'updateAiTabState\s*\(' apps/desktop/src/components # (Optional) locate the store definition rg -n --type=ts --type=tsx -C3 'create\(\s*\(set,\s*get' apps/desktop/src/components/main/sidebar -Sapps/desktop/src/components/main/sidebar/banner/registry.tsx (1)
8-28: Param additions are straightforward and keep registry creation explicit.No issues with adding the two booleans to
BannerRegistryParams/createBannerRegistry—it keeps the banner logic deterministic and easy to reason about.apps/desktop/src/components/main/sidebar/banner/types.ts (1)
8-16: No string-only assumptions found; type changes are correct.The
descriptionfield correctly acceptsReactNodeacross all usages—registry.tsx passes both JSX fragments and plain strings (both valid ReactNode), and component.tsx renders it safely in JSX (line 50). Makingtitleoptional is appropriate since banners conditionally render the heading block only whentitleexists (lines 36–48 in component.tsx).
Overview