-
Notifications
You must be signed in to change notification settings - Fork 432
feat: add calendar extension with store and tabs API exposure #1963
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
- Update extension-globals.ts to expose TinyBase store and Zustand tabs store - Update build.mjs to handle @hypr/store and @hypr/tabs externals - Add additional UI components to extension globals (ButtonGroup, Checkbox, Popover) - Create calendar extension with full calendar view functionality - Extension can access calendars, events, and sessions from the main store - Extension can navigate to sessions using the tabs store Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ 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. |
📝 WalkthroughWalkthroughAdds a Calendar extension (manifest, runtime hooks, UI) and exposes additional host UI components and globals to extensions by registering ButtonGroup/Checkbox/Popover and window.__hypr_store / window.__hypr_tabs, plus build-time externals for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (10)
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
🧹 Nitpick comments (3)
extensions/calendar/ui.tsx (2)
26-34: UnusedextensionIdprop.The
extensionIdprop is declared inExtensionViewPropsand destructured in the component but never used. Consider removing it if not needed, or document its intended future use.export interface ExtensionViewProps { - extensionId: string; state?: Record<string, unknown>; } export default function CalendarExtensionView({ - extensionId, state, }: ExtensionViewProps) {
486-607: Consider extracting icons to a shared location if reused.The inline SVG icons work fine for this extension. If these icons (CalendarIcon, ChevronLeftIcon, etc.) are needed elsewhere, consider importing from a shared icon library or
@hypr/uiinstead of duplicating.extensions/calendar/main.js (1)
13-19: Metadata duplication with manifest; consider deriving from context.The
getInforeturn values duplicate and slightly differ fromextension.json(name is "Calendar Extension" here vs "Calendar" in manifest). Ifactivatereceives the manifest via context, consider storing it and returning fromgetInfoto avoid drift.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/extension-globals.ts(2 hunks)extensions/build.mjs(2 hunks)extensions/calendar/extension.json(1 hunks)extensions/calendar/main.js(1 hunks)extensions/calendar/ui.tsx(1 hunks)extensions/package.json(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/extension-globals.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/extension-globals.tsextensions/calendar/ui.tsx
🧬 Code graph analysis (1)
apps/desktop/src/extension-globals.ts (1)
apps/desktop/src/store/zustand/tabs/index.ts (1)
useTabs(29-38)
⏰ 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: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
🔇 Additional comments (11)
extensions/package.json (1)
9-9: LGTM!The build script follows the established pattern used by other extensions (e.g.,
build:hello-world).extensions/calendar/ui.tsx (4)
242-249: Store instance access during render.The filtering logic uses
storeInstance?.getRowduring render, which works but couples the component to synchronous store access. This is acceptable for read-only filtering, but be aware that if the store becomes async or the instance is null, all events will be filtered out silently.
345-359: Creating session with random UUID may need backend coordination.When no linked session exists,
handleOpenNotecreates a new tab withcrypto.randomUUID(). Ensure the tab system or backend properly handles creating a new session entity for this ID, otherwise this may result in orphan references or errors.Also, the
"dummy"fallback foruseRowis a known pattern workaround but could be cleaner with conditional hook execution if supported.
417-440: LGTM!The component correctly prioritizes displaying the event title when a session is linked to an event, with appropriate fallbacks.
162-169: Good use ofcnutility with array syntax.The className logic correctly uses the
cnutility with array syntax as specified in the coding guidelines.extensions/calendar/extension.json (1)
1-16: LGTM!The extension manifest is well-structured with appropriate metadata. The panel entry correctly references the built output at
dist/ui.js.apps/desktop/src/extension-globals.ts (3)
6-13: LGTM!The new imports for UI components and store/tabs modules align with the extension requirements. The pattern follows existing conventions in the file.
22-23: LGTM!The type declarations correctly match the runtime assignments and provide proper typing for extension consumers.
33-42: LGTM!The global assignments correctly expose the new UI components and state management hooks for extension consumption. The pattern is consistent with existing globals.
extensions/build.mjs (2)
182-193: LGTM! Implementation is consistent with existing external handlers.The onLoad handlers for
@hypr/storeand@hypr/tabscorrectly follow the established pattern used for React and other externals. The module exports resolve to the appropriate window globals.
132-139: Perfect! I have verified all the necessary information. The globals are properly initialized early in the application lifecycle, well before extensions would be loaded.Verification complete – globals are properly initialized.
The
@hypr/storeand@hypr/tabsglobals are defined inapps/desktop/src/extension-globals.tsand initialized viainitExtensionGlobals()called at line 90 ofmain.tsx, which executes before React renders the application. This ensures the globals are available to extensions when they load. The implementation correctly follows the established pattern for external module resolution used by existing externals like@hypr/reactand@hypr/ui.
…darIds - Fix main.js to use context.manifest.api_version (snake_case) instead of apiVersion - Add useEffect to sync selectedCalendars when calendarIds changes, preserving existing selections while adding new calendars Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
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
🧹 Nitpick comments (6)
extensions/calendar/main.js (1)
13-19: Consider derivinggetInfofrom the manifest to avoid duplication.The hardcoded values in
getInfoduplicate the manifest data inextension.json. If the manifest is updated, this function will drift out of sync.You could return the manifest data passed during activation instead:
+let cachedContext = null; + __hypr_extension.activate = function (context) { + cachedContext = context; hypr.log.info( `Activating ${context.manifest.name} v${context.manifest.version}`, ); // ... }; __hypr_extension.getInfo = function () { + if (cachedContext) { + return { + name: cachedContext.manifest.name, + version: cachedContext.manifest.version, + description: cachedContext.manifest.description, + }; + } return { name: "Calendar Extension", version: "0.1.0", description: "Calendar view extension for viewing events and sessions", }; };extensions/calendar/ui.tsx (5)
31-34: UnusedextensionIdprop.The
extensionIdparameter is declared but never used within the component. If it's reserved for future use, consider adding a comment; otherwise, remove it from the props.
49-59: Return original reference when no calendars were added.The effect always returns a new
Setobject even when no IDs were added, which triggers unnecessary re-renders. Returnprevwhen unchanged:useEffect(() => { setSelectedCalendars((prev) => { const next = new Set(prev); + let changed = false; for (const id of calendarIds) { if (!prev.has(id)) { next.add(id); + changed = true; } } - return next; + return changed ? next : prev; }); }, [calendarIds]);
208-233: Duplicate ofCalendarCheckboxRowfrom main app.This component duplicates
apps/desktop/src/components/main/body/calendars/calendar-checkbox-row.tsx. Consider extracting to a shared location or importing if the extension build supports it.
357-361: Avoid "dummy" row lookup whenlinkedSessionIdis falsy.Using
"dummy"as a fallback triggers an unnecessary store lookup for a non-existent row. Since hooks can't be conditional, consider returning early or memoizing:const linkedSessionId = sessionIds[0]; -const linkedSession = store.UI.useRow( - "sessions", - linkedSessionId || "dummy", - store.STORE_ID, -); +const linkedSession = store.UI.useRow( + "sessions", + linkedSessionId ?? "", + store.STORE_ID, +);If the store gracefully handles empty string lookups (returns
undefined), this is cleaner. Otherwise, iflinkedSessionIdis commonly absent, consider restructuring to avoid the lookup entirely.
498-619: Consider importing icons from@hypr/uior a shared icon library.Six inline SVG icon components (~120 lines) could likely be replaced with imports from
@hypr/uior a shared icon package (e.g.,lucide-react), reducing duplication and bundle size.#!/bin/bash # Check if similar icons exist in the UI package or if lucide-react is available rg -l "CalendarIcon|ChevronLeftIcon|StickyNoteIcon" packages/ui/ fd "icon" packages/ui --type f | head -10 rg '"lucide-react"' package.json packages/*/package.json
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
extensions/calendar/main.js(1 hunks)extensions/calendar/ui.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
extensions/calendar/ui.tsx
🧬 Code graph analysis (1)
extensions/calendar/ui.tsx (3)
apps/desktop/src/components/main/body/calendars/calendar-checkbox-row.tsx (1)
CalendarCheckboxRow(5-30)packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/store/zustand/tabs/index.ts (1)
useTabs(29-38)
⏰ 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: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
🔇 Additional comments (3)
extensions/calendar/main.js (1)
1-11: LGTM!The lifecycle hooks are correctly implemented. The
api_versionproperty access uses the correct snake_case format matching the runtime manifest structure.extensions/calendar/ui.tsx (2)
170-182: LGTM oncnusage.The
cnutility is correctly used with array syntax and logical groupings per coding guidelines.
433-434: Verify behavior whenevent_idis empty.When
session?.event_idis falsy,eventIdbecomes an empty string, anduseRowfetches with"". Confirm the store handles this gracefully without errors or performance impact.
Resolved conflicts by keeping both: - My changes: @hypr/store and @hypr/tabs externals - Upstream changes: tinybase/ui-react external Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Summary
This PR extracts the calendar view functionality into a standalone extension by:
extensions/calendarextension with the full calendar view functionality@hypr/storeand@hypr/tabsas external dependenciesThe calendar extension can now access calendars, events, and sessions from the main store, and navigate to sessions using the tabs store.
Updates since last revision
main.jsto usecontext.manifest.api_version(snake_case) instead ofapiVersion(camelCase) to correctly log the API versionuseEffectto syncselectedCalendarswhencalendarIdschanges, preserving existing selections while automatically selecting newly added calendars@hypr/store/@hypr/tabsexternals and upstream'stinybase/ui-reactexternal)Review & Testing Checklist for Human
This is a high-risk change due to lack of end-to-end testing and new API surface exposure:
useTabs.openNew()__hypr_storeand__hypr_tabsas window globals is the desired pattern for extension data accessstore.INDEXES.eventsByDate,store.INDEXES.sessionByDateWithoutEvent, andstore.INDEXES.sessionsByEvent- confirm these are defined in the main storeRecommended test plan:
cd extensions && node build.mjs build calendarnode build.mjs install calendarONBOARDING=0 pnpm -F desktop tauri devNotes
apps/desktop/src/components/main/body/calendarscomponentLink to Devin run: https://app.devin.ai/sessions/19c73ebd08444ff198ca9f9e74b90285
Requested by: yujonglee (yujonglee.dev@gmail.com) / @yujonglee