feat(ota): add update drawer for updates#2318
feat(ota): add update drawer for updates#2318ComputelessComputer wants to merge 2 commits intomainfrom
Conversation
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded@ComputelessComputer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an OTA update drawer UI and integrates it into the left sidebar, extends the OTA state machine with a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
✅ 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
30-40:handleCloseDrawerwiring looks correct; confirm UX for closing mid-downloadExposing
handleCloseDrawerand routing it toupdateStore.trigger.closeDrawer()is consistent with the other handlers and keeps the hook API clean.One behavior to be aware of: if the user closes the drawer while
state === "downloading",showDrawerstaysfalseeven afterdownloadFinishedsets the state to"ready", so the ready state drawer will not auto-reopen for that update. If the intended UX is to always surface the “ready to restart” prompt, you may want a follow-up event (e.g., indownloadFinished) to re-show the drawer or otherwise notify the user.apps/desktop/src/components/main/sidebar/index.tsx (1)
26-80: Consider nestingUpdateDrawerinside the sidebar container to avoid layout surprisesBy changing
LeftSidebarto return a fragment and placing<UpdateDrawer />as a sibling before the main<div>, callers now see two DOM children where previously there was a single sidebar element. IfBottomSheetdoesn’t fully portal out of the normal flow, this can subtly affect flex layout or sizing of the main content.A safer pattern is to keep
LeftSidebar’s root as a single<div>and render<UpdateDrawer />inside it (e.g., as the first child). That preserves the external layout contract while still letting the bottom sheet overlay the UI via its own positioning.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/components/main/sidebar/index.tsx(2 hunks)apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/ota/store.ts(5 hunks)apps/desktop/src/components/main/sidebar/profile/ota/task.ts(1 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/components/main/sidebar/profile/ota/drawer.tsxapps/desktop/src/components/main/sidebar/index.tsxapps/desktop/src/components/main/sidebar/profile/ota/store.tsapps/desktop/src/components/main/sidebar/profile/ota/task.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/components/main/sidebar/profile/ota/store.tsapps/desktop/src/components/main/sidebar/profile/ota/task.ts
🧬 Code graph analysis (2)
apps/desktop/src/components/main/sidebar/index.tsx (3)
apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx (1)
UpdateDrawer(12-125)apps/desktop/src/components/main/sidebar/banner/index.tsx (1)
BannerArea(13-128)apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
ProfileSection(38-247)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
apps/desktop/src/components/main/sidebar/profile/ota/store.ts (1)
updateStore(27-130)
⏰ 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 (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
🔇 Additional comments (2)
apps/desktop/src/components/main/sidebar/profile/ota/store.ts (1)
24-128:showDrawerstate and transitions are coherent with the new drawerThe new
showDrawerflag is consistently managed: it is initialized tofalse, turned on instartDownload, and cleared incancelDownload,closeDrawer, andreset. This aligns withUpdateDrawer’sshouldShowlogic and keeps the drawer visibility decoupled from the underlying OTA state machine, which is a clean separation.apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx (1)
12-124: UpdateDrawer cleanly reflects OTA state and actionsThe component cleanly maps OTA state to UI:
shouldShowcorrectly gates visibility, the progress indicator and “ready” visuals reflectdownloadProgressandupdate.version, and the close/install/cancel actions are wired through the hook as expected.cnusage and conditional rendering patterns are consistent with local conventions.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
284-312: Consider adding accessibility attributes to the progress indicator.The circular progress SVG implementation is correct and visually clear. However, for better accessibility, consider adding ARIA attributes to communicate download progress to screen reader users.
Apply this diff to enhance accessibility:
- {isDownloading && ( - <svg className="absolute -inset-1 size-10" viewBox="0 0 40 40"> + {isDownloading && ( + <svg + className="absolute -inset-1 size-10" + viewBox="0 0 40 40" + role="progressbar" + aria-label="Download progress" + aria-valuenow={downloadProgress.percentage} + aria-valuemin={0} + aria-valuemax={100} + >apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx (1)
36-46: Consider using the Button component for the close button.For consistency, consider using the
Buttoncomponent with an icon-only variant instead of a custom button element. This ensures uniform styling and behavior across all interactive elements.If the
Buttoncomponent supports icon-only or ghost variants, apply this diff:- <button - onClick={handleCloseDrawer} - className={cn([ - "flex h-8 w-8 items-center justify-center", - "rounded-full", - "hover:bg-neutral-100", - "transition-colors", - ])} - > + <Button + variant="ghost" + size="icon" + onClick={handleCloseDrawer} + className="h-8 w-8 rounded-full" + > <X className="h-4 w-4 text-neutral-500" /> - </button> + </Button>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/components/main/sidebar/profile/index.tsx(3 hunks)apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx(1 hunks)apps/desktop/src/components/main/sidebar/profile/ota/store.ts(6 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/components/main/sidebar/profile/index.tsxapps/desktop/src/components/main/sidebar/profile/ota/store.tsapps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx
**/*.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/profile/ota/store.ts
🧬 Code graph analysis (2)
apps/desktop/src/components/main/sidebar/profile/index.tsx (2)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
useOTA(30-41)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx (3)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
useOTA(30-41)extensions/shared/types/hypr-extension.d.ts (3)
BottomSheet(139-139)BottomSheetContent(140-140)Button(159-159)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). (6)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: desktop_ci (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (8)
apps/desktop/src/components/main/sidebar/profile/index.tsx (3)
30-30: LGTM!Clean import of the OTA hook for integration with the profile UI.
259-261: LGTM!Clean destructuring of OTA state and derived boolean. The
downloadProgressobject is guaranteed to exist with apercentageproperty based on the store initialization.
313-331: LGTM!The avatar rendering maintains the existing logic while properly accommodating the progress overlay. Good use of
cnwith array syntax for className management.apps/desktop/src/components/main/sidebar/profile/ota/store.ts (2)
24-24: LGTM!Clean addition of the
showDrawerboolean flag to the OTA context with appropriate initialization.Also applies to: 38-38
69-69: LGTM!The state transitions for
showDrawerare logically sound:
- Shows drawer when download finishes (ready state)
- Hides drawer on start, cancel, close, and reset
The dedicated
closeDrawerevent provides a clean API for dismissing the drawer.Also applies to: 97-97, 108-113, 128-128
apps/desktop/src/components/main/sidebar/profile/ota/drawer.tsx (3)
1-10: LGTM!Clean import structure with all necessary dependencies for the UpdateDrawer component.
12-18: LGTM!Clean component setup with appropriate derived state. The double-check (
showDrawer && isReady) ensures the drawer only appears when both conditions are met.
19-64: LGTM!Well-structured drawer implementation with:
- Clear visual hierarchy (icon + version + description)
- Defensive programming with optional chaining on
update?.version- User-friendly action choices (Later vs Restart now)
- Proper semantic HTML and styling
The overall implementation provides a clean UX for OTA update notifications.
ccf13fe to
7771365
Compare
7771365 to
bb20768
Compare
Overview
Changes