-
Notifications
You must be signed in to change notification settings - Fork 417
proper left-sidebar timeline rendering for new bundle #1568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughReplaces date-based grouping with labeled timeline buckets (Today/Yesterday/Tomorrow/etc.), adds typed timeline utilities and tests, refactors TimelineView to render buckets with Today-specific UI/scrolling, adds mod+1..mod+9 tab-select hotkey, and makes timezone() return "UTC" outside browsers. Changes
Sequence Diagram(s)sequenceDiagram
participant Data as buildTimelineBuckets
participant View as TimelineView
participant DOM as ScrollContainer
participant IO as IntersectionObserver
actor User
Note over Data: Aggregate events & sessions -> buckets[{label, sortKey, precision, items}]
User->>View: Open timeline
View->>Data: Request buckets
Data-->>View: Return sorted buckets
View->>DOM: Render buckets (sticky headers) & items
alt Today bucket present
View->>IO: Observe Today header
IO-->>View: Visibility updates
alt Today not visible
View->>User: Show "Go to Today" button
User->>View: Click button
View->>DOM: Smooth-scroll Today into view
IO-->>View: Today visible
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)**/*.{js,ts,tsx,rs}⚙️ CodeRabbit configuration file
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). (2)
🔇 Additional comments (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
299-326: Fix timezone mis-bucketing (avoid Date('yyyy‑MM‑dd') UTC parsing).item.date is stored as "yyyy-MM-dd" then parsed with new Date(), which is interpreted as UTC. This shifts days in non-UTC timezones, mislabeling Today/Yesterday/Tomorrow. Store the full ISO timestamp instead to preserve local day semantics when bucketing.
Apply this diff:
@@ - items.push({ - type: "event", - id: eventId, - date: format(eventStartTime, "yyyy-MM-dd"), - data: row as unknown as persisted.Event, - }); + items.push({ + type: "event", + id: eventId, + date: eventStartTime.toISOString(), + data: row as unknown as persisted.Event, + }); @@ - items.push({ - type: "session", - id: sessionId, - date: format(new Date(timestamp), "yyyy-MM-dd"), - data: row as unknown as persisted.Session, - }); + items.push({ + type: "session", + id: sessionId, + date: new Date(timestamp).toISOString(), + data: row as unknown as persisted.Session, + });No other changes needed; getBucketInfo will now receive correct local dates via new Date(item.date).
Also applies to: 335-343
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/components/main/sidebar/timeline.tsx
**/*.{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/components/main/sidebar/timeline.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
346-347: Confirm sort order: future buckets above Today.Buckets are sorted by sortKey descending, so future groups (Tomorrow, etc.) render above Today. If you want past→Today→future, adjust to place Today first, then past (desc), then future (asc/desc as desired).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
1-1: Replace clsx with cn utility as per coding guidelines.The coding guidelines require using
cnfrom@hypr/ui/lib/utilsinstead ofclsxfor Tailwind class composition in desktop2. This applies to lines 1 (import), 96-99, and 227-231 (usage).Based on coding guidelines.
🧹 Nitpick comments (2)
apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
42-63: Consider removingbucketsfrom observer effect dependencies.The IntersectionObserver watches only the Today section element's position, not bucket content. Re-creating the observer whenever
bucketschanges adds unnecessary overhead.Apply this diff:
- }, [buckets]); + }, []);However, if you expect the Today section ref to change when buckets update, you could explicitly depend on
todaySectionRef.currentor use a separate effect trigger. The current implementation works but may recreate observers more frequently than needed.
265-275: Extend week range to 28 days for accurate 4-week handling.The week range currently stops at 27 days, so a 28-day span (exactly 4 weeks) falls through to the month calculation and gets labeled "a month ago" instead of "4 weeks ago." While the month clamping (lines 277-280, 305-308) prevents "0 months" labels, extending the week range to 28 is more accurate.
Apply this diff:
- if (absDays <= 27) { + if (absDays <= 28) { const weeks = Math.max(1, Math.round(absDays / 7)); @@ - if (absDays <= 27) { + if (absDays <= 28) { const weeks = Math.max(1, Math.round(absDays / 7));Also applies to: 293-302
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/components/main/sidebar/timeline.tsx
**/*.{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/components/main/sidebar/timeline.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the cn utility imported as `import { cn } from "hypr/ui/lib/utils"`
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Split cn array entries by logical grouping when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Always pass an array to cn when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
🧬 Code graph analysis (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (2)
apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
201-220: LGTM! Clean precision-based time display.The displayTime logic correctly handles both precision modes: showing just time for "time" precision and date+time for "date" precision. Proper guards for invalid timestamps and appropriate memoization.
365-379: LGTM! Correct bucket aggregation and sorting.The bucket map construction properly groups items by label, preserves bucket metadata (sortKey, precision), and sorts buckets in descending order for most-recent-first display.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
1-1: Replace clsx with shared cn utility per desktop2 guidelines.Use cn([...]) for Tailwind composition and keep array entries.
Apply:
-import { clsx } from "clsx"; +import { cn } from "@hypr/ui/lib/utils"; @@ - className={clsx([ + className={cn([ "absolute left-1/2 transform -translate-x-1/2 rounded-full shadow-lg bg-white hover:bg-gray-50 text-gray-700 border border-gray-200 z-40 flex items-center gap-1", isScrolledPastToday ? "top-2" : "bottom-2", ])} @@ - className={clsx([ + className={cn([ "w-full text-left px-3 py-2 rounded-lg", active && "bg-gray-200", !active && "hover:bg-gray-100", ])}As per coding guidelines
Also applies to: 64-67, 267-271
🧹 Nitpick comments (2)
apps/desktop2/src/utils/timeline.ts (1)
59-69: Extend week range to 28 days and clamp months via Math.max.Prevents labels like “a month ago/in 1 month” for 28-day spans and removes 0-month edge cases cleanly.
Apply:
- if (absDays <= 27) { + if (absDays <= 28) { @@ - let months = Math.abs(differenceInCalendarMonths(targetDay, now)); - if (months === 0) { - months = 1; - } + const months = Math.max(1, Math.abs(differenceInCalendarMonths(targetDay, now))); @@ - if (absDays <= 27) { + if (absDays <= 28) { @@ - let months = differenceInCalendarMonths(targetDay, now); - if (months === 0) { - months = 1; - } + const months = Math.max(1, differenceInCalendarMonths(targetDay, now));As per coding guidelines
Also applies to: 87-97, 71-81, 99-109
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
239-260: Optional: unify display time formatting with shared formatter.For consistent TZ-aware display, consider using the shared format helper instead of toLocale*.
Example:
// import { format } from "@hypr/utils/src/datetime"; const time = format(date, "p"); const dateStr = format(date, date.getFullYear() === new Date().getFullYear() ? "MMM d" : "MMM d, yyyy"); return precision === "time" ? time : `${dateStr}, ${time}`;Based on learnings
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop2/src/components/main/body/index.tsx(2 hunks)apps/desktop2/src/components/main/sidebar/timeline.tsx(4 hunks)apps/desktop2/src/utils/timeline.test.ts(1 hunks)apps/desktop2/src/utils/timeline.ts(1 hunks)
🧰 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/utils/timeline.tsapps/desktop2/src/components/main/body/index.tsxapps/desktop2/src/utils/timeline.test.tsapps/desktop2/src/components/main/sidebar/timeline.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/components/main/body/index.tsxapps/desktop2/src/components/main/sidebar/timeline.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : When there are many Tailwind classNames with conditional logic, use the cn utility imported as `import { cn } from "hypr/ui/lib/utils"`
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Split cn array entries by logical grouping when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
📚 Learning: 2025-10-12T11:14:50.454Z
Learnt from: CR
PR: fastrepl/hyprnote#0
File: apps/desktop2/.cursor/rules/style.mdc:0-0
Timestamp: 2025-10-12T11:14:50.454Z
Learning: Applies to apps/desktop2/**/*.{tsx,jsx} : Always pass an array to cn when composing Tailwind classNames
Applied to files:
apps/desktop2/src/components/main/sidebar/timeline.tsx
🧬 Code graph analysis (4)
apps/desktop2/src/utils/timeline.ts (1)
packages/utils/src/datetime.ts (1)
format(4-11)
apps/desktop2/src/components/main/body/index.tsx (1)
apps/desktop2/src/store/zustand/tabs.ts (1)
useTabs(99-271)
apps/desktop2/src/utils/timeline.test.ts (1)
apps/desktop2/src/utils/timeline.ts (4)
getBucketInfo(35-109)buildTimelineBuckets(111-182)EventsWithoutSessionTable(20-20)SessionsWithMaybeEventTable(21-21)
apps/desktop2/src/components/main/sidebar/timeline.tsx (2)
packages/ui/src/components/ui/button.tsx (1)
Button(37-89)apps/desktop2/src/utils/timeline.ts (4)
TimelineBucket(29-33)TimelineItem(23-25)TimelinePrecision(27-27)buildTimelineBuckets(111-182)
⏰ 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). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (2)
apps/desktop2/src/components/main/body/index.tsx (1)
254-277: Hotkeys for tab selection look solid.Simple, guarded, and consistent with close-hotkey behavior. LGTM.
apps/desktop2/src/components/main/sidebar/timeline.tsx (1)
299-314: Today indicator position depends on item sort; ensure order matches timestamp.Indicator assumes entries are sorted by getItemTimestamp. This will be correct once util sorting uses event_started_at || created_at for sessions (see utils/timeline.ts). Verify after applying that change.
Also applies to: 328-336
No description provided.