26 improve studies#61
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a Zag.js-powered Select component and dependency, UI animations and Dialog scrolling tweaks, a new EditStudyModal for editing study metadata and reviewers, integrates the modal into AllStudiesTab, and documents the Select usage and guidance. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AllStudiesTab
participant EditStudyModal
participant ProjectStore
User->>AllStudiesTab: Click "Edit" on a study
AllStudiesTab->>EditStudyModal: openModal(study)
EditStudyModal->>EditStudyModal: populate form from study props
User->>EditStudyModal: modify fields & select reviewers
User->>EditStudyModal: Click "Save Changes"
EditStudyModal->>EditStudyModal: validate & construct updates
EditStudyModal->>ProjectStore: onUpdateStudy(studyId, updates)
ProjectStore-->>EditStudyModal: update result (success/fail)
EditStudyModal-->>AllStudiesTab: closeModal() / callback
AllStudiesTab->>User: reflect updated study in list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
corates | 65a4485 | Commit Preview URL | Dec 16 2025, 05:12 AM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/web/src/components/zag/Collapsible.jsx (1)
40-67: Consider extracting animations to a shared stylesheet.Inline
<style>tags render on every component instance, duplicating these keyframe definitions in the DOM. While functionally correct, moving these animations to a global CSS file (e.g.,app.cssor a Zag-specific stylesheet) would reduce DOM bloat and improve maintainability.- <style>{` - .collapsible-content[data-state="open"] { - animation: collapsible-slideDown 200ms ease-out; - } - .collapsible-content[data-state="closed"] { - animation: collapsible-slideUp 200ms ease-out; - } - @keyframes collapsible-slideDown { - from { - opacity: 0; - height: 0; - } - to { - opacity: 1; - height: var(--height); - } - } - @keyframes collapsible-slideUp { - from { - opacity: 1; - height: var(--height); - } - to { - opacity: 0; - height: 0; - } - } - `}</style>Then add these styles to a global stylesheet that's loaded once.
packages/web/src/components/zag/Select.jsx (1)
22-22: Consider reducing prop count or grouping configuration.The component accepts 10 props, which exceeds the guideline of 1-5 props. While these are configuration props rather than shared state, consider grouping related props into configuration objects:
For example:
// Option 1: Group validation props const validation = { invalid: boolean, name: string } // Option 2: Group state props const config = { disabled, placeholder, inDialog, disabledValues }However, given this is a reusable Zag wrapper for a complex UI component, the current API may be acceptable if it improves usability.
As per coding guidelines, components should receive at most 1-5 props.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/copilot-instructions.md(1 hunks)packages/web/package.json(1 hunks)packages/web/src/components/auth-ui/CheckEmail.jsx(0 hunks)packages/web/src/components/project-ui/EditStudyModal.jsx(1 hunks)packages/web/src/components/project-ui/tabs/AllStudiesTab.jsx(5 hunks)packages/web/src/components/zag/Collapsible.jsx(1 hunks)packages/web/src/components/zag/Dialog.jsx(1 hunks)packages/web/src/components/zag/README.md(1 hunks)packages/web/src/components/zag/Select.jsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/web/src/components/auth-ui/CheckEmail.jsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
Files:
packages/web/package.jsonpackages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/README.mdpackages/web/src/components/zag/Dialog.jsx
packages/web/src/components/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
For UI icons, use the
solid-iconslibrary or SVGs only. Do not use emojis
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
packages/web/src/components/**/*.{jsx,tsx}: Use responsive design principles for UI components
When implementing UI components, use zag.js frompackages/web/src/components/zag/*and reuse existing components
Do NOT prop-drill application state in SolidJS components - shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file
Components should receive at most 1-5 props, and only for local configuration, not shared state - if a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context
In SolidJS components, do not destructure props as it breaks reactivity - instead access props directly from the props object, or wrap them in a function to ensure they are always up-to-date
Components should be lean and focused - do not implement business logic in components; move that into stores, utilities, or primitives
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
packages/web/src/components/**
📄 CodeRabbit inference engine (.cursorrules)
Group related components in subdirectories with an
index.jsbarrel export
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/README.mdpackages/web/src/components/zag/Dialog.jsx
packages/web/src/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
packages/web/src/**/*.{jsx,tsx,js,ts}: When you need to compute a value based on props or state in SolidJS, usecreateMemoto ensure it updates reactively
For complex state or state objects in SolidJS, use Solid'screateStorefor better performance and reactivity
Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
packages/web/src/**/*.{jsx,tsx,js,ts}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis
Use Solid'screateStorefor complex state or state objects for better performance and reactivity
Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Use responsive design principles for UI components
When implementing UI components, use zag.js. Reuse Zag components frompackages/web/src/components/zag/*
Do NOT prop-drill application state. Shared or cross-feature state must live in external stores under packages/web/src/stores/ or relative to the component file
Components should receive at most 1–5 props, and only for local configuration, not shared state
In SolidJS, do not destructure props as it breaks reactivity. Instead, access props directly from the props object or wrap them in a function
UsecreateMemoto compute values based on props or state to ensure they update reactively in SolidJS
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Zod for schema and input validation
Files:
packages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/project-ui/EditStudyModal.jsxpackages/web/src/components/project-ui/tabs/AllStudiesTab.jsxpackages/web/src/components/zag/Dialog.jsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : When implementing UI components, use zag.js. Reuse Zag components from `packages/web/src/components/zag/*`
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from `packages/web/src/components/zag/*` and reuse existing components
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from `packages/web/src/components/zag/*` and reuse existing components
Applied to files:
packages/web/package.jsonpackages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/zag/README.md.github/copilot-instructions.mdpackages/web/src/components/zag/Dialog.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : When implementing UI components, use zag.js. Reuse Zag components from `packages/web/src/components/zag/*`
Applied to files:
packages/web/package.jsonpackages/web/src/components/zag/Collapsible.jsxpackages/web/src/components/zag/Select.jsxpackages/web/src/components/zag/README.md.github/copilot-instructions.mdpackages/web/src/components/zag/Dialog.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : In SolidJS components, do not destructure props as it breaks reactivity - instead access props directly from the props object, or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Use Solid's `createStore` for complex state or state objects for better performance and reactivity
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx,js,ts} : For UI icons, use the `solid-icons` library or SVGs only. Do not use emojis
Applied to files:
packages/web/src/components/project-ui/tabs/AllStudiesTab.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Keep files small, focused, and modular. Extract sub-modules into a folder (e.g., `ComponentName/` with `index.jsx` and helper components). Move complex logic into separate utility files or primitives. Split large forms into section components
Applied to files:
packages/web/src/components/zag/README.md
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use responsive design principles for UI components
Applied to files:
packages/web/src/components/zag/README.mdpackages/web/src/components/zag/Dialog.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use responsive design principles for UI components
Applied to files:
packages/web/src/components/zag/README.mdpackages/web/src/components/zag/Dialog.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
Applied to files:
packages/web/src/components/zag/README.md
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Use the CORATES MCP tools to explore local documentation sources for Better-Auth, Drizzle, Icons, and Zag documentation
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/workers/**/*.{ts,sql} : Use Drizzle ORM for database interactions and migrations
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/workers/**/*.{ts,tsx,js,jsx} : Use Better-Auth for authentication and user management
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Zod for schema and input validation
Applied to files:
.github/copilot-instructions.md
🧬 Code graph analysis (2)
packages/web/src/components/project-ui/EditStudyModal.jsx (3)
packages/web/src/components/project-ui/ReviewerAssignment.jsx (1)
getMemberName(44-48)packages/web/src/components/zag/Collapsible.jsx (2)
Collapsible(15-70)api(32-32)packages/web/src/components/zag/Select.jsx (2)
api(52-52)Select(22-160)
packages/web/src/components/project-ui/tabs/AllStudiesTab.jsx (1)
packages/web/src/components/project-ui/EditStudyModal.jsx (1)
EditStudyModal(19-239)
⏰ 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). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (12)
packages/web/src/components/zag/Dialog.jsx (1)
56-64: LGTM! Scrollable dialog positioner improves UX for long content.The combination of
overflow-y-autoon the positioner andmy-autoon the content ensures dialogs with tall content remain usable while staying centered when shorter. This is a solid improvement for modals likeEditStudyModalthat may contain expandable sections.packages/web/package.json (1)
27-27: LGTM! Dependency version aligns with existing Zag packages.The
@zag-js/selectversion^1.31.1is consistent with other@zag-js/*packages in the project, ensuring compatibility.packages/web/src/components/zag/README.md (1)
12-12: LGTM! Documentation accurately describes the new Select component.The props list and the
inDialogusage note are helpful for developers integrating the Select component within modals..github/copilot-instructions.md (1)
29-29: LGTM! Enhanced guidance promotes Zag component reuse.Adding Zag.js to the Libraries section and emphasizing checking existing components before adding new ones aligns well with the project's component reuse philosophy. Based on learnings, this reinforces the established pattern of reusing components from
packages/web/src/components/zag/*.Also applies to: 37-37
packages/web/src/components/project-ui/tabs/AllStudiesTab.jsx (3)
31-32: LGTM! Modal state management follows established patterns.The
showEditModalandeditingStudysignals are appropriately scoped as local UI state, and the handlers correctly manage the modal lifecycle. The pattern mirrors the existingGoogleDrivePickerModalintegration.Also applies to: 81-91
363-370: LGTM! Edit button uses appropriate icon from solid-icons.The
FiSettingsicon fromsolid-icons/fifollows the coding guidelines for icon usage. The button styling is consistent with adjacent action buttons.
399-406: LGTM! EditStudyModal integration is well-wired.The modal receives exactly 5 props (within the guideline limit) and correctly delegates updates through the existing
handlers.studyHandlers.handleUpdateStudyflow.packages/web/src/components/project-ui/EditStudyModal.jsx (4)
1-8: LGTM! Imports correctly use Zag ecosystem components.The component properly imports from
@components/zag/*for Dialog, Select, Collapsible, and Toast, adhering to the project's Zag.js component reuse guidelines. Based on learnings, this follows the established pattern.
30-44: LGTM! Store access and derived state follow best practices.Accessing
projectStoredirectly avoids prop-drilling, andmemberItemsusescreateMemofor reactive derived data. As per coding guidelines, this keeps the component lean while maintaining reactivity.
191-208: LGTM! Select components correctly use inDialog prop.Setting
inDialogtotrueensures the Select dropdown doesn't use a Portal, avoiding focus trap conflicts within the Dialog. The mutual exclusion viadisabledValuesprevents assigning the same reviewer twice.
47-59: Verify effect doesn't cause stale closures.The
createEffectdepends onprops.studyandprops.open. In SolidJS, accessingprops.studyinside the effect is reactive, which is correct. Just ensure that ifstudyisnullwhen closing the modal, the form isn't unexpectedly reset. Currently guarded byif (study && props.open), which is appropriate.packages/web/src/components/zag/Select.jsx (1)
1-6: LGTM!Imports are well-organized and follow coding guidelines by using
solid-iconsfor UI icons.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/components/zag/Select.jsx (2)
77-102: Consider memoizing item disabled state for better performance.The
isValueDisabled(item.value)function is called multiple times per item during rendering (lines 90, 92), which performs array lookups each time. For large item lists, this could impact performance.Consider memoizing the disabled state per item:
const renderList = () => ( <ul {...api().getContentProps()} class='...'> <For each={items()}> {item => { + const itemDisabled = () => isValueDisabled(item.value); return ( <li {...api().getItemProps({ item })} class={` px-3 py-2 cursor-pointer flex items-center justify-between whitespace-nowrap hover:bg-gray-100 data-highlighted:bg-blue-50 - ${isValueDisabled(item.value) ? 'text-gray-400 cursor-not-allowed hover:bg-transparent' : 'text-gray-900'} + ${itemDisabled() ? 'text-gray-400 cursor-not-allowed hover:bg-transparent' : 'text-gray-900'} `} - data-disabled={isValueDisabled(item.value) || undefined} + data-disabled={itemDisabled() || undefined} > <span {...api().getItemTextProps({ item })}>{item.label}</span> <Show when={api().value.includes(item.value)}> <BiRegularCheck class='w-5 h-5 text-blue-600' /> </Show> </li> + ); + }} - )} </For> </ul> );This ensures
isValueDisabledis only called once per item per render cycle.
22-22: Note: Component has 10 props, exceeding the 1-5 guideline.The coding guidelines suggest components should receive at most 1-5 props for local configuration. This component accepts 10 props (items, value, onChange, label, placeholder, disabled, name, invalid, disabledValues, inDialog).
However, since these are all valid configuration options for a reusable Select component (not prop-drilled application state), this may be acceptable. If the prop count becomes unwieldy, consider grouping related props into objects (e.g.,
validation={{ invalid: boolean }},styling={{ label, placeholder }}).As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web/src/components/project-ui/EditStudyModal.jsx(1 hunks)packages/web/src/components/zag/Select.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/components/project-ui/EditStudyModal.jsx
🧰 Additional context used
📓 Path-based instructions (9)
packages/web/src/components/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
For UI icons, use the
solid-iconslibrary or SVGs only. Do not use emojis
Files:
packages/web/src/components/zag/Select.jsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Files:
packages/web/src/components/zag/Select.jsx
packages/web/src/components/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
packages/web/src/components/**/*.{jsx,tsx}: Use responsive design principles for UI components
When implementing UI components, use zag.js frompackages/web/src/components/zag/*and reuse existing components
Do NOT prop-drill application state in SolidJS components - shared or cross-feature state must live in external stores underpackages/web/src/stores/or relative to the component file
Components should receive at most 1-5 props, and only for local configuration, not shared state - if a component would need more than 5 props, move the shared data into an external store, a primitive, or Solid context
In SolidJS components, do not destructure props as it breaks reactivity - instead access props directly from the props object, or wrap them in a function to ensure they are always up-to-date
Components should be lean and focused - do not implement business logic in components; move that into stores, utilities, or primitives
Files:
packages/web/src/components/zag/Select.jsx
packages/web/src/components/**
📄 CodeRabbit inference engine (.cursorrules)
Group related components in subdirectories with an
index.jsbarrel export
Files:
packages/web/src/components/zag/Select.jsx
packages/web/src/**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.cursorrules)
packages/web/src/**/*.{jsx,tsx,js,ts}: When you need to compute a value based on props or state in SolidJS, usecreateMemoto ensure it updates reactively
For complex state or state objects in SolidJS, use Solid'screateStorefor better performance and reactivity
Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
packages/web/src/**/*.{jsx,tsx,js,ts}: For UI icons, use thesolid-iconslibrary or SVGs only. Do not use emojis
Use Solid'screateStorefor complex state or state objects for better performance and reactivity
Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
Files:
packages/web/src/components/zag/Select.jsx
**/*
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use emojis in code, comments, documentation, or commit messages
Files:
packages/web/src/components/zag/Select.jsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Files:
packages/web/src/components/zag/Select.jsx
packages/web/src/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/web/src/**/*.{jsx,tsx}: Use responsive design principles for UI components
When implementing UI components, use zag.js. Reuse Zag components frompackages/web/src/components/zag/*
Do NOT prop-drill application state. Shared or cross-feature state must live in external stores under packages/web/src/stores/ or relative to the component file
Components should receive at most 1–5 props, and only for local configuration, not shared state
In SolidJS, do not destructure props as it breaks reactivity. Instead, access props directly from the props object or wrap them in a function
UsecreateMemoto compute values based on props or state to ensure they update reactively in SolidJS
Components should be lean and focused. Do not implement business logic; move that into stores, utilities, or primitives
Files:
packages/web/src/components/zag/Select.jsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Zod for schema and input validation
Files:
packages/web/src/components/zag/Select.jsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : When implementing UI components, use zag.js. Reuse Zag components from `packages/web/src/components/zag/*`
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from `packages/web/src/components/zag/*` and reuse existing components
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : When implementing UI components, use zag.js from `packages/web/src/components/zag/*` and reuse existing components
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : When implementing UI components, use zag.js. Reuse Zag components from `packages/web/src/components/zag/*`
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : In SolidJS components, do not destructure props as it breaks reactivity - instead access props directly from the props object, or wrap them in a function to ensure they are always up-to-date
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Use Solid's `createStore` for complex state or state objects for better performance and reactivity
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx} : Use `createMemo` to compute values based on props or state to ensure they update reactively in SolidJS
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : When you need to compute a value based on props or state in SolidJS, use `createMemo` to ensure it updates reactively
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T00:29:26.670Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-16T00:29:26.670Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : For complex state or state objects in SolidJS, use Solid's `createStore` for better performance and reactivity
Applied to files:
packages/web/src/components/zag/Select.jsx
📚 Learning: 2025-12-16T03:19:31.899Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-16T03:19:31.899Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,js,ts} : Create reusable logic in 'primitives' (hooks) that can be shared across components to keep components clean and focused on rendering
Applied to files:
packages/web/src/components/zag/Select.jsx
🧬 Code graph analysis (1)
packages/web/src/components/zag/Select.jsx (3)
packages/web/src/components/zag/Collapsible.jsx (3)
disabled(18-18)service(22-30)api(32-32)packages/web/src/components/zag/FileUpload.jsx (1)
item(59-59)packages/web/src/components/zag/Dialog.jsx (4)
service(25-31)service(117-130)api(33-33)api(132-132)
⏰ 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). (1)
- GitHub Check: Workers Builds: corates
🔇 Additional comments (4)
packages/web/src/components/zag/Select.jsx (4)
1-21: LGTM! Clean imports and documentation.The imports follow best practices by using
solid-iconsfor icons (no emojis), and the JSDoc provides clear documentation of the component's API.
22-34: LGTM! Correct reactive prop access pattern.Props are properly accessed via functions rather than destructured, which preserves reactivity in SolidJS. The
isValueDisabledhelper correctly calls the reactiveitems()anddisabledValues()functions.
36-42: LGTM! Proper use of createMemo.The collection is correctly computed using
createMemo, ensuring it updates reactively whenitems()changes.
68-75: LGTM! Proper memoization of derived values.Both
apiandselectedLabelcorrectly usecreateMemoto ensure they update reactively based on their dependencies.
| const service = useMachine(select.machine, () => { | ||
| // Capture disabledValues here so it's tracked as a dependency | ||
| const currentDisabledValues = disabledValues(); | ||
| const currentItems = items(); | ||
|
|
||
| return { | ||
| id: createUniqueId(), | ||
| collection: collection(), | ||
| value: value() ? [value()] : [], | ||
| disabled: disabled(), | ||
| name: props.name, | ||
| invalid: props.invalid, | ||
| onValueChange(details) { | ||
| const newValue = details.value[0] || ''; | ||
| // Prevent selecting disabled values | ||
| const item = currentItems.find(i => i.value === newValue); | ||
| if (item?.disabled || currentDisabledValues.includes(newValue)) { | ||
| return; | ||
| } | ||
| props.onChange?.(newValue); | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Dialog.jsx to see the getter pattern mentioned
find . -name "Dialog.jsx" -type f | head -5Repository: InfinityBowman/corates
Length of output: 110
🏁 Script executed:
# Examine Select.jsx to understand full context
find . -name "Select.jsx" -path "*/components/zag/*" -type fRepository: InfinityBowman/corates
Length of output: 110
🏁 Script executed:
# Read Dialog.jsx to see the getter pattern mentioned
cat -n ./packages/web/src/components/zag/Dialog.jsxRepository: InfinityBowman/corates
Length of output: 12248
🏁 Script executed:
# Read full Select.jsx to understand the complete implementation
cat -n ./packages/web/src/components/zag/Select.jsxRepository: InfinityBowman/corates
Length of output: 6522
Use getter accessors instead of factory function for reactive prop updates.
The Select component should follow the same pattern as Dialog.jsx (lines 25-31) by replacing the factory function with an object containing get accessors. The current factory function captures currentDisabledValues and currentItems at initialization time; if those props change, the captured values become stale and the machine won't reflect the updates.
Change from:
const service = useMachine(select.machine, () => {
const currentDisabledValues = disabledValues();
const currentItems = items();
return {
// ...
onValueChange(details) {
// uses stale currentItems and currentDisabledValues
},
};
});To:
const service = useMachine(select.machine, {
id: createUniqueId(),
collection: collection(),
get value() {
return value() ? [value()] : [];
},
get disabled() {
return disabled();
},
name: props.name,
invalid: props.invalid,
get onValueChange() {
return details => {
const newValue = details.value[0] || '';
const item = items().find(i => i.value === newValue);
if (item?.disabled || disabledValues().includes(newValue)) {
return;
}
props.onChange?.(newValue);
};
},
});This ensures the machine reads fresh values each time, maintaining controlled component behavior when parent props change.
🤖 Prompt for AI Agents
packages/web/src/components/zag/Select.jsx lines 44-66: the factory function
passed to useMachine captures disabledValues() and items() once, causing stale
data when props change; replace the factory callback with an object that exposes
getters for value, disabled, and onValueChange (and other reactive fields like
collection) so the machine reads fresh values each access — implement get
value() { return value() ? [value()] : []; }, get disabled() { return
disabled(); }, and get onValueChange() { return details => { const newValue =
details.value[0] || ''; const item = items().find(i => i.value === newValue); if
(item?.disabled || disabledValues().includes(newValue)) return;
props.onChange?.(newValue); }; } and include id, collection, name, invalid as
static properties in the same object when calling useMachine.
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.