Skip to content

add ark ui and migrate most components#135

Merged
InfinityBowman merged 5 commits into
mainfrom
133-migrate-to-ark-ui
Dec 24, 2025
Merged

add ark ui and migrate most components#135
InfinityBowman merged 5 commits into
mainfrom
133-migrate-to-ark-ui

Conversation

@InfinityBowman
Copy link
Copy Markdown
Owner

@InfinityBowman InfinityBowman commented Dec 24, 2025

Summary by CodeRabbit

  • New Features

    • Added Ark UI as the primary UI component library and a new MCP server entry for Ark tooling.
    • Introduced a modular PDF subsystem (loading, file handling, rendering, scrolling) for improved PDF viewing.
  • Bug Fixes

    • Broadened PDF uploader acceptance to recognize .pdf extensions.
  • Documentation

    • Updated docs, diagrams and README to reference Ark UI and added a comprehensive "API Actions" diagram.
  • Chores

    • Replaced legacy UI dependencies with Ark UI, updated build/test config and refreshed many component implementations.

✏️ Tip: You can customize this high-level summary in your review settings.

@InfinityBowman InfinityBowman linked an issue Dec 24, 2025 that may be closed by this pull request
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Dec 24, 2025

📝 Walkthrough

Walkthrough

Replace Zag.js with Ark UI across the UI package: migrate many component implementations to Ark UI primitives, adjust Collapsible types/API (onOpenChange now receives { open } and useCollapsible added), refactor the PDF viewer into modular pieces (pdfDocument, pdfFileHandler, pdfRenderer, pdfScrollHandler), update docs/diagrams, add Ark UI MCP entry, and change package dependencies. No backend/API changes.

Changes

Cohort / File(s) Summary
MCP Configuration
\.cursor/mcp\.json
Added ark-ui mcpServers entry running npx -y @ark-ui/mcp``.
Docs & Guidance
\.cursor/rules/corates\.mdc, \.cursor/rules/ui-components\.mdc, README\.md, packages/mcp/README.md, packages/ui/README.md
Replaced Zag.js references with Ark UI, updated examples/imports, migration notes and MCP guidance.
Architecture docs & site
docs/architecture/diagrams\.md, docs/architecture/diagrams/01-package-architecture.md, docs/architecture/diagrams/02-system-architecture.md, docs/architecture/diagrams/07-api-actions.md, docs/architecture/index\.html
Added "API Actions" doc (detailed diagrams), adjusted diagrams table/formatting, widened layout and mermaid config for larger diagrams and responsive SVG handling.
Root package metadata
package\.json, packages/ui/package\.json
Added dotenv to devDependencies; removed many @zag-js/* deps and added @ark-ui/solid@^5.30.0; updated ui package description to Ark UI.
Vitest / bundler config & index note
packages/ui/vitest.config.js, packages/ui/src/index\.js
Added @ark-ui to inline deps, swapped optimizer include to @ark-ui/solid, updated header comment and re-export change for Collapsible (adds useCollapsible).
Type definitions (Collapsible)
packages/ui/src/index.d.ts
Added CollapsibleApi, changed onOpenChange to accept { open }, expanded trigger/indicator types, added nested Collapsible subcomponents and useCollapsible export. (API/type surface change)
Component migrations — Ark UI (bulk)
packages/ui/src/zag/* (Avatar, Checkbox, Combobox, Dialog, Editable, Menu, NumberInput, PasswordInput, PinInput, Progress, RadioGroup, Select, Switch, TagsInput, Toast, ToggleGroup, Collapsible, Drawer, FileUpload, FloatingPanel, Popover, QRCode, Splitter, Tabs, Tooltip, Tour, Accordion, Clipboard, Collapsible, etc.)
Large-scale rewrites: replace Zag machine-based implementations with Ark UI primitives; components renamed internally (e.g., *Component) then re-exported to preserve public symbols; many modules add Ark hooks (useCheckbox, useSwitch, usePopover, useTooltip, useCollapsible, etc.). Reviewers should focus on changed props/behavior, exported aliases, and any subtle prop-shape differences (e.g., onOpenChange signature).
PDF subsystem — new modular pieces
packages/web/src/components/checklist-ui/pdf/{pdfDocument.js,pdfFileHandler.js,pdfRenderer.js,pdfScrollHandler.js}, packages/web/src/components/checklist-ui/pdf/usePdfJs.js
Replaced monolithic PDF hook with modular composition: createPdfDocument, createPdfFileHandler, createPdfRenderer, createPdfScrollHandler; usePdfJs now composes these modules and re-exports mapped signals/actions. Key attention: lifecycle, ArrayBuffer cloning, blob URL lifecycle, render scheduling/priority, and public API mappings.
Web app consumer updates (Collapsible & PDF usage)
packages/web/src/components/** (ProjectTreeItem.jsx, StudyTreeItem.jsx, NoteEditor.jsx, NotesCompareSection.jsx, ReviewerAssignment.jsx, TodoStudyRow.jsx, PdfTagSelect.jsx, PdfEmptyState.jsx, CreateLocalChecklist.jsx, PdfUploadSection.jsx, ScoreTag.jsx, SplitScreenLayout.jsx, common/NoteEditor.jsx)
Updated Collapsible usages to new onOpenChange signature (({ open }) => ...) and switched from api.getTriggerProps to direct trigger elements; broadened PDF file accept attributes to application/pdf,.pdf; UI tweaks (animations, transition adjustments, a few comment/text fixes).
Tests
packages/ui/src/zag/__tests__/Tooltip.test.jsx
Updated tests to assert using accessible text queries (screen.getByText) instead of internal DOM structure.
Global styles
packages/web/src/global.css
Added slideDown/slideUp keyframes and collapsible trigger/content styles to support new Collapsible animations.
Misc / comments / small fixes
packages/workers/src/index.js, packages/web/src/components/...
Minor comment simplifications and small behavior-preserving adjustments.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant UI as UI Component
participant FileHandler as PdfFileHandler
participant Document as PdfDocument
participant Renderer as PdfRenderer
participant Scroll as PdfScrollHandler
participant PDFjs as PDF.js Library

note over UI,FileHandler: User selects/uploads a file
UI->>FileHandler: handleFile(file)
FileHandler->>Document: setPdfSourceAndName(blobUrl/ArrayBuffer, name)
FileHandler->>Document: (clone ArrayBuffer if needed)
FileHandler->>FileHandler: revoke old blob URL
FileHandler-->>UI: callback onPdfChange(bufferCopy, name)

note over Document,PDFjs: Document prepares and loads via PDF.js
Document->>PDFjs: loadDocument(source)
PDFjs-->>Document: pdfDoc / totalPages or error
Document-->>Renderer: notify new pdfDoc / docId
Document-->>Scroll: notify totalPages/doc change

note over Renderer,Scroll: rendering coordination
Scroll->>Renderer: visiblePages -> schedule rendering
Renderer->>PDFjs: getPage(pageNum)
PDFjs-->>Renderer: page rendering data
Renderer->>Renderer: render page to canvas & text layer
Renderer-->>UI: page rendered (via pageCanvases signal)
Scroll->>UI: currentPage updates (on scroll/visibility)

alt on clear / new load
UI->>Document: clearPdf()
Document->>Renderer: cancelAllRenders() / cleanup()
Renderer->>Renderer: clear canvases/timeouts
end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • 81 todo tab multiple pdf support #83 — updates Collapsible API/usages to the new onOpenChange({ open }) signature and adds useCollapsible; strongly related to Collapsible type and consumer updates here.
  • user can now change pdfs in the pdf viewer #88 — modifies the PDF viewer stack (usePdfJs, PdfViewer, PdfToolbar and related modules); closely related to the new modular PDF pieces introduced in this change.
  • 49 feedback and support #57 — touches PDF loading behavior (ArrayBuffer cloning and loadSourceId logic) and overlaps with the pdfDocument/usePdfJs refactor present in this diff.

Poem

🐰
From hops of Zag to Ark's bright shore,
I swapped each widget, one by four.
New docs, new flows, files rendered fast,
Exports aliased—compat will last.
A joyful nibble, soft and smart!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.64% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'add ark ui and migrate most components' clearly summarizes the main change: adding Ark UI as a dependency and migrating most UI components from Zag.js to Ark UI throughout the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 133-migrate-to-ark-ui

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Dec 24, 2025

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
corates 1d41fd4 Commit Preview URL Dec 24 2025, 05:10 PM

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/ui/src/zag/TagsInput.jsx (1)

39-48: Add the validate prop to TagsInput.Root to enable duplicate checking.

The validate function is defined and merged into the context (lines 39-48), but it's not passed to TagsInput.Root (lines 57-70). Since Ark UI's TagsInput supports the validate prop, add it to enable duplicate validation:

 <TagsInput.Root
   value={context.value}
   defaultValue={context.defaultValue}
   onValueChange={handleValueChange}
+  validate={context.validate}
   max={context.max}
   disabled={context.disabled}

Without this prop, the duplicate validation logic will not be executed when tags are added.

🧹 Nitpick comments (23)
packages/web/src/components/profile-ui/LinkedAccountsSection.jsx (1)

85-95: Update comment to be framework-agnostic; "machine" terminology is Zag.js-specific.

Line 89 still references "dialog machine" which is terminology from Zag.js. In the context of this Ark UI migration, the comment should be updated to remove framework-specific jargon.

🔎 Proposed comment update
      // Check if this is an "already linked" error - offer merge option
      if (error.code === 'ACCOUNT_ALREADY_LINKED_TO_DIFFERENT_USER') {
        // Show merge dialog instead of just an error toast
-       // Small delay to ensure dialog machine is fully initialized
+       // Small delay to ensure dialog component is fully mounted
        setMergeConflictProvider(provider);
        setTimeout(() => {
          setShowMergeDialog(true);
        }, 100);
        return;
      }
packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx (1)

22-35: Consider wrapping derived state in createMemo for SolidJS reactivity.

The disabledValues function computes derived state from reactive props, but it's called synchronously during render (disabledValues()). Per coding guidelines, derived values should use createMemo to ensure they update reactively if props change.

🔎 Proposed refactor using `createMemo`
-import { Select } from '@corates/ui';
+import { createMemo } from 'solid-js';
+import { Select } from '@corates/ui';

 export default function PdfTagSelect(props) {
   // props.value: current tag value
   // props.onChange: (tag: string) => void
   // props.disabled: boolean
   // props.disablePrimary: boolean - disable primary option if another PDF has it
   // props.disableProtocol: boolean - disable protocol option if another PDF has it
 
-  const disabledValues = () => {
+  const disabledValues = createMemo(() => {
     const disabled = [];
     if (props.disablePrimary) disabled.push('primary');
     if (props.disableProtocol) disabled.push('protocol');
     return disabled;
-  };
+  });
 
   return (
     <Select
       items={TAG_OPTIONS}
       value={props.value || 'secondary'}
       onChange={props.onChange}
       disabled={props.disabled}
-      disabledValues={disabledValues()}
+      disabledValues={disabledValues()}
       placeholder='Select tag'
     />
   );
 }

Based on coding guidelines: "Use createMemo for derived values to ensure they update reactively."

packages/web/src/components/sidebar/ProjectTreeItem.jsx (1)

12-93: Consider reducing prop drilling for shared state.

The component currently receives 7 props, exceeding the guideline of 1-5 props for local configuration. Consider refactoring to source shared application state from stores or context:

  • userId: Could come from a user authentication store or context
  • currentPath: Could use useLocation() from @solidjs/router to access current route directly

This would reduce prop drilling and align with the guideline: "Do NOT prop-drill application state. Shared or cross-feature state must live in external stores."

As per coding guidelines: "Components should receive at most 1–5 props, and only for local configuration, not shared state."

packages/ui/src/zag/ToggleGroup.jsx (3)

59-59: Long class string could be extracted for readability.

The class string is functional but quite long. Consider extracting to a helper or using a class merging utility for maintainability.


1-6: Directory naming may be misleading after migration.

The component is located in packages/ui/src/zag/ but now uses Ark UI instead of Zag.js. Consider renaming the directory to ark or a neutral name like primitives to accurately reflect the underlying library. This would prevent confusion when debugging or onboarding new contributors. Based on learnings, the existing documentation references Zag.js components in this folder, which would need updating.


19-19: Props documentation mismatch with Ark UI API.

The documentation describes loop but the implementation at line 56 uses loopFocus. Consider updating the documentation to match the actual Ark UI prop name for consistency.

Proposed fix
- * - loop: boolean - Loop focus navigation (default: true)
+ * - loopFocus: boolean - Loop focus navigation (default: true)
Ark UI ToggleGroup loopFocus prop documentation
packages/ui/src/zag/Progress.jsx (2)

1-6: Consider folder structure alignment with the Ark UI migration.

The file resides in packages/ui/src/zag/ but now uses Ark UI instead of Zag.js. This creates a naming inconsistency that could confuse developers. Consider renaming the folder to reflect the new framework (e.g., ark/ or a generic primitives/) as part of this migration.

Based on learnings, the codebase previously mandated Zag.js for UI components. Since this PR migrates to Ark UI, the documentation and folder structure references to Zag.js should also be updated.


23-30: Rename machineProps to reflect Ark UI usage.

The variable name machineProps is a leftover from Zag.js (which uses state machines). Ark UI doesn't use this pattern, so consider renaming to rootProps or restProps for clarity.

Suggested rename
-  const [local, machineProps] = splitProps(props, [
+  const [local, rootProps] = splitProps(props, [
     'label',
     'showValue',
     'size',
     'variant',
     'indeterminate',
     'class',
   ]);

Then update references on lines 59, 65, and 66 accordingly.

packages/ui/src/zag/RadioGroup.jsx (1)

22-32: Consider refactoring variable names and handler wrapper.

The variable name machineProps is misleading since the migration to Ark UI no longer uses Zag state machines. Additionally, the handleValueChange wrapper adds unnecessary indirection.

Proposed refactor
-export function RadioGroupComponent(props) {
-  const [local, machineProps] = splitProps(props, ['items', 'label', 'class']);
+export function RadioGroupComponent(props) {
+  const [local, rootProps] = splitProps(props, ['items', 'label', 'class']);

  const orientation = () => machineProps.orientation || 'vertical';
  const isVertical = createMemo(() => orientation() === 'vertical');

-  const handleValueChange = details => {
-    if (machineProps.onValueChange) {
-      machineProps.onValueChange(details);
-    }
-  };

Then pass rootProps.onValueChange directly to RadioGroup.Root instead of handleValueChange.

packages/ui/src/zag/Toast.jsx (2)

1-8: Directory structure inconsistency and unused import.

Two concerns:

  1. The file remains in the packages/ui/src/zag/ directory despite migrating to Ark UI. This creates confusion about which library is being used. Consider renaming the directory to reflect the actual library (e.g., packages/ui/src/ark/ or packages/ui/src/toast/).
  2. The For import from solid-js on line 6 is not used in the component.
Proposed fix for unused import
-import { Show, For } from 'solid-js';
+import { Show } from 'solid-js';

25-49: Consider extracting helper functions outside the component.

The getIcon and getStyles helper functions are defined inside ToasterComponent, which means they're recreated on every render. While this doesn't cause issues in Solid.js (they're not reactive), extracting them outside the component would be a minor optimization and improve code organization.

Proposed refactor
+const getIcon = type => {
+  switch (type) {
+    case 'success':
+      return <FiCheck class='h-5 w-5 text-green-500' />;
+    case 'error':
+      return <FiAlertCircle class='h-5 w-5 text-red-500' />;
+    case 'loading':
+      return <FiLoader class='h-5 w-5 animate-spin text-blue-500' />;
+    default:
+      return <FiInfo class='h-5 w-5 text-blue-500' />;
+  }
+};
+
+const getStyles = type => {
+  switch (type) {
+    case 'success':
+      return 'border-green-200 bg-green-50';
+    case 'error':
+      return 'border-red-200 bg-red-50';
+    case 'loading':
+      return 'border-blue-200 bg-blue-50';
+    default:
+      return 'border-gray-200 bg-white';
+  }
+};
+
 export function ToasterComponent() {
-  const getIcon = type => {
-    switch (type) {
-      case 'success':
-        return <FiCheck class='h-5 w-5 text-green-500' />;
-      case 'error':
-        return <FiAlertCircle class='h-5 w-5 text-red-500' />;
-      case 'loading':
-        return <FiLoader class='h-5 w-5 animate-spin text-blue-500' />;
-      default:
-        return <FiInfo class='h-5 w-5 text-blue-500' />;
-    }
-  };
-
-  const getStyles = type => {
-    switch (type) {
-    case 'success':
-      return 'border-green-200 bg-green-50';
-    case 'error':
-      return 'border-red-200 bg-red-50';
-    case 'loading':
-      return 'border-blue-200 bg-blue-50';
-    default:
-      return 'border-gray-200 bg-white';
-    }
-  };
-
   return (
packages/ui/src/zag/Editable.jsx (1)

128-131: The controlled/uncontrolled pattern here is consistent with other Ark UI wrapped components in the codebase and works correctly; refactoring is optional.

The current approach of passing both value and defaultValue with conditional undefined is intentional and consistent across ToggleGroup, TagsInput, RadioGroup, NumberInput, and Combobox components. Ark UI handles this pattern correctly by treating undefined as "not provided." If cleaner code is desired, the spread operator approach would work, but it's not necessary for correctness.

.cursor/mcp.json (1)

6-6: Consider using a relative path for the corates MCP server.

The hardcoded absolute path /Users/jacobmaynard/Documents/Repos/corates/packages/mcp/server.js will break for other developers. Consider using a relative path or environment variable instead.

Proposed fix using relative path
-      "args": ["/Users/jacobmaynard/Documents/Repos/corates/packages/mcp/server.js"],
+      "args": ["./packages/mcp/server.js"],
packages/ui/package.json (1)

24-51: Audit and remove unused Zag.js packages from dependencies.

Currently, 28 Zag.js packages are declared in package.json, but only 13 are actually imported in the source code. At least 15 packages—avatar, checkbox, combobox, editable, menu, number-input, password-input, pin-input, progress, radio-group, select, switch, tags-input, toast, and toggle-group—appear to be completely unused. Removing these would reduce bundle size and simplify dependency management.

packages/ui/src/zag/TagsInput.jsx (2)

6-6: Remove unused import.

The For import from 'solid-js' is not used in the component. The code uses Index instead at line 80.

Proposed fix
-import { For, splitProps, mergeProps, Index } from 'solid-js';
+import { splitProps, mergeProps, Index } from 'solid-js';

57-70: Consider spreading remaining props to TagsInput.Root.

Currently, only specific props are explicitly passed to TagsInput.Root. If machineProps contains additional props that Ark UI supports (e.g., id, dir, allowOverflow, etc.), they won't be forwarded. Consider using prop spreading for better flexibility.

Proposed refactor
 <TagsInput.Root
-  value={context.value}
-  defaultValue={context.defaultValue}
-  onValueChange={handleValueChange}
-  max={context.max}
-  disabled={context.disabled}
-  readOnly={context.readOnly}
-  invalid={context.invalid}
-  name={context.name}
-  blurBehavior={context.blurBehavior}
-  addOnPaste={context.addOnPaste}
-  editable={context.editable}
+  {...context}
+  onValueChange={handleValueChange}
   class={`w-full ${local.class || ''}`}
 >

Note: Test thoroughly to ensure this doesn't introduce unintended props.

packages/ui/src/zag/Select.jsx (1)

69-90: Consider using For instead of Index for simpler item access.

Index provides a signal that must be called (item()) to get the value, which is correct here. However, For would allow direct access to the item without the extra function call, simplifying the code.

Optional simplification using For
-          <Index each={items()}>
-            {item => {
-              const itemValue = item();
-              const isDisabled = isValueDisabled(itemValue.value);
+          <For each={items()}>
+            {itemValue => {
+              const isDisabled = isValueDisabled(itemValue.value);
               return (
                 <Select.Item
                   item={itemValue}
                   ...
packages/ui/src/zag/Dialog.jsx (1)

274-291: Consider renaming for clarity.

The returned ConfirmDialogComponent property (line 291) references the locally-defined ConfirmDialogHookComponent, which could cause confusion since there's also a module-level ConfirmDialogComponent. Consider renaming the local component to something like BoundConfirmDialog or adjusting the return property name for clarity.

Proposed refactor for naming clarity
  // Pre-bound component that uses the hook's state
- function ConfirmDialogHookComponent() {
+ function BoundConfirmDialog() {
    return (
      <ConfirmDialogComponent
        open={isOpen()}
        onOpenChange={handleOpenChange}
        onConfirm={handleConfirm}
        loading={loading()}
        {...config()}
      />
    );
  }

  return {
    isOpen,
    open,
    close,
    setLoading,
-   ConfirmDialogComponent: ConfirmDialogHookComponent,
+   ConfirmDialogComponent: BoundConfirmDialog,
    dialogProps: () => ({
      open: isOpen(),
      onOpenChange: handleOpenChange,
      onConfirm: handleConfirm,
      loading: loading(),
      ...config(),
    }),
  };
packages/ui/src/zag/Checkbox.jsx (1)

1-3: Consider folder structure consistency.

The file resides in packages/ui/src/zag/ but now uses Ark UI. If the migration is complete across this PR, consider renaming the folder to avoid confusion (e.g., ark/ or primitives/). If preserving zag/ is intentional for incremental migration, updating the folder's README or adding a note would help clarify the transition.

Based on learnings, the project previously used Zag.js - the deviation here aligns with the PR objective to migrate to Ark UI.

packages/ui/src/zag/PinInput.jsx (3)

1-7: Consider renaming the folder to reflect Ark UI migration.

The file path still contains zag but the component now uses Ark UI. Consider renaming packages/ui/src/zag/ to packages/ui/src/ark/ or similar to maintain consistency between folder structure and implementation.

Based on learnings, the previous instruction was to use Zag.js for UI components. This PR intentionally migrates to Ark UI, which makes that learning obsolete.


19-27: Consider adding Zod schema for prop validation.

Per coding guidelines, Zod should be used for schema and input validation. Consider adding a Zod schema to validate the component props.

As per coding guidelines, "Use Zod for schema and input validation."


64-68: Hardcoded PIN length reduces flexibility.

The component hardcodes 6 inputs via [0, 1, 2, 3, 4, 5]. Consider making the length configurable through a length prop to support different PIN/OTP lengths (e.g., 4, 6, 8 digits).

Proposed refactor to support configurable length
  const merged = mergeProps(
    {
      required: true,
      otp: true,
      autoComplete: 'one-time-code',
+     length: 6,
    },
    props,
  );
-       <Index each={[0, 1, 2, 3, 4, 5]}>
+       <Index each={Array.from({ length: merged.length }, (_, i) => i)}>
          {(_, index) => (
            <PinInput.Input index={index()} required={required()} class={inputClass()} />
          )}
        </Index>
packages/ui/src/zag/PasswordInput.jsx (1)

22-29: Consider adding Zod schema for prop validation.

Per coding guidelines, Zod should be used for schema and input validation. Consider adding a Zod schema to validate the component props.

As per coding guidelines, "Use Zod for schema and input validation."

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a07c28a and e3d8f99.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (36)
  • .cursor/mcp.json
  • .cursor/rules/corates.mdc
  • .cursor/rules/ui-components.mdc
  • README.md
  • docs/architecture/diagrams.md
  • docs/architecture/diagrams/01-package-architecture.md
  • docs/architecture/diagrams/02-system-architecture.md
  • docs/architecture/diagrams/07-api-actions.md
  • docs/architecture/index.html
  • package.json
  • packages/mcp/README.md
  • packages/ui/README.md
  • packages/ui/package.json
  • packages/ui/src/index.js
  • packages/ui/src/zag/Avatar.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/vitest.config.js
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/workers/src/index.js
💤 Files with no reviewable changes (1)
  • package.json
🧰 Additional context used
📓 Path-based instructions (12)
**/*

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not use emojis in code, comments, documentation, or commit messages

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • docs/architecture/index.html
  • README.md
  • packages/mcp/README.md
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • docs/architecture/diagrams/07-api-actions.md
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/README.md
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • docs/architecture/diagrams/02-system-architecture.md
  • packages/workers/src/index.js
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/package.json
  • docs/architecture/diagrams.md
  • packages/ui/src/zag/Switch.jsx
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
packages/web/src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
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
Use createMemo for derived values to ensure they update reactively

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components
Each file should handle one coherent responsibility
Use Zod for schema and input validation

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/workers/src/index.js
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
packages/web/src/components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/components/**/*.{js,jsx,ts,tsx}: Use responsive design principles for UI components
Group related components in subdirectories with an index.js barrel export
Use Zag.js for UI components and design system
Zag component exist in packages/web/src/components/zag/* and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
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
Do not destructure props in SolidJS components 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. They should not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Group related components in subdirectories with an index.js barrel export

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
**/*.{js,jsx,ts,tsx,md,json,sql,txt}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Do not use emojis in code, comments, documentation, plan files, or commit messages

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • README.md
  • packages/mcp/README.md
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • docs/architecture/diagrams/07-api-actions.md
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/README.md
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • docs/architecture/diagrams/02-system-architecture.md
  • packages/workers/src/index.js
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/package.json
  • docs/architecture/diagrams.md
  • packages/ui/src/zag/Switch.jsx
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

**/*.{jsx,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Use responsive design principles for UI components

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
packages/web/**/*.{js,jsx,ts,tsx,css}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Ensure browser compatibility for all frontend code (Safari is usually problematic)

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
packages/web/src/components/**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

packages/web/src/components/**/*.{jsx,tsx}: Use Zag.js for UI components and design system
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
Do NOT destructure props in SolidJS components - access props directly from the props object or wrap in a function to maintain reactivity
Components should be lean and focused - do not implement business logic; move it into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
packages/web/src/**/*.{jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Use createMemo for derived values and createStore for complex state in SolidJS

Files:

  • packages/web/src/components/profile-ui/LinkedAccountsSection.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
packages/workers/src/**/*.{js,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/workers/src/**/*.{js,ts}: Use Zod for schema and input validation on the backend
Use Drizzle ORM for database interactions and migrations
Use Better-Auth for authentication and user management

Files:

  • packages/workers/src/index.js
packages/workers/**/*.{js,ts,sql}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Use Drizzle ORM for database interactions and migrations

Files:

  • packages/workers/src/index.js
packages/workers/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Use Better-Auth for authentication and user management

ALWAYS use db.batch() for multiple related database operations to ensure atomicity in Drizzle ORM

Files:

  • packages/workers/src/index.js
🧠 Learnings (30)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Follow standard JavaScript/SolidJS/Cloudflare best practices

Applied to files:

  • README.md
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/mcp/README.md
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • .cursor/rules/ui-components.mdc
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/README.md
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • docs/architecture/diagrams/02-system-architecture.md
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • .cursor/rules/corates.mdc
  • packages/ui/package.json
  • packages/ui/src/zag/Switch.jsx
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging

Applied to files:

  • packages/mcp/README.md
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • .cursor/rules/ui-components.mdc
  • packages/ui/README.md
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • docs/architecture/diagrams/02-system-architecture.md
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • .cursor/rules/corates.mdc
  • packages/ui/package.json
  • packages/ui/src/zag/Switch.jsx
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/mcp/README.md
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/src/index.js
  • .cursor/rules/ui-components.mdc
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/README.md
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/Progress.jsx
  • docs/architecture/diagrams/02-system-architecture.md
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Combobox.jsx
  • .cursor/rules/corates.mdc
  • packages/ui/package.json
  • packages/ui/src/zag/Switch.jsx
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfTagSelect.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Select.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/primitives/**/*.{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/ui/src/index.js
  • docs/architecture/diagrams/01-package-architecture.md
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/primitives/**/*.{ts,tsx} : Create reusable logic in primitives (hooks) that can be shared across components

Applied to files:

  • packages/ui/src/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : For UI icons, use the `solid-icons` library or SVGs only. Do not use emojis

Applied to files:

  • .cursor/rules/ui-components.mdc
  • packages/ui/vitest.config.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use responsive design principles for UI components

Applied to files:

  • .cursor/rules/ui-components.mdc
  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Group related components in subdirectories with an `index.js` barrel export

Applied to files:

  • .cursor/rules/ui-components.mdc
  • .cursor/rules/corates.mdc
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Group related components in subdirectories with an index.js barrel export

Applied to files:

  • .cursor/rules/ui-components.mdc
  • .cursor/rules/corates.mdc
  • docs/architecture/diagrams/01-package-architecture.md
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Components should be lean and focused. They should not implement business logic; move that into stores, utilities, or primitives

Applied to files:

  • .cursor/rules/ui-components.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Components should be lean and focused - do not implement business logic; move it into stores, utilities, or primitives

Applied to files:

  • .cursor/rules/ui-components.mdc
  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Never have a component act as a God component coordinating multiple large concerns

Applied to files:

  • .cursor/rules/ui-components.mdc
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/stores/**/*.{js,ts} : Use Solid's `createStore` for complex state or state objects for better performance and reactivity

Applied to files:

  • docs/architecture/diagrams/02-system-architecture.md
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Do not destructure props in SolidJS components 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:

  • docs/architecture/diagrams/02-system-architecture.md
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,ts} : Use `createMemo` for derived values and `createStore` for complex state in SolidJS

Applied to files:

  • docs/architecture/diagrams/02-system-architecture.md
  • docs/architecture/diagrams/01-package-architecture.md
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components

Applied to files:

  • .cursor/rules/corates.mdc
  • docs/architecture/diagrams/01-package-architecture.md
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Zod for schema and input validation on the backend

Applied to files:

  • .cursor/rules/corates.mdc
  • docs/architecture/diagrams/01-package-architecture.md
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Each file should handle one coherent responsibility

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Better-Auth for authentication and user management

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/workers/**/*.{js,ts,sql} : Use Drizzle ORM for database interactions and migrations

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use Zod for schema and input validation

Applied to files:

  • .cursor/rules/corates.mdc
  • packages/ui/package.json
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Drizzle ORM for database interactions and migrations

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Use the Corates MCP tools to explore local documentation sources for Better-Auth, Drizzle, Icons, and Zag documentation

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:16.398Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/workers.mdc:0-0
Timestamp: 2025-12-23T04:40:16.398Z
Learning: Applies to packages/workers/src/config/validation.{js,ts} : Add new Zod schemas to `config/validation.js` and reuse `commonFields` when possible to avoid duplication

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Do NOT add tests unless specifically asked. See TESTING.md for testing guidelines and best practices

Applied to files:

  • .cursor/rules/corates.mdc
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{jsx,tsx} : For UI icons, use the `solid-icons` library or SVGs only. Do not use emojis

Applied to files:

  • packages/ui/vitest.config.js
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/**/*.{js,jsx,ts,tsx,css} : Ensure browser compatibility for all frontend code (Safari is usually problematic)

Applied to files:

  • packages/ui/vitest.config.js
🧬 Code graph analysis (10)
packages/ui/src/zag/NumberInput.jsx (1)
packages/ui/src/index.d.ts (1)
  • NumberInput (530-530)
packages/ui/src/zag/TagsInput.jsx (3)
packages/ui/src/zag/Editable.jsx (2)
  • handleValueChange (106-110)
  • value (86-86)
packages/ui/src/zag/PinInput.jsx (1)
  • handleValueChange (34-38)
packages/ui/src/index.d.ts (1)
  • TagsInput (831-831)
packages/ui/src/zag/Checkbox.jsx (1)
packages/ui/src/zag/Switch.jsx (3)
  • merged (18-23)
  • defaultChecked (26-26)
  • name (28-28)
packages/ui/src/zag/Editable.jsx (3)
packages/ui/src/index.d.ts (1)
  • Editable (350-350)
packages/ui/src/lib/cn.js (1)
  • cn (16-18)
packages/ui/src/zag/NumberInput.jsx (1)
  • showControls (50-50)
packages/ui/src/zag/Menu.jsx (3)
packages/ui/src/zag/Combobox.jsx (4)
  • isOpen (73-73)
  • handleOpenChange (75-80)
  • renderContent (88-115)
  • local (36-43)
packages/ui/src/zag/Dialog.jsx (4)
  • isOpen (223-223)
  • handleOpenChange (29-33)
  • handleOpenChange (108-112)
  • handleOpenChange (267-271)
packages/ui/src/index.d.ts (1)
  • Menu (479-479)
packages/ui/src/zag/PinInput.jsx (5)
packages/ui/src/zag/Combobox.jsx (1)
  • handleValueChange (66-70)
packages/ui/src/zag/Editable.jsx (3)
  • handleValueChange (106-110)
  • merged (72-84)
  • inputClass (102-102)
packages/ui/src/zag/TagsInput.jsx (1)
  • handleValueChange (50-54)
packages/ui/src/index.d.ts (1)
  • PinInput (576-576)
packages/ui/src/zag/PasswordInput.jsx (2)
  • required (25-25)
  • inputClass (26-26)
packages/ui/src/zag/ToggleGroup.jsx (2)
packages/ui/src/zag/RadioGroup.jsx (3)
  • orientation (25-25)
  • isVertical (26-26)
  • handleValueChange (28-32)
packages/ui/src/zag/Editable.jsx (1)
  • handleValueChange (106-110)
packages/ui/src/zag/Avatar.jsx (1)
packages/ui/src/index.d.ts (1)
  • Avatar (74-74)
packages/ui/src/zag/Dialog.jsx (4)
packages/ui/src/zag/Drawer.jsx (6)
  • open (24-24)
  • size (26-26)
  • title (27-27)
  • description (28-28)
  • children (29-29)
  • getSizeClass (44-59)
packages/ui/src/zag/Tabs.jsx (1)
  • children (18-18)
packages/ui/src/index.d.ts (1)
  • Dialog (267-267)
packages/ui/src/constants/zIndex.js (2)
  • Z_INDEX (15-44)
  • Z_INDEX (15-44)
packages/ui/src/zag/Toast.jsx (2)
packages/ui/src/index.d.ts (1)
  • Toaster (848-848)
packages/ui/src/constants/zIndex.js (2)
  • Z_INDEX (15-44)
  • Z_INDEX (15-44)
⏰ 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

Comment thread packages/ui/src/zag/Checkbox.jsx
Comment thread packages/ui/src/zag/Dialog.jsx Outdated
Comment on lines +49 to +81
<Dialog.Root open={open()} onOpenChange={handleOpenChange}>
<Show when={open()}>
<Portal>
<Dialog.Backdrop
class={`fixed inset-0 ${Z_INDEX.BACKDROP} bg-black/50 transition-opacity`}
/>
<Dialog.Positioner
class={`fixed inset-0 ${Z_INDEX.DIALOG} flex items-center justify-center overflow-y-auto p-4`}
>
{/* Header */}
<div class='flex items-center justify-between border-b border-gray-200 p-4'>
<div>
<h2 {...api().getTitleProps()} class='text-lg font-semibold text-gray-900'>
{title()}
</h2>
<Show when={description()}>
<p {...api().getDescriptionProps()} class='mt-1 text-sm text-gray-500'>
{description()}
</p>
</Show>
<Dialog.Content
class={`my-auto w-full rounded-lg bg-white shadow-xl ${getSizeClass()}`}
>
{/* Header */}
<div class='flex items-center justify-between border-b border-gray-200 p-4'>
<div>
<Dialog.Title class='text-lg font-semibold text-gray-900'>{title()}</Dialog.Title>
<Show when={description()}>
<Dialog.Description class='mt-1 text-sm text-gray-500'>
{description()}
</Dialog.Description>
</Show>
</div>
<Dialog.CloseTrigger class='rounded-md p-1 text-gray-400 transition-colors hover:bg-gray-100 hover:text-gray-500'>
<FiX class='h-5 w-5' />
</Dialog.CloseTrigger>
</div>
<button
{...api().getCloseTriggerProps()}
class='rounded-md p-1 text-gray-400 transition-colors hover:bg-gray-100 hover:text-gray-500'
>
<FiX class='h-5 w-5' />
</button>
</div>
{/* Body */}
<div class='p-4'>{children()}</div>
</div>
</div>
</Portal>
</Show>
{/* Body */}
<div class='p-4'>{children()}</div>
</Dialog.Content>
</Dialog.Positioner>
</Portal>
</Show>
</Dialog.Root>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Ark UI Solid Dialog.Root open state mounting unmounting Portal Show

💡 Result:

Short answer — behavior and how to control it

  • Dialog.Root’s open (or controlled open + onOpenChange) controls when the dialog becomes visible; the dialog tree inside Dialog.Root (including anything wrapped in Portal) is mounted according to the dialog mounting props. [1]
  • Use lazyMount on Dialog.Root to delay mounting until the dialog is first opened. Use unmountOnExit to remove the dialog nodes when it closes. These two together give you “mount on open” and “unmount on close” behavior. onExitComplete runs after the close animation and unmount (if used). [1]
  • present is a lower-level prop for controlling presence (useful for animation libraries); skipAnimationOnMount affects initial animations. [1]
  • Ark’s Portal simply renders its children into document.body (or a specified mount). If the dialog content isn’t mounted (because of lazyMount/unmountOnExit or you control open), the Portal has nothing to render. In Solid use the Solid Portal (solid-js/web) as shown by Ark — it will mount into the given node when its children exist. [1][2]

Minimal Solid example

import { Dialog } from '@ark-ui/solid/dialog'
import { Portal } from 'solid-js/web'

<Dialog.Root lazyMount unmountOnExit onExitComplete={() => console.log('closed')}>
<Dialog.Trigger>Open</Dialog.Trigger>

<Dialog.Backdrop />
<Dialog.Positioner>
<Dialog.Content>…</Dialog.Content>
</Dialog.Positioner>

</Dialog.Root>

Sources

  • Ark UI Dialog docs (lazyMount, unmountOnExit, present, onExitComplete, Portal examples). [1]
  • Solid docs (portal mounts children into document.body by default). [2]

🌐 Web query:

Ark UI Solid Dialog documentation best practices

💡 Result:

Here are concise best practices for using Ark UI’s Dialog in Solid (sources cited):

  • Use the provided API (Dialog.Root / Dialog.Trigger / Dialog.Content / Backdrop / Positioner / CloseTrigger / Title / Description) and Portal for overlay rendering. [1][2]
  • Choose one control approach: declarative Dialog.Root OR programmatic useDialog() + Dialog.RootProvider — don’t mix both. [2]
  • Accessibility: set aria-label or render a Dialog.Title/Description; use role='dialog' or 'alertdialog' as needed. Ensure trapFocus, restoreFocus, and preventScroll are enabled for modal dialogs. [1][2]
  • Focus management: provide initialFocusEl / finalFocusEl when you need custom focus placement; rely on defaults when possible. [1]
  • Closing behavior: configure closeOnEscape and closeOnInteractOutside for expected UX; provide explicit CloseTrigger buttons. [1]
  • Non-modal dialogs: use modal={false} to allow interaction outside the dialog (disables focus trapping/scroll blocking). Use only when appropriate (auxiliary panels). [2]
  • Lazy mount / unmountOnExit: enable lazyMount or unmountOnExit for performance when dialogs are heavy or rarely used; consider skipAnimationOnMount for initial render. [1]
  • Nested dialogs: Ark handles stacking/z-index automatically (via Zag.js variables); use nested useDialog instances for programmatic control. [2]
  • Styling: Ark is unstyled — use data-part and data-scope attributes to target parts reliably (e.g., [data-part]="content"/"backdrop"). Keep styles separate from logic. [2][3]
  • Testing & state: rely on Ark’s Zag.js state machine semantics for predictable behavior; test keyboard interactions (Tab, Shift+Tab, Esc) and screen-reader announcements. [3][1]

Sources: Ark UI Solid Dialog docs and examples; Ark UI component docs; Ark GitHub (lib/ark). [1][2][3]


🏁 Script executed:

# Check if there are other Dialog implementations in the codebase that might provide context
find . -type f \( -name "*.jsx" -o -name "*.tsx" -o -name "*.ts" -o -name "*.js" \) -exec grep -l "Dialog\\.Root" {} \; | head -20

Repository: InfinityBowman/corates

Length of output: 98


🏁 Script executed:

# Look for Ark UI Dialog-related imports and patterns
rg "from.*@ark-ui" --type jsx --type tsx | head -30

Repository: InfinityBowman/corates

Length of output: 93


🏁 Script executed:

# Check the specific Dialog.jsx file to understand its full context
wc -l packages/ui/src/zag/Dialog.jsx

Repository: InfinityBowman/corates

Length of output: 100


🏁 Script executed:

# Check the Dialog.jsx file to see Dialog.Root configuration
head -100 packages/ui/src/zag/Dialog.jsx

Repository: InfinityBowman/corates

Length of output: 3501


🏁 Script executed:

# Look for the Dialog.Root component definition and props
sed -n '1,100p' packages/ui/src/zag/Dialog.jsx | cat -n

Repository: InfinityBowman/corates

Length of output: 4201


Remove the Show wrapper and add proper mounting control to Dialog.Root.

The <Show when={open()}> wrapper creates conflicting state management with Ark UI's internal Dialog state machine. Ark UI's Dialog.Root handles mounting/unmounting via lazyMount and unmountOnExit props. Remove the Show wrapper and add these props to Dialog.Root instead:

<Dialog.Root open={open()} onOpenChange={handleOpenChange} lazyMount unmountOnExit>
  <Portal>
    {/* Dialog content */}
  </Portal>
</Dialog.Root>

This ensures Ark UI's state machine and transitions work correctly without external Show logic interfering.

🤖 Prompt for AI Agents
In packages/ui/src/zag/Dialog.jsx around lines 49 to 81, remove the surrounding
<Show when={open()}> wrapper because it conflicts with Ark UI Dialog's internal
state machine; instead keep Dialog.Root as the top-level component and add the
props lazyMount and unmountOnExit to Dialog.Root while preserving open={open()}
and onOpenChange={handleOpenChange}, and ensure Portal and the Dialog children
are direct descendants of Dialog.Root so Ark UI can handle mounting/unmounting
and transitions correctly.

Comment thread packages/ui/src/zag/Editable.jsx Outdated
Comment thread packages/ui/src/zag/PasswordInput.jsx Outdated
Comment on lines +59 to +63
<PasswordInput.VisibilityTrigger class='absolute top-1/2 right-3 -translate-y-1/2 text-gray-400 sm:right-4'>
<PasswordInput.Indicator fallback={<FiEyeOff size={iconSize() - 2} />}>
<FiEye size={iconSize()} />
</PasswordInput.Indicator>
</PasswordInput.VisibilityTrigger>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Icon size inconsistency in visibility toggle.

Line 60 uses iconSize() - 2 for FiEyeOff, while line 61 uses iconSize() for FiEye. This creates an inconsistent visual experience when toggling visibility.

Proposed fix for consistent icon sizing
-        <PasswordInput.VisibilityTrigger class='absolute top-1/2 right-3 -translate-y-1/2 text-gray-400 sm:right-4'>
-          <PasswordInput.Indicator fallback={<FiEyeOff size={iconSize() - 2} />}>
-            <FiEye size={iconSize()} />
+        <PasswordInput.VisibilityTrigger class='absolute top-1/2 right-3 -translate-y-1/2 text-gray-400 sm:right-4'>
+          <PasswordInput.Indicator fallback={<FiEyeOff size={iconSize()} />}>
+            <FiEye size={iconSize()} />
           </PasswordInput.Indicator>
         </PasswordInput.VisibilityTrigger>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<PasswordInput.VisibilityTrigger class='absolute top-1/2 right-3 -translate-y-1/2 text-gray-400 sm:right-4'>
<PasswordInput.Indicator fallback={<FiEyeOff size={iconSize() - 2} />}>
<FiEye size={iconSize()} />
</PasswordInput.Indicator>
</PasswordInput.VisibilityTrigger>
<PasswordInput.VisibilityTrigger class='absolute top-1/2 right-3 -translate-y-1/2 text-gray-400 sm:right-4'>
<PasswordInput.Indicator fallback={<FiEyeOff size={iconSize()} />}>
<FiEye size={iconSize()} />
</PasswordInput.Indicator>
</PasswordInput.VisibilityTrigger>
🤖 Prompt for AI Agents
In packages/ui/src/zag/PasswordInput.jsx around lines 59 to 63, the visibility
toggle uses inconsistent icon sizes (FiEyeOff uses iconSize() - 2 while FiEye
uses iconSize()); change both uses to the same size by computing the size once
(e.g., const size = iconSize()) and use that variable for both FiEye and
FiEyeOff so the icons remain visually consistent when toggling.

Comment on lines +79 to +86
<Progress.Track class={`w-full overflow-hidden rounded-full bg-gray-200 ${getSizeClass()}`}>
<Progress.Range
class={`h-full rounded-full transition-all duration-300 ${getVariantClass()} ${
local.indeterminate ? 'animate-progress-indeterminate' : ''
}`}
style={local.indeterminate ? { width: '30%' } : undefined}
/>
</div>
</div>
</Progress.Track>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for animate-progress-indeterminate class definition
echo "=== Searching for animate-progress-indeterminate in config files ==="
rg -n "progress-indeterminate|animate.*progress" --type-list | grep -i config
find . -type f \( -name "tailwind.config.*" -o -name "*.config.*" \) | head -20

echo ""
echo "=== Searching in Tailwind config files ==="
find . -type f -name "tailwind.config.*" -exec rg -n "progress-indeterminate" {} \;

echo ""
echo "=== Searching in CSS/SCSS files ==="
rg -n "progress-indeterminate|@keyframes.*progress" -t css -t scss

echo ""
echo "=== Searching globally for animate-progress-indeterminate ==="
rg -n "animate-progress-indeterminate" --no-heading

echo ""
echo "=== Check for animation definitions in all files ==="
rg -n "@keyframes|animation.*progress" --type-list | head -10
rg -n "@keyframes|animation.*progress"

Repository: InfinityBowman/corates

Length of output: 2529


Define the animate-progress-indeterminate CSS animation.

The code references the class animate-progress-indeterminate at line 82, but this animation is not defined anywhere in the codebase. Add the keyframes definition to your Tailwind config or global CSS file. The test at packages/ui/src/zag/__tests__/Progress.test.jsx:142 also expects this class.

🤖 Prompt for AI Agents
packages/ui/src/zag/Progress.jsx lines 79-86: the component uses the CSS class
animate-progress-indeterminate which is not defined; add a keyframes + animation
entry named animate-progress-indeterminate either in your Tailwind config
(theme.extend.keyframes and theme.extend.animation) or in a global CSS file.
Define keyframes that perform a continuous indeterminate progress motion (e.g.,
translateX or width shift) and expose an animation utility called
animate-progress-indeterminate with appropriate duration, timing-function and
infinite repetition so the class can be applied exactly as referenced by the
component and expected by the test.

Comment on lines +1 to 8
/**
* Select - A custom select/dropdown component using Ark UI
*/

import { Select, createListCollection } from '@ark-ui/solid/select';
import { Portal } from 'solid-js/web';
import { useMachine, normalizeProps } from '@zag-js/solid';
import { createMemo, createUniqueId, For, Show } from 'solid-js';
import { createMemo, For, Show, Index } from 'solid-js';
import { BiRegularCheck, BiRegularChevronDown } from 'solid-icons/bi';
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n -i "zag" --type md -g '!**/CHANGELOG.md' -g '!**/node_modules/**'

Repository: InfinityBowman/corates

Length of output: 605


🏁 Script executed:

cat -n .cursor/rules/corates.mdc

Repository: InfinityBowman/corates

Length of output: 6238


🏁 Script executed:

cat -n .github/copilot-instructions.md

Repository: InfinityBowman/corates

Length of output: 6644


Guidance files contain outdated and inconsistent statements about Ark UI vs. Zag.js that should be cleaned up.

The migration to Ark UI is correct. However, .cursor/rules/corates.mdc line 39 and .github/copilot-instructions.md line 39 still recommend Zag.js for UI components, which contradicts the Ark UI guidance elsewhere in these same files and the completed migration documented in packages/ui/README.md. Additionally, .github/copilot-instructions.md lines 45-47 incorrectly reference Zag components in packages/web/src/components/zag/* rather than the actual @corates/ui package location. These inconsistencies should be resolved by removing the outdated Zag.js recommendations and standardizing documentation to reflect that Ark UI is the current standard for UI components in the /ui package.

🤖 Prompt for AI Agents
In packages/ui/src/zag/Select.jsx lines 1-8 and related docs
(.cursor/rules/corates.mdc line 39, .github/copilot-instructions.md lines 39 and
45-47), there are outdated references recommending Zag.js and pointing to
packages/web/src/components/zag/*; update documentation and comments to reflect
the Ark UI migration: remove Zag.js recommendations, replace any Zag component
paths with the correct @corates/ui package paths, and standardize all guidance
to state Ark UI as the current standard for UI components in the /ui package.

Comment on lines +120 to +124
<Select.ValueText placeholder={placeholder()} />
<Select.Indicator>
<BiRegularChevronDown class='h-5 w-5 text-gray-400 transition-transform data-[state=open]:rotate-180' />
</Select.Indicator>
</Select.Trigger>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n packages/ui/src/zag/Select.jsx

Repository: InfinityBowman/corates

Length of output: 6110


🌐 Web query:

Ark UI SolidJS Select.Indicator data-state attribute children

💡 Result:

Short answer: the Select.Indicator doesn't provide a render prop with state — it sets a data-state attribute on the rendered element ("open" | "closed"), so you should either (A) inspect that attribute with CSS to show/animate different children (preferred), or (B) render both children and hide/show them via the data-state attribute. Example SolidJS code + CSS:

Code (Solid):
import { Select, createListCollection } from "@ark-ui/solid/select";
const collection = createListCollection({ items: ["A","B","C"] });

<Select.Root collection={collection}>
<Select.Trigger>
<Select.ValueText placeholder="Choose…" />
<Select.Indicator>
{/* render both and use CSS to show the right one */}


</Select.Indicator>
</Select.Trigger>

</Select.Root>

CSS:
[data-part="indicator"] .icon-chevron { display: inline-block; }
[data-part="indicator"] .icon-x { display: none; }

/* when open, swap icons or animate */
[data-part="indicator"][data-state="open"] .icon-chevron { display: none; }
[data-part="indicator"][data-state="open"] .icon-x { display: inline-block; }

/* or rotate the chevron when open */
[data-part="indicator"] .icon-chevron { transition: transform .15s; }
[data-part="indicator"][data-state="open"] .icon-chevron { transform: rotate(180deg); }

Notes:

  • Ark sets data-state on the Indicator element; use that selector to style or hide/show children. (ark-ui.com)

If you want, I can give a complete runnable Solid + CSS example (with SVGs) or show how to use asChild to pass your own element.

Citations:


🌐 Web query:

Ark UI documentation Select component data-[state=open] styling

💡 Result:

Use the component's data attributes. Ark UI emits data-scope, data-part and data-state (e.g. "open" / "closed") on Select parts, so target those in CSS.

Example CSS targets

  • trigger when open:
    [data-scope="select"][data-part="trigger"][data-state="open"] { /* styles */ }
  • control when open:
    [data-scope="select"][data-part="control"][data-state="open"] { /* styles */ }
  • content when open:
    [data-scope="select"][data-part="content"][data-state="open"] { /* styles */ }

Example — change trigger background + rotate indicator:
[data-scope="select"][data-part="trigger"][data-state="open"] { background:#f3f4f6; }
[data-scope="select"][data-part="indicator"][data-state="open"] { transform: rotate(180deg); }

See Ark UI styling guide and Select API (lists data-part and data-state values). [1][2]

Sources
[1] Ark UI — Styling guide.
[2] Ark UI — Select component docs (data-part / data-state).


Move the data-[state=open]:rotate-180 class from the icon to the Select.Indicator element.

Ark UI sets the data-state attribute on the Select.Indicator element itself, not on child elements. The Tailwind selector data-[state=open]:rotate-180 on BiRegularChevronDown will not match because the icon does not receive the data-state attribute. Apply the rotation class to Select.Indicator instead, or use a global CSS selector targeting [data-part="indicator"][data-state="open"].

🤖 Prompt for AI Agents
In packages/ui/src/zag/Select.jsx around lines 120 to 124, the Tailwind rotation
class `data-[state=open]:rotate-180` is applied to the BiRegularChevronDown icon
but Ark UI sets `data-state` on the Select.Indicator element, so move the
`data-[state=open]:rotate-180` class from the icon to the Select.Indicator
element (or alternatively add a global CSS selector targeting
`[data-part="indicator"][data-state="open"]`); ensure the icon keeps sizing and
color classes while the indicator receives the state-based rotation class so the
chevron rotates when the select opens.

Comment on lines +50 to +54
const handleValueChange = details => {
if (context.onValueChange) {
context.onValueChange(details);
}
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -name "TagsInput.jsx" -type f

Repository: InfinityBowman/corates

Length of output: 101


🏁 Script executed:

find . -name "PinInput.jsx" -o -name "Editable.jsx" | grep -E "(PinInput|Editable)\.jsx$"

Repository: InfinityBowman/corates

Length of output: 135


🏁 Script executed:

find . -path "*/packages/ui/src/zag/*" -name "*.jsx" -type f | head -20

Repository: InfinityBowman/corates

Length of output: 859


🏁 Script executed:

cat -n packages/ui/src/zag/TagsInput.jsx | head -70

Repository: InfinityBowman/corates

Length of output: 2727


🏁 Script executed:

cat -n packages/ui/src/zag/PinInput.jsx | head -70

Repository: InfinityBowman/corates

Length of output: 2624


🏁 Script executed:

cat -n packages/ui/src/zag/Editable.jsx | head -70

Repository: InfinityBowman/corates

Length of output: 4036


🌐 Web query:

Ark UI solid TagsInput onValueChange callback signature documentation

💡 Result:

The callback signature is:

onValueChange: (details: ValueChangeDetails) => void

Where ValueChangeDetails contains the updated tags (as an array). See the TagsInput API docs (lists onValueChange as (details: ValueChangeDetails) => void) and the TagsInput context showing the controlled value is string[] (tags). [1][2]

Sources:
[1] Ark UI — Tags Input (events & props).
[2] Ark UI — Tags Input (Context / value = string[]).


🌐 Web query:

@ark-ui/solid tags-input version 5.30.0 onValueChange details parameter

💡 Result:

In v5.30.0 the onValueChange handler receives a single details object whose main property is value — the updated tags array (string[]). Example:

<TagsInput.Root value={value} onValueChange={(details) => setValue(details.value)} />

Sources: Ark UI TagsInput docs (events & API). [1][2]

References

  • Tags Input docs (events/API) — Ark UI. [1]
  • Tags Input examples (controlled onValueChange usage) — Ark UI. [2]

Update JSDoc to reflect the actual ValueChangeDetails object structure.

The JSDoc at line 17 documents onValueChange: (details: { value: string[] }), but the handler forwards the entire Ark UI ValueChangeDetails object. The actual callback receives additional properties beyond just value. Either update the JSDoc to accurately document the full object structure or extract and transform the details object to match the documented signature (like PinInput.jsx does with details.valueAsString).

🤖 Prompt for AI Agents
In packages/ui/src/zag/TagsInput.jsx around lines 50 to 54, the handler
currently forwards the entire Ark UI ValueChangeDetails object to
context.onValueChange but the JSDoc claims the callback signature is
onValueChange: (details: { value: string[] }); either update the JSDoc to
reflect the full ValueChangeDetails shape (list all properties forwarded, e.g.,
value, valueAsString, ...), or change the handler to extract and forward only
the value (e.g., call context.onValueChange({ value: details.value })) so the
runtime matches the documented signature; pick one approach and make the JSDoc
and implementation consistent.

Comment on lines +14 to 19
export const toaster = createToaster({
placement: 'top-end',
overlap: true,
gap: 12,
offsets: '16px',
removeDelay: 200, // Allow time for fade out animation
offset: '16px',
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find the package.json to check Ark UI version
fd package.json --type f | head -5

Repository: InfinityBowman/corates

Length of output: 188


🏁 Script executed:

# Read the Toast.jsx file to see the full context
cat -n packages/ui/src/zag/Toast.jsx

Repository: InfinityBowman/corates

Length of output: 4429


🏁 Script executed:

# Check if there's any Ark UI documentation or type definitions in the repo
fd -e d.ts -e ts | xargs grep -l "createToaster" 2>/dev/null | head -5

Repository: InfinityBowman/corates

Length of output: 48


🏁 Script executed:

# Check the UI package.json for Ark UI version
cat packages/ui/package.json | grep -A 5 -B 5 "ark-ui"

Repository: InfinityBowman/corates

Length of output: 375


🌐 Web query:

Ark UI createToaster API documentation for Solid.js configuration options

💡 Result:

Here are the Solid.js API/configuration options for createToaster (Ark UI), with typical toast creation options:

createToaster (top-level options)

  • placement: string — group placement on screen (e.g., "top-start", "top-end", "bottom-start", "bottom-end"). Default examples use "bottom-end". [1][2]
  • overlap: boolean — whether to allow toasts to overlap. [1]
  • gap: number — pixel gap between toasts in a group. [1]
  • max: number — maximum visible toasts; extra toasts are queued. [1]
    (Note: createToaster is exposed from each framework package; Solid implementation delegates to @zag-js/toast.group.machine and uses these props.) [2]

Toaster component props (when rendering the Toaster returned by createToaster)

  • toaster: CreateToasterReturn — the object returned by createToaster. [1]
  • dir: 'ltr' | 'rtl' — document writing direction (default 'ltr'). [1]
  • getRootNode: () => Node | ShadowRoot | Document — custom root resolver (iframes, shadow DOM, Electron). [1]

Per-toast options (used with toaster.create / toaster.info/success/etc.)

  • id: string | number — optional id for the toast.
  • title: string
  • description: string
  • type: 'info' | 'success' | 'warning' | 'error' — shorthand type helpers exist (info/success/warning/error).
  • duration: number | Infinity — milliseconds toast stays visible (Infinity to disable auto-close).
  • action: { label: string, onClick: () => void } — optional action that closes the toast when triggered.
    (These per-toast options are shown in examples and the docs for creating toasts.) [1][4]

Sources

  1. Ark UI Toast documentation (props, examples, Toaster API). [ark-ui.com/docs/components/toast]. [1]
  2. Solid package create-toaster source (createToaster delegates to @zag-js/toast group machine). [unpkg/@ark-ui/solid create-toaster source]. [2]
  3. Ark UI examples mentioning max/overlap/gap/placement and per-toast options. [ark-ui.com (examples)]. [1]

If you want, I can paste the TypeScript types (CreateToasterProps / toast options) from the package distribution for your exact project version.


Remove the invalid offset property from createToaster configuration.

The offset: '16px' property is not a valid configuration option for Ark UI's createToaster API (version 5.30.0). Valid options are placement, overlap, gap, and max. Remove the offset property or handle spacing through CSS/Tailwind classes if needed.

🤖 Prompt for AI Agents
In packages/ui/src/zag/Toast.jsx around lines 14 to 19, the createToaster config
includes an invalid offset: '16px' option; remove the offset property from the
configuration and, if needed, apply spacing via CSS/Tailwind or adjust
gap/placement/max instead so the createToaster call only uses valid keys
(placement, overlap, gap, max).

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
packages/ui/src/zag/Avatar.jsx (1)

24-30: Potential runtime error with whitespace-only names.

If name is a whitespace-only string (e.g., ' '), displayName.trim() produces an empty string, but the early return on line 26 won't catch it because the original string is truthy. This causes parts[0][0] to be undefined, and calling toUpperCase() on it throws a TypeError.

Proposed fix
 const getInitials = () => {
   const displayName = name();
   if (!displayName) return '';
-  const parts = displayName.trim().split(' ');
+  const trimmed = displayName.trim();
+  if (!trimmed) return '';
+  const parts = trimmed.split(' ');
   if (parts.length === 1) return parts[0][0].toUpperCase();
   return (parts[0][0] + parts[parts.length - 1][0]).toUpperCase();
 };
packages/ui/src/zag/__tests__/Tooltip.test.jsx (1)

36-57: Placement tests don't verify actual placement behavior.

Both placement tests only verify that the trigger text renders, but don't actually test whether the placement prop is applied correctly or affects the tooltip's position. The test names are misleading.

Consider either:

  1. Updating test names to reflect what's actually being tested (e.g., "should render with placement prop")
  2. Adding assertions to verify the placement prop is passed to the underlying Ark UI component (e.g., checking data attributes, computed styles, or using more specific queries)
Suggested approach to verify placement

If Ark UI exposes data attributes or ARIA attributes that indicate placement, verify those:

 it('should use default top placement', () => {
-  render(() => (
+  const { container } = render(() => (
     <Tooltip content='Tooltip text'>
       <button>Hover me</button>
     </Tooltip>
   ));

-  // Tooltip is rendered, placement is set via positioning prop
-  expect(screen.getByText('Hover me')).toBeInTheDocument();
+  expect(screen.getByText('Hover me')).toBeInTheDocument();
+  // Verify placement attribute or positioning data
+  const tooltipElement = container.querySelector('[data-placement]');
+  expect(tooltipElement).toHaveAttribute('data-placement', 'top');
 });

Alternatively, if placement verification isn't feasible with Ark UI's rendering, rename tests to accurately reflect what they verify.

packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx (1)

55-62: Standardize FileUpload event handlers across components.

Both onFileAccept and onFilesChange are supported by the FileUpload component (defined at packages/ui/src/zag/FileUpload.jsx). However, PdfEmptyState.jsx uses onFileAccept while PdfUploadSection.jsx and CreateLocalChecklist.jsx use onFilesChange. For consistency and maintainability, standardize on one approach across all three components. The simpler onFilesChange pattern (already used in two of three cases) is recommended for consistency.

packages/ui/src/zag/QRCode.jsx (1)

18-50: Remove redundant code and simplify unnecessary accessor indirection.

The component correctly maintains backward compatibility through the re-export alias in packages/ui/src/zag/index.js, and Ark UI migration is structurally sound. However, there are redundancies that should be cleaned up:

  • Lines 27 and 29: containerSize() duplicates pixelSize() identically—remove containerSize and use pixelSize() in the style block (line 40-41).
  • Lines 19 and 26: The multi-layer accessor pattern (props.datadata()value()) creates unnecessary indirection. Simplify to const value = () => props.data; or use props.data directly in the JSX.
packages/ui/package.json (1)

19-25: Fix @ark-ui/solid version constraint—5.30.0 does not exist.

The version specified as ^5.30.0 is incorrect. The latest available version of @ark-ui/solid is 5.29.1 (published November 22, 2025). Update the dependency to ^5.29.1 or compatible version to ensure the package resolves correctly.

The folder naming (./zag/* exports) is intentional despite containing Ark UI components; this preserves the existing API surface. Retaining @zag-js/solid and @zag-js/tour is appropriate for components like Tour that still depend on Zag.js.

♻️ Duplicate comments (9)
packages/ui/src/zag/Select.jsx (1)

120-124: The rotation class is still on the wrong element.

As previously noted, Ark UI sets the data-state attribute on the Select.Indicator element (line 121), not on child elements. The data-[state=open]:rotate-180 class on line 122 (the BiRegularChevronDown icon) will not match. Move this class to the Select.Indicator element.

Suggested fix
-        <Select.Indicator>
-          <BiRegularChevronDown class='h-5 w-5 text-gray-400 transition-transform data-[state=open]:rotate-180' />
+        <Select.Indicator class='transition-transform data-[state=open]:rotate-180'>
+          <BiRegularChevronDown class='h-5 w-5 text-gray-400' />
         </Select.Indicator>
packages/ui/src/zag/Editable.jsx (1)

6-6: Unused imports have been removed.

The previously flagged unused imports (createEffect and on) have been correctly removed from the import statement.

packages/ui/src/zag/Checkbox.jsx (1)

58-67: Address the past review: clarify the misleading comment.

The previous review correctly identified that the comment on line 60 is inaccurate. The code checks if the new state is indeterminate (details.checked === 'indeterminate'), not whether it's transitioning from indeterminate. The logic treats the indeterminate state as truthy/checked when converting to a boolean for the onChange callback.

Consider revising the comment to:

// Treat indeterminate state as checked (truthy) when converting to boolean for onChange
const newChecked = details.checked === true || details.checked === 'indeterminate';
packages/ui/src/zag/Toast.jsx (1)

14-19: Remove the invalid offset property from createToaster configuration.

The offset: '16px' property at line 18 is not a valid configuration option for Ark UI's createToaster API. Valid options are placement, overlap, gap, and max. This issue was previously flagged in past review comments but remains unresolved.

🔎 Proposed fix
 export const toaster = createToaster({
   placement: 'top-end',
   overlap: true,
   gap: 12,
-  offset: '16px',
 });

If you need spacing from the viewport edge, handle it via CSS padding on the Toaster container component or adjust the gap value.

packages/ui/src/zag/PasswordInput.jsx (1)

1-69: Migration to Ark UI looks good overall.

The component correctly uses Ark UI's PasswordInput composition pattern with proper SolidJS reactivity (props accessed via functions). The controlled input pattern with password() and onPasswordChange callback is well-implemented.

Note: The icon size inconsistency at lines 60-61 (iconSize() - 2 vs iconSize()) was previously flagged and should still be addressed.

packages/ui/src/zag/TagsInput.jsx (1)

50-54: JSDoc mismatch with implementation (previously flagged).

The handler forwards the entire Ark UI ValueChangeDetails object, but the JSDoc at line 17 documents a simpler signature. This was previously flagged and should be addressed by either updating the JSDoc or transforming the details object.

packages/ui/src/zag/Progress.jsx (1)

79-86: Missing CSS animation definition (previously flagged).

The animate-progress-indeterminate class referenced at line 82 requires a keyframes definition in your Tailwind config or global CSS. This was identified in a previous review.

packages/ui/src/zag/Dialog.jsx (2)

49-83: Remove the <Show> wrapper and use Ark UI's mounting props instead.

This issue was previously identified. The <Show when={open()}> wrapper conflicts with Ark UI's internal state machine. Ark UI's Dialog.Root provides lazyMount and unmountOnExit props for controlling mount/unmount behavior, which should be used instead:

-    <ArkDialog.Root open={open()} onOpenChange={handleOpenChange}>
-      <Show when={open()}>
-        <Portal>
+    <ArkDialog.Root open={open()} onOpenChange={handleOpenChange} lazyMount unmountOnExit>
+      <Portal>
          ...
-        </Portal>
-      </Show>
+      </Portal>
     </ArkDialog.Root>

150-211: Apply the same fix to ConfirmDialogComponent.

Same issue as DialogComponent - remove the <Show> wrapper and add lazyMount and unmountOnExit to the ArkDialog.Root:

     <ArkDialog.Root
       open={open()}
       onOpenChange={handleOpenChange}
       role='alertdialog'
       closeOnInteractOutside={!loading()}
       closeOnEscape={!loading()}
+      lazyMount
+      unmountOnExit
     >
-      <Show when={open()}>
-        <Portal>
+      <Portal>
           ...
-        </Portal>
-      </Show>
+      </Portal>
     </ArkDialog.Root>
🧹 Nitpick comments (14)
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx (1)

19-24: Consider aligning initial state with the effect's default.

The initial state is false, but the effect immediately sets it to true when props.showSecondPanel is undefined. This causes an extra render cycle.

🔎 Proposed refinement
-  const [showSecondPanel, setShowSecondPanel] = createSignal(false);
+  const [showSecondPanel, setShowSecondPanel] = createSignal(props.showSecondPanel ?? true);
packages/ui/src/zag/Avatar.jsx (1)

36-45: Consider using clsx for class merging.

The template literal approach works, but since clsx is already a project dependency, using it would provide cleaner class merging and handle edge cases (like multiple spaces or undefined values) more gracefully.

Proposed improvement
+import { clsx } from 'clsx';
+
 // ...
 
 return (
-  <Avatar.Root onStatusChange={onStatusChange()} class={`overflow-hidden ${props.class || ''}`}>
+  <Avatar.Root onStatusChange={onStatusChange()} class={clsx('overflow-hidden', props.class)}>
     <Avatar.Fallback class={fallbackClass()}>{getInitials()}</Avatar.Fallback>
     <Avatar.Image
       alt={alt()}
       src={src()}
       referrerPolicy='no-referrer'
       class='h-full w-full object-cover'
     />
   </Avatar.Root>
 );
packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx (1)

15-26: Consider refactoring to reduce props count.

The component receives 10+ props, which exceeds the coding guideline of 1-5 props. Consider grouping related props into objects (e.g., checklistCallbacks, pdfCallbacks) or moving study-related data into a store or SolidJS context to improve maintainability.

As per coding guidelines: "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."

packages/ui/src/zag/Tooltip.jsx (1)

70-84: DOM query in createMemo runs only once at mount time.

getBoundary uses document.querySelector inside createMemo, which isn't reactive—it captures the navbar's position at component creation and won't update if the navbar moves or loads later. If the navbar isn't in the DOM when the tooltip mounts (e.g., lazy-loaded), the boundary defaults to 'viewport' permanently.

If this is intentional (compute-once behavior), consider adding a brief comment. If dynamic updates are needed, use an effect with a resize/scroll listener or query the DOM in a getElement callback that Ark UI's positioning might support.

packages/ui/src/zag/Select.jsx (1)

66-66: Consider using a z-index constant for consistency.

The hardcoded z-10 class could be replaced with a Z_INDEX constant for consistency with other components like Menu.jsx which uses Z_INDEX.MENU.

Suggested approach

Import and use a shared z-index constant:

+import { Z_INDEX } from '../constants'; // or wherever Z_INDEX is defined
+
 const renderContent = () => (
-  <Select.Positioner class={inDialog() ? 'absolute top-full right-0 left-0 z-10' : ''}>
+  <Select.Positioner class={inDialog() ? `absolute top-full right-0 left-0 ${Z_INDEX.SELECT}` : ''}>
packages/ui/src/zag/__tests__/Tooltip.test.jsx (1)

109-139: Props tests don't verify prop behavior.

The tests for openDelay, closeDelay, and interactive props only verify that the component renders without errors when these props are provided. They don't verify that the props actually affect the tooltip's behavior.

While testing timing-based and interaction behaviors can be challenging in unit tests, consider:

  1. Adding comments to clarify these are smoke tests only
  2. Verifying props are passed through to the underlying Ark UI component (if possible via data attributes or refs)
  3. Creating integration tests that verify the actual behavior
  4. At minimum, document that behavioral testing is covered elsewhere
Example: Document test limitations
 it('should accept openDelay prop', () => {
   render(() => (
     <Tooltip content='Tooltip text' openDelay={500}>
       <button>Hover me</button>
     </Tooltip>
   ));

+  // Note: This is a smoke test. Actual delay behavior should be tested in integration tests.
   expect(screen.getByText('Hover me')).toBeInTheDocument();
 });
packages/ui/src/zag/Editable.jsx (1)

128-131: Simplify the value prop assignment.

Line 129 contains redundant conditional logic. When value() is undefined, passing undefined to a prop is equivalent to not passing it at all, so the ternary is unnecessary.

Proposed simplification
    <Editable.Root
-      value={value() !== undefined ? value() : undefined}
+      value={value()}
      defaultValue={value() === undefined ? editableValue() : undefined}
      placeholder={placeholder()}
packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx (1)

139-144: LGTM! Improved file type acceptance.

The addition of .pdf extension alongside the MIME type enhances cross-browser compatibility, particularly for Safari.

Consider adding explicit directory control for consistency

For consistency with PdfUploadSection.jsx (line 27) and to be explicit about the intent, consider adding allowDirectories={false}:

 <FileUpload
   accept='application/pdf,.pdf'
+  allowDirectories={false}
   helpText='PDF files only'
   showFileList={false}
   onFilesChange={handleFilesChange}
 />
packages/ui/src/zag/FloatingPanel.jsx (1)

41-41: Simplify the export pattern to avoid confusion.

The component uses both a default export (FloatingPanelComponent) and a named export (FloatingPanel). This dual-export pattern with different names can be confusing. Since the type definition in packages/ui/src/index.d.ts (line 479) expects a named export of FloatingPanel, consider removing the default export or renaming the function to match the exported name.

Proposed simplification
-export default function FloatingPanelComponent(props) {
+function FloatingPanelComponent(props) {
   // ... rest of the implementation
 }
 
 export { FloatingPanelComponent as FloatingPanel };

Or alternatively, rename the function to match:

-export default function FloatingPanelComponent(props) {
+function FloatingPanel(props) {
   // ... rest of the implementation
 }
 
-export { FloatingPanelComponent as FloatingPanel };
+export { FloatingPanel };

Also applies to: 182-182

packages/ui/src/zag/Switch.jsx (1)

49-55: Props spread order is correct but could be cleaner.

The spread {...machineProps} at line 50 includes onCheckedChange which is then overridden by the explicit onCheckedChange={handleCheckedChange} at line 55. While this works correctly (explicit props override spread), consider excluding onCheckedChange from the spread via splitProps to make the intent clearer and avoid passing it twice.

Optional: Exclude onCheckedChange from spread
-  const [local, machineProps] = splitProps(merged, ['class', 'onChange']);
+  const [local, machineProps] = splitProps(merged, ['class', 'onChange', 'onCheckedChange']);

Then explicitly handle both callbacks in handleCheckedChange (which you already do).

packages/ui/src/zag/Menu.jsx (1)

83-97: Redundant closeOnSelect prop on both Root and Item.

closeOnSelect is specified both on Menu.Root (line 111) and on each Menu.Item (line 86). While not harmful, this is redundant. Consider removing it from individual items unless you need per-item override behavior.

Remove redundant closeOnSelect from Menu.Item
              <Menu.Item
                value={item.value}
                disabled={item.disabled}
-               closeOnSelect={machineProps.closeOnSelect ?? true}
                class={`flex cursor-pointer items-center gap-2 px-3 py-2 text-sm transition-colors ${
packages/ui/src/zag/Popover.jsx (1)

71-73: Consider memoizing the positioning object.

The positioning accessor creates a new object on every access. While this works, it could cause unnecessary re-renders in some scenarios. Consider using createMemo for consistency with other derived values.

Optional: Memoize positioning
+import { mergeProps, splitProps, Show, createMemo } from 'solid-js';
...
-  const positioning = () => ({
-    placement: machineProps.placement || 'bottom',
-  });
+  const positioning = createMemo(() => ({
+    placement: machineProps.placement || 'bottom',
+  }));
packages/ui/src/zag/Collapsible.jsx (1)

56-69: Extract duplicated handleTriggerClick to avoid repetition.

The handleTriggerClick function is duplicated in both CollapsibleWithApi (lines 56-69) and CollapsibleComponent (lines 120-133). Extract it to a shared utility or pass it as a prop to reduce duplication.

Suggested refactor
+// Shared handler to prevent toggling when clicking interactive elements
+const createTriggerClickHandler = () => e => {
+  const target = e.target;
+  const interactive = target.closest(
+    'button:not([data-part="trigger"]), [role="button"]:not([data-part="trigger"]), [role="menuitem"], input, textarea, [data-editable], [data-scope="menu"], [data-scope="editable"], [data-selectable], a',
+  );
+  if (interactive) {
+    e.stopPropagation();
+    if (!interactive.matches('input, textarea')) {
+      e.preventDefault();
+    }
+  }
+};

 function CollapsibleWithApi(props) {
   // ...
-  const handleTriggerClick = e => {
-    const target = e.target;
-    const interactive = target.closest(
-      'button:not([data-part="trigger"]), [role="button"]:not([data-part="trigger"]), [role="menuitem"], input, textarea, [data-editable], [data-scope="menu"], [data-scope="editable"], [data-selectable], a',
-    );
-    if (interactive) {
-      e.stopPropagation();
-      if (!interactive.matches('input, textarea')) {
-        e.preventDefault();
-      }
-    }
-  };
+  const handleTriggerClick = createTriggerClickHandler();
   // ...
 }

 export default function CollapsibleComponent(props) {
   // ...
-  const handleTriggerClick = e => {
-    // ... duplicate code
-  };
+  const handleTriggerClick = createTriggerClickHandler();
   // ...
 }

Also applies to: 120-133

packages/ui/src/index.d.ts (1)

217-223: Consider adding stricter types for nested components.

The nested component types use any, which provides flexibility but loses type safety. Consider defining specific prop interfaces for Root, Trigger, Content, Indicator, and RootProvider in a future iteration.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3d8f99 and 35b35c9.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (47)
  • package.json
  • packages/ui/package.json
  • packages/ui/src/index.d.ts
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/NumberInput.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/QRCode.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/index.js
  • packages/ui/vitest.config.js
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
  • packages/web/src/global.css
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/web/src/components/sidebar/ProjectTreeItem.jsx
  • packages/ui/vitest.config.js
  • packages/ui/src/zag/NumberInput.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/src/global.css
  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/ui/src/zag/QRCode.jsx
  • package.json
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/ui/package.json
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
  • packages/ui/src/index.d.ts
  • packages/ui/src/zag/index.js
packages/web/**/*.{js,jsx,ts,tsx,css}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Ensure browser compatibility for all frontend code (Safari is usually problematic)

Files:

  • packages/web/src/global.css
  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
packages/web/src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
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
Use createMemo for derived values to ensure they update reactively

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components
Each file should handle one coherent responsibility
Use Zod for schema and input validation

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/ui/src/zag/QRCode.jsx
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
  • packages/ui/src/index.d.ts
  • packages/ui/src/zag/index.js
packages/web/src/components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/components/**/*.{js,jsx,ts,tsx}: Use responsive design principles for UI components
Group related components in subdirectories with an index.js barrel export
Use Zag.js for UI components and design system
Zag component exist in packages/web/src/components/zag/* and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
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
Do not destructure props in SolidJS components 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. They should not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Group related components in subdirectories with an index.js barrel export

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
**/*.{js,jsx,ts,tsx,md,json,sql,txt}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Do not use emojis in code, comments, documentation, plan files, or commit messages

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/ui/src/zag/QRCode.jsx
  • package.json
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/ui/package.json
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
  • packages/ui/src/index.d.ts
  • packages/ui/src/zag/index.js
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

**/*.{jsx,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Use responsive design principles for UI components

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/ui/src/zag/QRCode.jsx
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
packages/web/src/components/**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

packages/web/src/components/**/*.{jsx,tsx}: Use Zag.js for UI components and design system
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
Do NOT destructure props in SolidJS components - access props directly from the props object or wrap in a function to maintain reactivity
Components should be lean and focused - do not implement business logic; move it into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
packages/web/src/**/*.{jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Use createMemo for derived values and createStore for complex state in SolidJS

Files:

  • packages/web/src/components/checklist-ui/ScoreTag.jsx
  • packages/web/src/components/checklist-ui/pdf/PdfEmptyState.jsx
  • packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx
  • packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx
  • packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx
  • packages/web/src/components/project-ui/all-studies-tab/study-card/StudyCard.jsx
  • packages/web/src/components/project-ui/add-studies/PdfUploadSection.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/QRCode.jsx
  • package.json
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/ui/package.json
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/PinInput.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/ui/src/zag/__tests__/Tooltip.test.jsx
  • packages/ui/src/zag/Avatar.jsx
  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/QRCode.jsx
  • package.json
  • packages/ui/src/zag/FloatingPanel.jsx
  • packages/ui/package.json
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/Toast.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Popover.jsx
  • packages/ui/src/zag/RadioGroup.jsx
  • packages/ui/src/zag/Drawer.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/**/*.{jsx,tsx,ts} : Use `createMemo` for derived values and `createStore` for complex state in SolidJS

Applied to files:

  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/Editable.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Do not destructure props in SolidJS components 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/ui/src/zag/PasswordInput.jsx
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/ui/package.json
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/web/src/components/sidebar/StudyTreeItem.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Do NOT destructure props in SolidJS components - access props directly from the props object or wrap in a function to maintain reactivity

Applied to files:

  • packages/ui/src/zag/PasswordInput.jsx
  • packages/ui/src/zag/Editable.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging

Applied to files:

  • packages/ui/src/zag/TagsInput.jsx
  • packages/ui/src/zag/Tour.jsx
  • packages/ui/src/zag/Select.jsx
  • packages/ui/src/zag/Progress.jsx
  • packages/ui/src/zag/QRCode.jsx
  • package.json
  • packages/ui/package.json
  • packages/ui/src/zag/Collapsible.jsx
  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Dialog.jsx
  • packages/ui/src/zag/Tabs.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Accordion.jsx
  • packages/ui/src/zag/Switch.jsx
  • packages/ui/src/zag/Editable.jsx
  • packages/ui/src/zag/ToggleGroup.jsx
  • packages/ui/src/zag/Menu.jsx
  • packages/ui/src/zag/Clipboard.jsx
  • packages/ui/src/zag/Checkbox.jsx
  • packages/ui/src/zag/Splitter.jsx
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use responsive design principles for UI components

Applied to files:

  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{jsx,tsx} : Use responsive design principles for UI components

Applied to files:

  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/workers/src/**/*.{js,ts} : Use Drizzle ORM for database interactions and migrations

Applied to files:

  • package.json
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/workers/**/*.{js,ts,sql} : Use Drizzle ORM for database interactions and migrations

Applied to files:

  • package.json
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : 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

Applied to files:

  • packages/ui/src/zag/FileUpload.jsx
  • packages/ui/src/zag/Editable.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Use aliases for imports when appropriate to improve readability

Applied to files:

  • packages/ui/src/zag/Editable.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Components should be lean and focused - do not implement business logic; move it into stores, utilities, or primitives

Applied to files:

  • packages/ui/src/zag/Editable.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components

Applied to files:

  • packages/ui/src/zag/Splitter.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components

Applied to files:

  • packages/ui/src/zag/Splitter.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Group related components in subdirectories with an `index.js` barrel export

Applied to files:

  • packages/ui/src/zag/index.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Group related components in subdirectories with an index.js barrel export

Applied to files:

  • packages/ui/src/zag/index.js
🧬 Code graph analysis (17)
packages/ui/src/zag/__tests__/Tooltip.test.jsx (1)
packages/ui/src/index.d.ts (1)
  • Tooltip (940-940)
packages/ui/src/zag/Avatar.jsx (1)
packages/ui/src/index.d.ts (1)
  • Avatar (74-74)
packages/ui/src/zag/PasswordInput.jsx (1)
packages/ui/src/index.d.ts (1)
  • PasswordInput (583-583)
packages/web/src/components/project-ui/overview-tab/ReviewerAssignment.jsx (2)
packages/ui/src/zag/Dialog.jsx (3)
  • open (23-23)
  • open (102-102)
  • open (237-249)
packages/ui/src/zag/Drawer.jsx (1)
  • open (27-27)
packages/ui/src/zag/Select.jsx (2)
packages/ui/src/zag/Popover.jsx (1)
  • inDialog (62-62)
packages/ui/src/zag/Menu.jsx (1)
  • renderContent (61-103)
packages/ui/src/zag/QRCode.jsx (11)
packages/ui/src/zag/Dialog.jsx (1)
  • size (24-24)
packages/ui/src/zag/Drawer.jsx (1)
  • size (29-29)
packages/ui/src/zag/Checkbox.jsx (2)
  • classValue (36-36)
  • value (44-44)
packages/ui/src/zag/Editable.jsx (2)
  • classValue (100-100)
  • value (86-86)
packages/ui/src/zag/FileUpload.jsx (1)
  • classValue (48-48)
packages/ui/src/zag/PasswordInput.jsx (1)
  • classValue (28-28)
packages/ui/src/zag/Popover.jsx (1)
  • classValue (63-63)
packages/ui/src/zag/Switch.jsx (1)
  • classValue (31-31)
packages/ui/src/zag/Avatar.jsx (1)
  • alt (21-21)
packages/ui/src/zag/Select.jsx (1)
  • value (27-27)
packages/ui/src/zag/Tabs.jsx (1)
  • value (18-18)
packages/ui/src/zag/FloatingPanel.jsx (4)
packages/ui/src/zag/Dialog.jsx (3)
  • handleOpenChange (29-33)
  • handleOpenChange (110-114)
  • handleOpenChange (269-273)
packages/ui/src/zag/Drawer.jsx (1)
  • handleOpenChange (36-40)
packages/ui/src/zag/Popover.jsx (1)
  • handleOpenChange (65-69)
packages/ui/src/index.d.ts (1)
  • FloatingPanel (480-480)
packages/ui/src/zag/Toast.jsx (2)
packages/ui/src/index.d.ts (2)
  • toaster (878-884)
  • Toaster (876-876)
packages/ui/src/constants/zIndex.js (2)
  • Z_INDEX (15-44)
  • Z_INDEX (15-44)
packages/ui/src/zag/FileUpload.jsx (2)
packages/ui/src/primitives/useWindowDrag.js (2)
  • useWindowDrag (9-47)
  • isDraggingOverWindow (10-10)
packages/ui/src/index.d.ts (1)
  • FileUpload (413-413)
packages/ui/src/zag/Drawer.jsx (5)
packages/ui/src/zag/Dialog.jsx (6)
  • handleOpenChange (29-33)
  • handleOpenChange (110-114)
  • handleOpenChange (269-273)
  • open (23-23)
  • open (102-102)
  • open (237-249)
packages/ui/src/zag/FloatingPanel.jsx (1)
  • handleOpenChange (48-52)
packages/web/src/lib/form-errors.js (1)
  • details (23-23)
packages/ui/src/index.d.ts (1)
  • Dialog (295-295)
packages/ui/src/constants/zIndex.js (2)
  • Z_INDEX (15-44)
  • Z_INDEX (15-44)
packages/ui/src/zag/Combobox.jsx (1)
packages/ui/src/constants/zIndex.js (2)
  • Z_INDEX (15-44)
  • Z_INDEX (15-44)
packages/ui/src/zag/Accordion.jsx (3)
packages/ui/src/zag/Editable.jsx (1)
  • handleValueChange (106-110)
packages/ui/src/zag/Tabs.jsx (1)
  • handleValueChange (23-27)
packages/ui/src/index.d.ts (1)
  • Accordion (53-53)
packages/ui/src/zag/Switch.jsx (5)
packages/ui/src/zag/Checkbox.jsx (9)
  • merged (26-31)
  • local (33-33)
  • classValue (36-36)
  • onChange (38-38)
  • checked (40-40)
  • defaultChecked (41-41)
  • disabled (42-42)
  • name (43-43)
  • handleCheckedChange (58-67)
packages/ui/src/zag/Popover.jsx (3)
  • merged (33-43)
  • local (45-54)
  • classValue (63-63)
packages/ui/src/zag/Tooltip.jsx (2)
  • merged (39-54)
  • local (56-63)
packages/ui/src/zag/Accordion.jsx (1)
  • local (23-23)
packages/ui/src/zag/QRCode.jsx (1)
  • classValue (22-22)
packages/ui/src/zag/Clipboard.jsx (3)
packages/web/src/components/profile-ui/TwoFactorSetup.jsx (1)
  • copied (25-25)
packages/ui/src/zag/Tabs.jsx (1)
  • handleValueChange (23-27)
packages/ui/src/index.d.ts (1)
  • Clipboard (147-147)
packages/ui/src/zag/PinInput.jsx (6)
packages/ui/src/zag/Clipboard.jsx (1)
  • handleValueChange (34-38)
packages/ui/src/zag/Combobox.jsx (2)
  • handleValueChange (76-80)
  • merged (38-44)
packages/ui/src/zag/Tabs.jsx (1)
  • handleValueChange (23-27)
packages/ui/src/zag/Tooltip.jsx (1)
  • merged (39-54)
packages/ui/src/index.d.ts (1)
  • PinInput (604-604)
packages/ui/src/zag/PasswordInput.jsx (2)
  • required (25-25)
  • inputClass (26-26)
packages/ui/src/zag/Checkbox.jsx (1)
packages/ui/src/zag/Switch.jsx (9)
  • merged (22-27)
  • local (29-29)
  • classValue (31-31)
  • onChange (32-32)
  • checked (34-34)
  • defaultChecked (35-35)
  • disabled (36-36)
  • name (37-37)
  • handleCheckedChange (39-46)
packages/ui/src/zag/Splitter.jsx (1)
packages/ui/src/index.d.ts (1)
  • Splitter (770-770)
🪛 Biome (2.1.2)
packages/ui/src/zag/Collapsible.jsx

[error] 159-159: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

⏰ 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

Comment thread package.json
"@eslint/js": "^9.39.2",
"@typescript-eslint/parser": "^8.50.0",
"concurrently": "^9.2.1",
"dotenv": "^17.2.3",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the latest dotenv version on npm
echo "=== Latest dotenv version ===" 
curl -s https://registry.npmjs.org/dotenv/latest 2>/dev/null | jq -r '.version // "Failed to fetch"'

# Search for dotenv usage in the codebase
echo ""
echo "=== Searching for dotenv/ENV usage ===" 
rg -n "require.*dotenv|import.*dotenv|process\.env" --type js --type ts --type jsx --type tsx -C2 -g '!node_modules'

# Check package.json scripts
echo ""
echo "=== Checking package.json scripts ===" 
rg -A50 '"scripts"' package.json | head -60

# Look for .env references
echo ""
echo "=== Searching for .env references ===" 
rg -n "\.env|config\(|dotenv" --type md --type txt --type json -C1 -g '!node_modules' | head -40

Repository: InfinityBowman/corates

Length of output: 4423


🏁 Script executed:

#!/bin/bash
# Search for dotenv in sub-package directories
echo "=== Searching for dotenv in sub-packages ===" 
rg -n "dotenv" packages/ --type json --type js --type ts -C1 -g '!node_modules'

# Check if dotenv exists in sub-package package.json files
echo ""
echo "=== Checking sub-package package.json files for dotenv ===" 
find packages -name "package.json" -type f | xargs grep -l "dotenv" 2>/dev/null || echo "No dotenv found in sub-packages"

# Check root .env files
echo ""
echo "=== Checking for .env files in root ===" 
find . -maxdepth 1 -name ".env*" -type f 2>/dev/null | head -20

Repository: InfinityBowman/corates

Length of output: 243


Remove unused dotenv from devDependencies.

The dotenv package is not imported or used anywhere in the codebase. No dotenv references exist in root or sub-packages (landing, web, workers, mcp), and there are no .env files in the root. While process.env is used in the code, it does not require the dotenv package.

🤖 Prompt for AI Agents
In package.json around line 49, the devDependency "dotenv": "^17.2.3" is unused;
remove the "dotenv" entry from devDependencies in package.json, update the
lockfile by running npm install or yarn install (depending on the project), and
commit the updated package.json and lockfile so the dependency is fully removed
from the repository.

Comment on lines 60 to 69
it('should wrap trigger in span', () => {
const { container } = render(() => (
render(() => (
<Tooltip content='Tooltip text'>
<button>Hover me</button>
</Tooltip>
));

const triggerWrapper = container.querySelector('[data-part="trigger"]');
expect(triggerWrapper.tagName).toBe('SPAN');
// Ark UI renders trigger content - verify it's accessible
expect(screen.getByText('Hover me')).toBeInTheDocument();
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Test name doesn't match what's being verified.

The test is named "should wrap trigger in span" but only verifies that the trigger text is accessible in the document. It doesn't actually verify the span wrapper exists or that the trigger is properly wrapped.

Either:

  1. Rename the test to match what it verifies: "should render trigger content accessibly"
  2. Add assertions to verify the span wrapper structure if it's important for the component's behavior
Option to verify span wrapper
 it('should wrap trigger in span', () => {
-  render(() => (
+  const { container } = render(() => (
     <Tooltip content='Tooltip text'>
       <button>Hover me</button>
     </Tooltip>
   ));

-  // Ark UI renders trigger content - verify it's accessible
   expect(screen.getByText('Hover me')).toBeInTheDocument();
+  // Verify the trigger is wrapped appropriately
+  const button = screen.getByText('Hover me');
+  expect(button.parentElement?.tagName).toBe('SPAN');
 });
🤖 Prompt for AI Agents
In packages/ui/src/zag/__tests__/Tooltip.test.jsx around lines 60 to 69, the
test is named "should wrap trigger in span" but only asserts the trigger text is
present; either rename the test to "should render trigger content accessibly" or
add assertions to verify the span wrapper (e.g., locate the trigger element and
assert its parent is a span with expected attributes/class). Update the test
name if you choose renaming, or add the wrapper checks if the span structure is
required for behavior.

Comment on lines 96 to 107
describe('Accessibility', () => {
it('should have trigger with proper attributes', () => {
const { container } = render(() => (
render(() => (
<Tooltip content='Tooltip text'>
<button>Hover me</button>
</Tooltip>
));

const trigger = container.querySelector('[data-part="trigger"]');
expect(trigger).toBeInTheDocument();
// Trigger is rendered with proper attributes via Ark UI
expect(screen.getByText('Hover me')).toBeInTheDocument();
});
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Accessibility test doesn't verify any accessibility attributes.

The test claims to verify "trigger with proper attributes" but only checks that the trigger text is rendered. It doesn't verify any accessibility attributes such as aria-describedby, role, or other ARIA properties that are crucial for accessibility.

This creates a gap in test coverage for an important aspect of the component.

Suggested accessibility assertions
 it('should have trigger with proper attributes', () => {
   render(() => (
     <Tooltip content='Tooltip text'>
       <button>Hover me</button>
     </Tooltip>
   ));

-  // Trigger is rendered with proper attributes via Ark UI
-  expect(screen.getByText('Hover me')).toBeInTheDocument();
+  const trigger = screen.getByText('Hover me');
+  expect(trigger).toBeInTheDocument();
+  
+  // Verify accessibility attributes
+  expect(trigger).toHaveAttribute('aria-describedby');
+  // Or verify other relevant ARIA attributes based on Ark UI's implementation
 });
🤖 Prompt for AI Agents
In packages/ui/src/zag/__tests__/Tooltip.test.jsx around lines 96 to 107, the
test title says it verifies "trigger with proper attributes" but it only checks
that the trigger text renders; update the test to assert real accessibility
attributes: grab the trigger button via getByText and assert it has
aria-describedby (or aria-controls) pointing to the tooltip id and that
aria-expanded or aria-hidden reflects the tooltip visibility; then open the
tooltip (e.g., fireEvent.mouseOver or focus) and assert the tooltip element
exists with role="tooltip" and an id that matches the trigger's
aria-describedby, and assert the tooltip's visibility attribute/state; these
assertions will verify proper ARIA linkage between trigger and tooltip.

Comment on lines +40 to +48
// Create API object for render prop compatibility
const api = createMemo(() => ({
get copied() {
return copied();
},
copy: () => {
// Trigger copy via the Clipboard.Trigger
},
}));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The api.copy function is a no-op, breaking render prop pattern.

The api object exposed to render prop consumers includes a copy function that does nothing. If consumers call api.copy(), nothing will happen. Either remove it or provide a working implementation using a ref to the trigger.

Consider removing non-functional copy method
   // Create API object for render prop compatibility
   const api = createMemo(() => ({
     get copied() {
       return copied();
     },
-    copy: () => {
-      // Trigger copy via the Clipboard.Trigger
-    },
   }));

If copy() functionality is needed, you'll need to use a ref or the useClipboard hook from Ark UI for programmatic control.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Create API object for render prop compatibility
const api = createMemo(() => ({
get copied() {
return copied();
},
copy: () => {
// Trigger copy via the Clipboard.Trigger
},
}));
// Create API object for render prop compatibility
const api = createMemo(() => ({
get copied() {
return copied();
},
}));
🤖 Prompt for AI Agents
In packages/ui/src/zag/Clipboard.jsx around lines 40 to 48, the api.copy method
is currently a no-op which breaks the render-prop contract; either remove the
non-functional copy export or implement it to trigger the actual copy action via
the trigger ref or programmatic clipboard helper. Fix by wiring a ref (e.g.,
triggerRef) to the Clipboard.Trigger element and make api.copy invoke the
trigger action (for example call triggerRef.current?.click() or the equivalent
trigger function), or, alternatively, use the useClipboard/programmatic copy
helper and have api.copy call that function so consumers can call api.copy() and
it performs the copy operation. Ensure the trigger ref or helper is defined in
the same scope as createMemo so the returned api.copy is functional.

Comment thread packages/ui/src/zag/Combobox.jsx Outdated
Comment on lines +59 to +61
<RadioGroup.ItemControl class='flex h-4 w-4 items-center justify-center rounded-full border-2 border-gray-300 transition-colors group-hover:border-gray-400 data-[disabled]:cursor-not-allowed data-[disabled]:opacity-50 data-[focus]:ring-2 data-[focus]:ring-blue-500 data-[focus]:ring-offset-2 data-[state=checked]:border-blue-500 data-[state=checked]:bg-blue-500 data-[state=checked]:group-hover:border-blue-600'>
<div class='h-1.5 w-1.5 scale-0 rounded-full bg-white transition-transform data-[state=checked]:scale-100' />
</RadioGroup.ItemControl>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inner indicator div won't receive data attributes.

The inner div at line 60 has data-[state=checked]:scale-100 styling, but data attributes are applied to RadioGroup.ItemControl, not its children. Use the parent's data attribute with group selectors.

Proposed fix
-                <RadioGroup.ItemControl class='flex h-4 w-4 items-center justify-center rounded-full border-2 border-gray-300 transition-colors group-hover:border-gray-400 data-[disabled]:cursor-not-allowed data-[disabled]:opacity-50 data-[focus]:ring-2 data-[focus]:ring-blue-500 data-[focus]:ring-offset-2 data-[state=checked]:border-blue-500 data-[state=checked]:bg-blue-500 data-[state=checked]:group-hover:border-blue-600'>
-                  <div class='h-1.5 w-1.5 scale-0 rounded-full bg-white transition-transform data-[state=checked]:scale-100' />
+                <RadioGroup.ItemControl class='group/control flex h-4 w-4 items-center justify-center rounded-full border-2 border-gray-300 transition-colors group-hover:border-gray-400 data-[disabled]:cursor-not-allowed data-[disabled]:opacity-50 data-[focus]:ring-2 data-[focus]:ring-blue-500 data-[focus]:ring-offset-2 data-[state=checked]:border-blue-500 data-[state=checked]:bg-blue-500 data-[state=checked]:group-hover:border-blue-600'>
+                  <div class='h-1.5 w-1.5 scale-0 rounded-full bg-white transition-transform group-data-[state=checked]/control:scale-100' />
                 </RadioGroup.ItemControl>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<RadioGroup.ItemControl class='flex h-4 w-4 items-center justify-center rounded-full border-2 border-gray-300 transition-colors group-hover:border-gray-400 data-[disabled]:cursor-not-allowed data-[disabled]:opacity-50 data-[focus]:ring-2 data-[focus]:ring-blue-500 data-[focus]:ring-offset-2 data-[state=checked]:border-blue-500 data-[state=checked]:bg-blue-500 data-[state=checked]:group-hover:border-blue-600'>
<div class='h-1.5 w-1.5 scale-0 rounded-full bg-white transition-transform data-[state=checked]:scale-100' />
</RadioGroup.ItemControl>
<RadioGroup.ItemControl class='group/control flex h-4 w-4 items-center justify-center rounded-full border-2 border-gray-300 transition-colors group-hover:border-gray-400 data-[disabled]:cursor-not-allowed data-[disabled]:opacity-50 data-[focus]:ring-2 data-[focus]:ring-blue-500 data-[focus]:ring-offset-2 data-[state=checked]:border-blue-500 data-[state=checked]:bg-blue-500 data-[state=checked]:group-hover:border-blue-600'>
<div class='h-1.5 w-1.5 scale-0 rounded-full bg-white transition-transform group-data-[state=checked]/control:scale-100' />
</RadioGroup.ItemControl>
🤖 Prompt for AI Agents
In packages/ui/src/zag/RadioGroup.jsx around lines 59 to 61, the inner div's
data-[state=checked]:scale-100 won't ever match because data attributes are on
the parent ItemControl; change the child's selector to use the parent's group
state (e.g. replace data-[state=checked]:scale-100 with
group-data-[state=checked]:scale-100) so the inner indicator scales when the
parent has data-state="checked"; keep the parent's existing "group" class.

Comment on lines +7 to 25
export default function SplitterComponent() {
return (
<div {...api().getRootProps()}>
<div {...api().getPanelProps({ id: 'a' })}>
<Splitter.Root
panels={[
{ id: 'a', minSize: 10 },
{ id: 'b', minSize: 10 },
]}
defaultSize={[80, 20]}
>
<Splitter.Panel id='a'>
<p>A</p>
</div>
<div {...api().getResizeTriggerProps({ id: 'a:b' })} />
<div {...api().getPanelProps({ id: 'b' })}>
</Splitter.Panel>
<Splitter.ResizeTrigger id='a:b' aria-label='Resize' />
<Splitter.Panel id='b'>
<p>B</p>
</div>
</div>
</Splitter.Panel>
</Splitter.Root>
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Splitter implementation is hardcoded and not reusable.

The current implementation is a placeholder with hardcoded values and no props. A production-ready Splitter component should accept:

  • panels configuration (IDs, min/max sizes) as props
  • defaultSize or controlled size prop
  • children for custom panel content
  • orientation ('horizontal' | 'vertical')
  • Event handlers (onSizeChange, etc.)

The current hardcoded structure with <p>A</p> and <p>B</p> is not suitable for real usage.

🔎 Example of a properly prop-driven Splitter
-export default function SplitterComponent() {
+export default function SplitterComponent(props) {
   return (
     <Splitter.Root
-      panels={[
-        { id: 'a', minSize: 10 },
-        { id: 'b', minSize: 10 },
-      ]}
-      defaultSize={[80, 20]}
+      panels={props.panels}
+      defaultSize={props.defaultSize}
+      size={props.size}
+      onSizeChange={props.onSizeChange}
+      orientation={props.orientation || 'horizontal'}
     >
-      <Splitter.Panel id='a'>
-        <p>A</p>
-      </Splitter.Panel>
-      <Splitter.ResizeTrigger id='a:b' aria-label='Resize' />
-      <Splitter.Panel id='b'>
-        <p>B</p>
-      </Splitter.Panel>
+      {props.children}
     </Splitter.Root>
   );
 }

Then consumers would provide their own panels and content.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export default function SplitterComponent() {
return (
<div {...api().getRootProps()}>
<div {...api().getPanelProps({ id: 'a' })}>
<Splitter.Root
panels={[
{ id: 'a', minSize: 10 },
{ id: 'b', minSize: 10 },
]}
defaultSize={[80, 20]}
>
<Splitter.Panel id='a'>
<p>A</p>
</div>
<div {...api().getResizeTriggerProps({ id: 'a:b' })} />
<div {...api().getPanelProps({ id: 'b' })}>
</Splitter.Panel>
<Splitter.ResizeTrigger id='a:b' aria-label='Resize' />
<Splitter.Panel id='b'>
<p>B</p>
</div>
</div>
</Splitter.Panel>
</Splitter.Root>
);
}
export default function SplitterComponent(props) {
return (
<Splitter.Root
panels={props.panels}
defaultSize={props.defaultSize}
size={props.size}
onSizeChange={props.onSizeChange}
orientation={props.orientation || 'horizontal'}
>
{props.children}
</Splitter.Root>
);
}
🤖 Prompt for AI Agents
In packages/ui/src/zag/Splitter.jsx around lines 7 to 25, the component is
hardcoded with two fixed panels and static content which makes it non-reusable;
change it to accept props (panels: array of {id, minSize?, maxSize?},
defaultSize or controlled size, orientation, children or renderPanel callback,
and event handlers like onSizeChange), iterate over the panels prop to render
Splitter.Panel and Splitter.ResizeTrigger elements dynamically (matching
triggers between panels), support controlled/uncontrolled sizing by honoring
size/defaultSize and calling onSizeChange, forward remaining props and
className, and add sensible defaults and prop-types or TypeScript types for
validation.

Comment on lines 46 to 50
<Show when={tab.count !== undefined || tab.getCount}>
<span class='ml-1 rounded-full bg-gray-100 px-2 py-0.5 text-xs text-gray-600 data-selected:bg-blue-100 data-selected:text-blue-700'>
<span class='ml-1 rounded-full bg-gray-100 px-2 py-0.5 text-xs text-gray-600 data-[selected]:bg-blue-100 data-[selected]:text-blue-700'>
{tab.getCount ? tab.getCount() : tab.count}
</span>
</Show>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Data attribute styling may not apply to nested elements.

The data-[selected] classes on the count badge span (line 47) won't work as expected. In Ark UI, the data-selected attribute is applied to the Tabs.Trigger element itself, not propagated to child elements. You need to use parent-based selectors.

Proposed fix using group selector
-            <Tabs.Trigger
-              value={tab.value}
-              class='flex items-center gap-2 border-b-2 border-transparent px-4 py-3 text-sm font-medium whitespace-nowrap text-gray-600 transition-colors hover:bg-gray-50 hover:text-gray-900 data-[selected]:border-blue-600 data-[selected]:bg-blue-50/50 data-[selected]:text-blue-600'
+            <Tabs.Trigger
+              value={tab.value}
+              class='group flex items-center gap-2 border-b-2 border-transparent px-4 py-3 text-sm font-medium whitespace-nowrap text-gray-600 transition-colors hover:bg-gray-50 hover:text-gray-900 data-[selected]:border-blue-600 data-[selected]:bg-blue-50/50 data-[selected]:text-blue-600'
             >
               <Show when={tab.icon}>
                 <span class='h-4 w-4'>{tab.icon}</span>
               </Show>
               {tab.label}
               <Show when={tab.count !== undefined || tab.getCount}>
-                <span class='ml-1 rounded-full bg-gray-100 px-2 py-0.5 text-xs text-gray-600 data-[selected]:bg-blue-100 data-[selected]:text-blue-700'>
+                <span class='ml-1 rounded-full bg-gray-100 px-2 py-0.5 text-xs text-gray-600 group-data-[selected]:bg-blue-100 group-data-[selected]:text-blue-700'>
                   {tab.getCount ? tab.getCount() : tab.count}
                 </span>
               </Show>
             </Tabs.Trigger>

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In packages/ui/src/zag/Tabs.jsx around lines 46 to 50, the count badge uses
data-[selected] which won’t match because Ark sets data-selected on the
Tabs.Trigger (parent) not on children; add a grouping class to the trigger
(e.g., className="group" on Tabs.Trigger) and change the badge classes to use
parent-based selectors (e.g., replace data-[selected]:bg-blue-100 and
data-[selected]:text-blue-700 with group-data-[selected]:bg-blue-100 and
group-data-[selected]:text-blue-700) so the badge styles respond to the
trigger’s selected state.

Comment on lines +56 to +58
loopFocus={machineProps.loop ?? true}
rovingFocus={machineProps.rovingFocus ?? true}
deselectable={machineProps.deselectable ?? true}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "package.json" -type f | head -5 | xargs cat | grep -A 5 -B 5 "@ark-ui"

Repository: InfinityBowman/corates

Length of output: 318


🌐 Web query:

Ark UI solid ToggleGroup component API documentation props

💡 Result:

Here are the ToggleGroup (Solid) API props from Ark UI:

  1. Root props
  • asChild: boolean. Use the provided child as the rendered element.
  • defaultValue: string[] — initial selected value(s).
  • deselectable: boolean (default: true) — allow empty selection (ignored when multiple is true).
  • disabled: boolean.
  • id: string.
  • ids: Partial<{ root: string; item(value: string): string }>.
  • Data attributes: [data-scope]="toggle-group", [data-part]="root", [data-disabled], [data-orientation], [data-focus]. [1][2]
  1. Item props
  • value: string.
  • asChild: boolean.
  • disabled: boolean.
  • Data attributes: [data-part]="item", [data-scope]="toggle-group", [data-state]="on" | "off", [data-disabled], [data-focus], [data-orientation]. [1][2]
  1. RootProvider props
  • value: UseToggleGroupReturn (the object returned by useToggleGroup).
  • asChild: boolean. [1][2]
  1. Context / useToggleGroup (exposed API)
  • value: string[] — current value(s).
  • setValue: (value: string[]) => void.
  • getItemState: (props: ItemProps) => ItemState. [1]
  1. Keyboard accessibility (summary)
  • Tab, Space, Enter, Arrow keys, Home, End for focus/activation. [1][2]

Sources: Ark UI docs (Toggle Group). [1] Ark UI Solid docs — Toggle Group. [2] Ark UI React docs — Toggle Group.


🏁 Script executed:

cat -n packages/ui/src/zag/ToggleGroup.jsx | head -80

Repository: InfinityBowman/corates

Length of output: 3662


🏁 Script executed:

rg "loopFocus|rovingFocus" --type jsx --type ts --type js -B 2 -A 2

Repository: InfinityBowman/corates

Length of output: 93


🏁 Script executed:

rg "loopFocus|rovingFocus|loop.*focus" packages/ui/src/zag/ToggleGroup.jsx -B 2 -A 2

Repository: InfinityBowman/corates

Length of output: 826


🌐 Web query:

Ark UI 5.30.0 ToggleGroup Root component all props complete list

💡 Result:

Do you want the official prop list (TypeScript definitions) pulled from the Ark UI v5.30.0 source (I can fetch the package/GitHub and list every prop), or is the docs-level summary (asChild, defaultValue, plus usage notes like multiple) sufficient?


🏁 Script executed:

find . -path "*/node_modules/@ark-ui/solid/*toggle-group*" -type f 2>/dev/null | head -20

Repository: InfinityBowman/corates

Length of output: 48


🏁 Script executed:

rg "interface.*ToggleGroup|type.*ToggleGroup" . --type ts --type tsx 2>/dev/null | head -20

Repository: InfinityBowman/corates

Length of output: 48


🌐 Web query:

Ark UI @ark-ui/solid 5.30.0 npm ToggleGroup.Root TypeScript definitions interface

💡 Result:

I couldn't open the shipped .d.ts file here, but the ToggleGroup.Root props in @ark-ui/solid v5.30.0 are the same as the documented Root API: at minimum the Root accepts

  • asChild?: boolean
  • defaultValue?: string[]
  • deselectable?: boolean
  • disabled?: boolean
  • id?: string
  • ids?: Partial<{ root: string; item(value: string): string }>
  • (plus the RootProvider alternative: value: UseToggleGroupReturn)

For full, exact TypeScript declarations inspect the package's dist .d.ts file at runtime (./dist/components/toggle-group/index.d.ts) in the installed package or on the CDN (unpkg/jsdelivr). See Ark UI docs and the package.json that points to the types file. [1][2]

Sources:
[1] Ark UI Toggle Group docs (API / Root props).
[2] @ark-ui/solid package.json (exports → types: ./dist/components/*/index.d.ts).


Remove invalid props loopFocus and rovingFocus from ToggleGroup.Root.

These props don't exist in Ark UI v5.30.0's ToggleGroup.Root API (which only accepts asChild, defaultValue, deselectable, disabled, id, and ids). They will be silently ignored, breaking the intended focus navigation behavior documented in the component. Remove lines 56–57 or replace them with valid Ark UI props if equivalent functionality is needed.

🤖 Prompt for AI Agents
In packages/ui/src/zag/ToggleGroup.jsx around lines 56 to 58, the component is
passing non-existent props loopFocus and rovingFocus to ToggleGroup.Root (only
asChild, defaultValue, deselectable, disabled, id, ids are supported in Ark UI
v5.30.0); remove the two invalid props (lines 56–57) so ToggleGroup.Root only
receives supported props, or if you need equivalent focus behavior implement it
using the correct Ark API (or a wrapper that manages roving focus) rather than
passing these unsupported props.

Comment thread packages/ui/src/zag/Tooltip.jsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx (1)

19-24: Refactor to use createMemo to avoid initial flash and improve reactivity.

The current implementation initializes showSecondPanel to false (line 19), then immediately updates it to props.showSecondPanel ?? true in a createEffect (lines 22-24). Since createEffect runs after the initial render in SolidJS, this causes a two-phase render: the panel starts hidden, then appears on the next render cycle if the prop is undefined. This can result in a visible layout shift or flash.

As per coding guidelines, derived values should use createMemo to ensure reactive updates. Since showSecondPanel is derived directly from a prop, a memo is more appropriate.

🔎 Proposed refactor using createMemo
-  const [showSecondPanel, setShowSecondPanel] = createSignal(false);
-
-  // Sync showSecondPanel with prop changes
-  createEffect(() => {
-    setShowSecondPanel(props.showSecondPanel ?? true);
-  });
+  const showSecondPanel = createMemo(() => props.showSecondPanel ?? true);

Note: You'll also need to update line 74 in toggleSecondPanel() since this would make showSecondPanel read-only. Consider whether the toggle should be controlled (via a callback prop) or if local state is genuinely needed. If local state with prop sync is required, initialize the signal to props.showSecondPanel ?? true instead of false to avoid the flash.

Based on coding guidelines: "Use createMemo for derived values to ensure they update reactively."

♻️ Duplicate comments (1)
packages/ui/src/zag/Combobox.jsx (1)

133-137: onInputValueChange should be on Root, not Input.

This issue was flagged in a previous review: In Ark UI's Combobox, the onInputValueChange callback should be passed to ArkCombobox.Root, not ArkCombobox.Input. The Input component is just a styled input element and doesn't accept this callback.

Proposed fix

Move the onInputValueChange handler from Input to Root:

     <ArkCombobox.Root
       {...machineProps}
       collection={collection()}
       onValueChange={handleValueChange}
+      onInputValueChange={handleInputValueChange}
       class={`w-full ${local.class || ''}`}
     >
...
       <ArkCombobox.Input
         placeholder={local.placeholder}
         class={`flex-1 bg-transparent px-3 py-2 text-sm outline-none placeholder:text-gray-400 disabled:cursor-not-allowed ${local.inputClass || ''}`}
-        onInputValueChange={handleInputValueChange}
       />
🧹 Nitpick comments (9)
packages/ui/src/zag/Tooltip.jsx (1)

66-70: Consider removing redundant accessor wrapper functions.

In Solid.js, properties from splitProps are already reactive. Creating additional wrapper functions like const content = () => local.content; adds unnecessary indirection. You can use local.content, local.children, etc. directly in the JSX.

Proposed refactor to simplify accessor usage

Remove the wrapper functions and use local properties directly:

- const content = () => local.content;
- const children = () => local.children;
- const showArrow = () => local.showArrow;
- const placement = () => local.placement;
- const interactive = () => local.interactive;
-
  // Calculate boundary for positioning (avoid navbar overlap)
  const getBoundary = createMemo(() => {

Then update the JSX to use local properties directly:

      <ArkTooltip.Trigger>{children()}</ArkTooltip.Trigger>
+     <ArkTooltip.Trigger>{local.children}</ArkTooltip.Trigger>
-         <Show when={showArrow()}>
+         <Show when={local.showArrow}>
-           class={`data-[state=open]:animate-fade-in data-[state=closed]:animate-fade-out ${interactive() ? '' : 'pointer-events-none'} max-w-xs rounded bg-gray-900 px-2 py-1 text-xs text-white shadow-lg ${local.contentClass || ''}`}
+           class={`data-[state=open]:animate-fade-in data-[state=closed]:animate-fade-out ${local.interactive ? '' : 'pointer-events-none'} max-w-xs rounded bg-gray-900 px-2 py-1 text-xs text-white shadow-lg ${local.contentClass || ''}`}
-           {content()}
+           {local.content}

And update the positioning memo:

  const positioning = createMemo(() => ({
-   placement: placement(),
+   placement: local.placement,
packages/web/src/components/checklist-ui/pdf/pdfDocument.js (1)

59-66: Source deduplication based on byteLength alone may skip legitimate loads.

Two different PDF files with identical byte lengths would generate the same sourceId, potentially causing the second load to be skipped. Consider using a hash or combining with other properties for more robust deduplication.

Proposed improvement
-    const sourceId = source.data ? source.data.byteLength : JSON.stringify(source);
+    // Use byteLength + first few bytes for better uniqueness
+    const sourceId = source.data 
+      ? `${source.data.byteLength}-${new Uint8Array(source.data.slice(0, 16)).join('')}`
+      : JSON.stringify(source);
packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js (2)

13-13: blobUrl is tracked and revoked but never assigned.

The code revokes blobUrl in multiple places, but there's no code path that actually creates and assigns a blob URL. This appears to be dead code or an incomplete implementation.

Proposed fix - remove unused blobUrl handling or complete the implementation

If blob URLs are not needed:

 export function createPdfFileHandler(document, options = {}) {
-  let blobUrl = null;
   let fileInputRef = null;

   async function handleFile(file) {
     if (!file) return;

     if (file.type !== 'application/pdf') {
       document.setError('Please select a PDF file');
       return;
     }

-    if (blobUrl) {
-      URL.revokeObjectURL(blobUrl);
-      blobUrl = null;
-    }
-
     try {
   function clearPdf() {
-    if (blobUrl) {
-      URL.revokeObjectURL(blobUrl);
-      blobUrl = null;
-    }
-
     document.clearPdf();

Also applies to: 24-27, 52-55


19-22: Consider adding file extension fallback for PDF validation.

Some browsers or file systems may not correctly report the MIME type. Adding a file extension check as a fallback would improve robustness.

Proposed enhancement
-    if (file.type !== 'application/pdf') {
+    const isPdf = file.type === 'application/pdf' || file.name?.toLowerCase().endsWith('.pdf');
+    if (!isPdf) {
       document.setError('Please select a PDF file');
       return;
     }
packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js (2)

143-148: Storing timeout on the function object is unconventional but functional.

Using handleWheel.zoomEndTimeout to store the timeout ID works, but is an unusual pattern that could confuse maintainers. Consider using a module-scoped variable like the other timeout IDs.

Proposed refactor for consistency
+  let wheelZoomEndTimeout = null;
+
   // Handle pinch-to-zoom via wheel event with ctrlKey (trackpad gesture - Chrome/Firefox)
   function handleWheel(e) {
     // ... existing code ...
     
     // Clear wheel zoom flag after a short delay (when gesture ends)
-    clearTimeout(handleWheel.zoomEndTimeout);
-    handleWheel.zoomEndTimeout = setTimeout(() => {
+    clearTimeout(wheelZoomEndTimeout);
+    wheelZoomEndTimeout = setTimeout(() => {
       isWheelZooming = false;
     }, 150);
   }

And update cleanup:

-    if (handleWheel.zoomEndTimeout) {
-      clearTimeout(handleWheel.zoomEndTimeout);
-      handleWheel.zoomEndTimeout = null;
+    if (wheelZoomEndTimeout) {
+      clearTimeout(wheelZoomEndTimeout);
+      wheelZoomEndTimeout = null;
     }

356-396: Nested RAF + setTimeout pattern is fragile for DOM timing.

Using setTimeout(50ms) to wait for canvas rendering is a timing heuristic that may fail on slower devices or under heavy load. Consider using a more robust approach like ResizeObserver on the target page or polling for dimension changes.

This works for most cases, but if users report scroll jank during zoom, this would be a good area to revisit.

packages/web/src/components/checklist-ui/pdf/pdfRenderer.js (2)

27-28: Unused variables: prevContainerWidth, prevContainerHeight, and RESIZE_THRESHOLD.

These variables are declared but never used in the code. If resize-based re-rendering is intentionally disabled, consider removing these to reduce confusion.

Proposed cleanup
   let resizeRafId = null;
   let prevScale = null;
-  let prevContainerWidth = null;
-  let prevContainerHeight = null;

   const RENDER_CONCURRENCY = 3; // Limit concurrent renders to avoid overwhelming GPU
   const CANVAS_CLEAR_DELAY = 5000; // Clear canvas 5 seconds after page leaves viewport
   const canvasClearTimeouts = new Map(); // Track timeouts for clearing canvases
-  const RESIZE_THRESHOLD = 50; // Only re-render if container size changes by more than 50px

Also applies to: 33-33


35-56: ResizeObserver created but does nothing - consider removing or implementing.

The ResizeObserver is created with an empty callback, wasting browser resources. Either implement the "fit-to-width on resize" mentioned in the comment, or remove the observer entirely until needed.

Proposed fix - remove unused observer
   function setupResizeObserver(container) {
-    if (resizeObserver) {
-      resizeObserver.disconnect();
-    }
-
     containerRef = container;
-
-    // ResizeObserver is kept for potential future use (e.g., fit-to-width on resize)
-    // But we don't trigger re-renders here to prevent white flashes
-    resizeObserver = new ResizeObserver(() => {
-      // No-op: We don't re-render on resize to prevent white flashes
-      // The canvas will scale with CSS transforms
-    });
-
-    if (containerRef) {
-      resizeObserver.observe(containerRef);
-    }
   }

And in cleanup:

-    if (resizeObserver) {
-      resizeObserver.disconnect();
-    }
packages/web/src/components/checklist-ui/pdf/usePdfJs.js (1)

93-98: Redundant cancelAllRenders() call after cleanup().

renderer.cleanup() already calls cancelAllRenders() internally (see pdfRenderer.js line 441). The extra call is harmless but unnecessary.

Proposed fix
   onCleanup(() => {
     renderer.cleanup();
     scrollHandler.cleanup();
-    renderer.cancelAllRenders();
   });
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35b35c9 and 1d41fd4.

📒 Files selected for processing (8)
  • packages/ui/src/zag/Combobox.jsx
  • packages/ui/src/zag/Tooltip.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
🧰 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/ui/src/zag/Tooltip.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/ui/src/zag/Combobox.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability

**/*.{js,jsx,ts,tsx}: Prefer modern ES6+ syntax and features
Use aliases for imports when appropriate to improve readability
Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components
Each file should handle one coherent responsibility
Use Zod for schema and input validation

Files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/ui/src/zag/Combobox.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
**/*.{js,jsx,ts,tsx,md,json,sql,txt}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Do not use emojis in code, comments, documentation, plan files, or commit messages

Files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/ui/src/zag/Combobox.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

**/*.{jsx,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Use responsive design principles for UI components

Files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/Combobox.jsx
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
packages/web/src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/**/*.{js,jsx,ts,tsx}: For UI icons, use the solid-icons library or SVGs only. Do not use emojis
Ensure browser compatibility for all frontend code (Safari is usually problematic)
Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components
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
Use createMemo for derived values to ensure they update reactively

Files:

  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
packages/web/src/components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

packages/web/src/components/**/*.{js,jsx,ts,tsx}: Use responsive design principles for UI components
Group related components in subdirectories with an index.js barrel export
Use Zag.js for UI components and design system
Zag component exist in packages/web/src/components/zag/* and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
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
Do not destructure props in SolidJS components 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. They should not implement business logic; move that into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Group related components in subdirectories with an index.js barrel export

Files:

  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
packages/web/**/*.{js,jsx,ts,tsx,css}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Ensure browser compatibility for all frontend code (Safari is usually problematic)

Files:

  • packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js
  • packages/web/src/components/checklist-ui/pdf/pdfDocument.js
  • packages/web/src/components/checklist-ui/pdf/pdfRenderer.js
  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
  • packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js
  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
packages/web/src/components/**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

packages/web/src/components/**/*.{jsx,tsx}: Use Zag.js for UI components and design system
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
Do NOT destructure props in SolidJS components - access props directly from the props object or wrap in a function to maintain reactivity
Components should be lean and focused - do not implement business logic; move it into stores, utilities, or primitives
Never have a component act as a God component coordinating multiple large concerns

Files:

  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
packages/web/src/**/*.{jsx,tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/corates.mdc)

Use createMemo for derived values and createStore for complex state in SolidJS

Files:

  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to packages/web/src/components/**/*.{jsx,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/Combobox.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Use Zag.js for UI components and design system

Applied to files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/Combobox.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Zag component exist in `packages/web/src/components/zag/*` and should be reused. Check the README.md in that folder for a list of existing components before adding new components and when debugging

Applied to files:

  • packages/ui/src/zag/Tooltip.jsx
  • packages/ui/src/zag/Combobox.jsx
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/components/**/*.{js,jsx,ts,tsx} : Do not destructure props in SolidJS components 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/ui/src/zag/Combobox.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{jsx,tsx} : Use responsive design principles for UI components

Applied to files:

  • packages/web/src/components/checklist-ui/SplitScreenLayout.jsx
📚 Learning: 2025-12-23T04:40:11.191Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .cursor/rules/corates.mdc:0-0
Timestamp: 2025-12-23T04:40:11.191Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular - extract sub-modules into folders, move complex logic into utility files, and split large forms into section components

Applied to files:

  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
📚 Learning: 2025-12-19T14:49:49.730Z
Learnt from: CR
Repo: InfinityBowman/corates PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-19T14:49:49.730Z
Learning: Applies to packages/web/src/**/*.{js,jsx,ts,tsx} : Keep files small, focused, and modular. If a file exceeds a high number of lines, consider refactoring by extracting sub-modules into a folder with index.jsx and helper components, moving complex logic into separate utility files or primitives, or splitting large forms into section components

Applied to files:

  • packages/web/src/components/checklist-ui/pdf/usePdfJs.js
🧬 Code graph analysis (4)
packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js (3)
packages/web/src/components/checklist-ui/pdf/usePdfJs.js (1)
  • document (22-22)
packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx (1)
  • clearPdf (41-43)
packages/web/src/components/checklist-ui/pdf/PdfViewer.jsx (1)
  • setFileInputRef (41-43)
packages/web/src/components/checklist-ui/pdf/pdfDocument.js (2)
packages/web/src/lib/pdfUtils.js (1)
  • initPdfJs (41-60)
packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx (1)
  • clearPdf (41-43)
packages/web/src/components/checklist-ui/pdf/pdfRenderer.js (3)
packages/web/src/components/checklist-ui/pdf/usePdfJs.js (2)
  • document (22-22)
  • scrollHandler (23-23)
packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js (1)
  • visiblePages (24-24)
packages/web/src/components/checklist-ui/pdf/pdfDocument.js (1)
  • pdfjsLib (10-10)
packages/web/src/components/checklist-ui/pdf/usePdfJs.js (4)
packages/web/src/components/checklist-ui/pdf/pdfDocument.js (3)
  • createPdfDocument (16-161)
  • totalPages (18-18)
  • docId (24-24)
packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js (1)
  • createPdfScrollHandler (15-478)
packages/web/src/components/checklist-ui/pdf/pdfRenderer.js (1)
  • createPdfRenderer (14-473)
packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js (1)
  • createPdfFileHandler (12-79)
⏰ 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 (20)
packages/web/src/components/checklist-ui/SplitScreenLayout.jsx (1)

107-168: Excellent drag-aware transition improvements!

The refactored transitions are well-implemented:

  • Drag-aware disabling: Transitions are disabled during dragging (isDragging() ? 'none' : ...) to eliminate jank and ensure responsive resizing.
  • Performance hints: The addition of willChange, translateZ(0), and contain: layout provides appropriate browser optimization hints for smooth animations.
  • Consistent timing: 200ms ease-in-out timing across all panels creates a coherent animation experience.
  • Targeted transitions: Each panel only animates the properties it needs (width/height for sizing, opacity/transform for show/hide).

These changes significantly improve the animation quality and performance during both interactive dragging and programmatic panel visibility changes.

packages/ui/src/zag/Combobox.jsx (5)

1-12: Migration from Zag.js to Ark UI is intentional.

The imports and setup correctly reflect the migration from Zag.js to Ark UI, which is the objective of this PR. The previous learnings about using Zag.js for UI components are now outdated by this intentional architectural change.

Based on learnings, the previous guidance to use Zag.js no longer applies to this PR.


57-73: Reactivity concern addressed correctly.

The createEffect at lines 70-73 properly syncs the collection when props.items changes by calling set(items), which addresses the previous review concern about the collection not updating reactively. This is the correct pattern for keeping Ark UI's useListCollection in sync with external props in SolidJS.


75-92: Handler implementation looks correct.

The handlers follow a good pattern: perform internal operations (filtering, state updates) before delegating to the optional prop callbacks. The hasSelectedItems memo correctly accesses reactive props and will update appropriately.


94-151: Render structure follows Ark UI patterns correctly.

The component structure properly uses Ark UI's declarative components (Positioner, Content, ItemGroup, Item, etc.) and correctly accesses reactive values with function calls (collection(), item()). The Portal usage for positioning and conditional rendering with Show and Index follow SolidJS best practices.


154-157: Export pattern is appropriate.

The exports provide both a default export and named exports, including re-exporting useCombobox for programmatic control. This pattern maintains backwards compatibility while exposing the new Ark UI primitives.

packages/ui/src/zag/Tooltip.jsx (3)

1-10: Migration from Zag.js to Ark UI is inconsistent with existing learnings.

This PR intentionally migrates the Tooltip component from Zag.js to Ark UI. However, retrieved learnings indicate that "Zag.js should be used for UI components and design system." Since this architectural change is the explicit goal of PR #135, the learnings should be updated to reflect the new Ark UI adoption.

Based on learnings, the previous guidance was to use Zag.js, but this PR changes that pattern.


107-111: Previous review concern successfully addressed.

The hardcoded pointer-events-none issue from the previous review has been correctly resolved. The interactive prop is now extracted in splitProps (line 61) and the class is conditionally applied at line 108: ${local.interactive ? '' : 'pointer-events-none'}.

This ensures that when interactive: true is passed, users can interact with tooltip content (e.g., clicking links), while maintaining the default non-interactive behavior.


118-122: Clean export pattern provides both convenience and flexibility.

The export structure is well-designed:

  • Default export of TooltipComponent for standard usage
  • Named export as Tooltip for semantic clarity
  • Re-export of useTooltip for programmatic control

This API surface supports both declarative (component) and imperative (hook) usage patterns.

packages/web/src/components/checklist-ui/pdf/pdfDocument.js (4)

9-10: Module-level pdfjsLib is shared across all createPdfDocument instances.

If multiple components call createPdfDocument(), they'll share the same pdfjsLib reference. This is likely intentional (PDF.js library is a singleton), but worth noting that the first component to mount will initialize it, and subsequent ones will reuse. If a component unmounts before initialization completes, other components may fail.


71-79: LGTM!

Proper resource cleanup by destroying old documents, and correctly cloning ArrayBuffer to prevent detachment issues when PDF.js transfers ownership to the web worker.

Also applies to: 86-96


113-136: LGTM!

The clearPdf function properly handles resource cleanup with fire-and-forget destroy (appropriate here since we're clearing all state anyway). The setPdfSourceAndName correctly resets the userCleared flag before setting the new source.


138-161: LGTM!

Clean API surface with proper separation of state signals and action functions. The userCleared: () => userCleared pattern appropriately exposes the non-reactive flag as a function accessor.

packages/web/src/components/checklist-ui/pdf/pdfFileHandler.js (1)

45-49: LGTM!

Clean delegation pattern in handleFileUpload, and openFilePicker safely handles null refs with optional chaining.

Also applies to: 64-70

packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js (2)

219-225: LGTM!

The IntersectionObserver configuration with a 200% vertical margin provides aggressive preloading for smooth scrolling. The 0.01 threshold ensures pages are tracked as soon as they enter the expanded viewport.


399-431: LGTM!

Comprehensive cleanup function properly removes all event listeners, cancels RAF/timeouts, disconnects observers, and clears collections. The returned API is well-organized with clear groupings for state, refs, visibility, navigation, and zoom.

Also applies to: 433-477

packages/web/src/components/checklist-ui/pdf/pdfRenderer.js (2)

354-360: LGTM!

Good Safari compatibility handling with setTimeout(0) fallback for browsers that don't support requestIdleCallback. The 2000ms timeout ensures text layer eventually renders even under heavy load.


431-448: LGTM!

Thorough cleanup function that properly clears all timeouts, cancels renders, disconnects observers, and clears canvas contents. This ensures no resource leaks when the component unmounts.

packages/web/src/components/checklist-ui/pdf/usePdfJs.js (2)

21-40: LGTM!

Excellent modular composition pattern. The hook properly wires up inter-module communication (renderer callbacks, before-load hooks) while exposing a clean, unified API to consumers. This refactoring aligns well with the coding guidelines for keeping files small, focused, and modular.

Also applies to: 100-135


6-6: LGTM!

The createSignal import is used for the loadedPdfName state tracking on line 43.

Comment on lines +72 to +86
// Calculate boundary for positioning (avoid navbar overlap)
const getBoundary = createMemo(() => {
if (typeof document === 'undefined') return 'viewport';
const navbar = document.querySelector('nav[class*="sticky"]');
if (navbar) {
const navbarRect = navbar.getBoundingClientRect();
return {
x: 0,
y: navbarRect.bottom,
width: window.innerWidth,
height: window.innerHeight - navbarRect.bottom,
};
}
return 'viewport';
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Brittle DOM selector couples tooltip to navbar implementation.

The selector nav[class*="sticky"] at line 75 is fragile and tightly couples the tooltip positioning to the navbar's class naming convention. This selector will fail if:

  • The navbar doesn't include "sticky" in its class name
  • The styling approach changes (e.g., CSS-in-JS with hashed classes)
  • The navbar class is refactored

While the fallback to 'viewport' prevents breakage, tooltip positioning will be suboptimal in these scenarios.

Consider one of these alternatives:

  1. Use a data attribute like data-navbar="true" for more explicit and stable querying
  2. Accept an optional boundaryElement prop to allow consumers to specify the boundary
  3. Use a CSS variable or context to communicate the navbar height
  4. Document this coupling clearly and add a code comment explaining the selector dependency
Example: Add boundaryElement prop for explicit control
 /**
  * - positioning: PositioningOptions - Custom positioning options
+ * - boundaryElement: Element | string - Element or selector for boundary (default: auto-detect sticky navbar)
  * - class: string - Additional class for root element
  const [local, machineProps] = splitProps(merged, [
    'content',
    'children',
    'placement',
    'showArrow',
    'interactive',
+   'boundaryElement',
    'class',
    'contentClass',
  ]);
  // Calculate boundary for positioning (avoid navbar overlap)
  const getBoundary = createMemo(() => {
    if (typeof document === 'undefined') return 'viewport';
+   
+   // Use explicit boundary if provided
+   if (local.boundaryElement) {
+     const element = typeof local.boundaryElement === 'string' 
+       ? document.querySelector(local.boundaryElement)
+       : local.boundaryElement;
+     if (element) {
+       const rect = element.getBoundingClientRect();
+       return {
+         x: 0,
+         y: rect.bottom,
+         width: window.innerWidth,
+         height: window.innerHeight - rect.bottom,
+       };
+     }
+   }
+   
+   // Fallback to auto-detection
    const navbar = document.querySelector('nav[class*="sticky"]');

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
packages/ui/src/zag/Tooltip.jsx around lines 72-86: the current DOM query
nav[class*="sticky"] is brittle and couples tooltip positioning to navbar class
names; modify the component to accept an optional boundaryElement prop (use it
when provided), otherwise look for a stable data attribute like
document.querySelector('[data-navbar="true"]') as the preferred fallback, and
only then fall back to 'viewport'; update the component props/type definition
and add a short code comment explaining the selector choice and that consumer
should prefer passing boundaryElement or marking their navbar with
data-navbar="true".

Comment on lines +302 to +309
if (dimensionsChanged) {
context.scale(dpr, dpr);
context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
} else {
// If dimensions haven't changed, we can skip clearing and just re-render
// This prevents white flash during scale changes when size is the same
context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Redundant conditional - both branches call clearRect.

The comment says "skip clearing" for unchanged dimensions, but both branches execute clearRect. Either the comment is outdated, or the else branch should truly skip clearing.

Proposed fix - if clearing is always needed
       if (dimensionsChanged) {
         context.scale(dpr, dpr);
-        context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
-      } else {
-        // If dimensions haven't changed, we can skip clearing and just re-render
-        // This prevents white flash during scale changes when size is the same
-        context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
       }
+      context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (dimensionsChanged) {
context.scale(dpr, dpr);
context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
} else {
// If dimensions haven't changed, we can skip clearing and just re-render
// This prevents white flash during scale changes when size is the same
context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
}
if (dimensionsChanged) {
context.scale(dpr, dpr);
}
context.clearRect(0, 0, canvas.width / dpr, canvas.height / dpr);
🤖 Prompt for AI Agents
In packages/web/src/components/checklist-ui/pdf/pdfRenderer.js around lines 302
to 309, the conditional is redundant because both branches call
context.clearRect; follow the existing comment intent and remove the clearRect
call from the else branch (or remove the else entirely) so that when dimensions
haven't changed we skip clearing to prevent the white flash; keep context.scale
and clearRect only in the dimensionsChanged branch and leave a brief comment
explaining why we skip clearing when size is unchanged.

Comment on lines +186 to +192
} else {
pageRefs.delete(pageNum);
// Unobserve if observer exists
if (intersectionObserver && ref) {
intersectionObserver.unobserve(ref);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Dead code in setPageRef - unreachable unobserve call.

When ref is falsy (null/undefined), the else branch executes. The condition if (intersectionObserver && ref) on line 189 will never be true since ref is already known to be falsy.

Proposed fix
   function setPageRef(pageNum, ref) {
     if (ref) {
       pageRefs.set(pageNum, ref);
       // Set data attribute for IntersectionObserver
       ref.dataset.pageNum = pageNum;
       // Observe the page element if observer is set up
       if (intersectionObserver) {
         intersectionObserver.observe(ref);
       }
     } else {
+      // Unobserve before removing from map
+      const existingRef = pageRefs.get(pageNum);
+      if (intersectionObserver && existingRef) {
+        intersectionObserver.unobserve(existingRef);
+      }
       pageRefs.delete(pageNum);
-      // Unobserve if observer exists
-      if (intersectionObserver && ref) {
-        intersectionObserver.unobserve(ref);
-      }
     }
   }
🤖 Prompt for AI Agents
packages/web/src/components/checklist-ui/pdf/pdfScrollHandler.js around lines
186 to 192: the else branch deletes pageRefs for a falsy ref but then checks `if
(intersectionObserver && ref)` which is unreachable because `ref` is falsy;
change the logic to retrieve the existing ref from pageRefs (e.g. const
existingRef = pageRefs.get(pageNum)) and if intersectionObserver and existingRef
are truthy call intersectionObserver.unobserve(existingRef) before deleting the
entry, then delete pageRefs.delete(pageNum); this ensures unobserve is called
for the previous ref and removes the unreachable check.

Comment on lines +62 to 65
if (document.userCleared()) {
// Reset userCleared flag in document module
// We'll handle this by setting the source directly
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Empty if block for userCleared handling.

The comment suggests resetting the flag, but the block is empty. Either implement the reset or remove the dead code.

Proposed fix - remove empty block if no action needed
     // If user cleared but now we have a DIFFERENT file, accept it
     // (This handles the case where parent sends new data after clear)
-    if (document.userCleared()) {
-      // Reset userCleared flag in document module
-      // We'll handle this by setting the source directly
-    }
+    // Note: setPdfSourceAndName will reset userCleared flag automatically
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (document.userCleared()) {
// Reset userCleared flag in document module
// We'll handle this by setting the source directly
}
// Note: setPdfSourceAndName will reset userCleared flag automatically
🤖 Prompt for AI Agents
In packages/web/src/components/checklist-ui/pdf/usePdfJs.js around lines 62 to
65, there is an empty if block checking document.userCleared() with only a
comment; either implement the intended reset or remove the block to avoid dead
code. If the module should clear the flag here, call the appropriate reset
method on document (e.g., document.resetUserCleared() or
document.setUserCleared(false)) before setting the source; otherwise delete the
if block and its comment so no no-op remains. Ensure imports/types align with
the chosen approach and add a brief test or comment if needed to document the
behavior.

@InfinityBowman InfinityBowman merged commit c9980ac into main Dec 24, 2025
2 checks passed
@InfinityBowman InfinityBowman deleted the 133-migrate-to-ark-ui branch December 24, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate to ark ui

2 participants