Skip to content

notes show on completed checklists - #153

Merged
InfinityBowman merged 1 commit into
mainfrom
140-final-notes-for-checklist
Dec 25, 2025
Merged

notes show on completed checklists#153
InfinityBowman merged 1 commit into
mainfrom
140-final-notes-for-checklist

Conversation

@InfinityBowman

@InfinityBowman InfinityBowman commented Dec 25, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • Changes
    • Updated note editor initialization behavior in read-only mode to improve default expansion state handling
    • Modified the navigation target for the checklist creation button

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

@InfinityBowman InfinityBowman linked an issue Dec 25, 2025 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Dec 25, 2025

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Two component updates adjust state initialization behavior and navigation routing. NoteEditor.jsx now initializes expanded state based on readOnly status and tracks manual toggles. Sidebar.jsx updates the "Create one" button navigation target from /checklist/new to /checklist.

Changes

Cohort / File(s) Summary
State & Initialization Logic
packages/web/src/components/checklist-ui/common/NoteEditor.jsx
Modified expanded state initialization to become true when readOnly is enabled, otherwise inverse of collapsed prop. Added hasBeenManuallyToggled state (initially false) to track manual expand/collapse interactions. Collapsible onOpenChange handler now records toggle before updating state.
Navigation Route
packages/web/src/components/sidebar/Sidebar.jsx
Changed "Create one" button navigation target from /checklist/new to /checklist. No changes to control flow or error handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop and a skip through state we go,
Toggle tracking and expanded flow!
Routes now lighter, navigation so bright,
Two small tweaks make everything right! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'notes show on completed checklists' is vague and doesn't clearly describe the actual technical changes in the PR, which involve expanded state initialization in NoteEditor and navigation URL changes in Sidebar. Clarify the title to better reflect the technical changes, such as 'Initialize note editor expanded state based on readOnly mode' or 'Fix note visibility and navigation for completed checklists'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 140-final-notes-for-checklist

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

@cloudflare-workers-and-pages

Copy link
Copy Markdown

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 416a2f9 Commit Preview URL Dec 25 2025, 10:35 PM

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/web/src/components/checklist-ui/common/NoteEditor.jsx (1)

32-32: Remove or utilize the unused hasBeenManuallyToggled state.

The hasBeenManuallyToggled signal is set when the collapsible is toggled but is never read or used elsewhere in the component. This appears to be dead code.

Proposed fix to remove unused state

Remove the unused signal:

-  const [hasBeenManuallyToggled, setHasBeenManuallyToggled] = createSignal(false);
   let textareaRef;

And simplify the onOpenChange handler:

-            onOpenChange={({ open }) => {
-              setHasBeenManuallyToggled(true);
-              setExpanded(open);
-            }}
+            onOpenChange={({ open }) => setExpanded(open)}

Alternatively, if this state is intended for future functionality, please clarify its purpose.

Also applies to: 138-141

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd21fef and 416a2f9.

📒 Files selected for processing (2)
  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
🧰 Additional context used
📓 Path-based instructions (14)
**/*

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

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

NEVER use emojis anywhere in code, comments, documentation, plan files, or commit messages. This includes unicode symbols. For UI icons, use solid-icons library or SVGs only.

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.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

Use import aliases from jsconfig.json instead of relative paths

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.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 in JavaScript/TypeScript code
Comments should explain why something is being done, not narrate what the code does. Avoid comments that repeat variable names or describe obvious code behavior.

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.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

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/{web,ui}/src/**/*.{jsx,tsx}

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

Group related components in subdirectories with barrel exports

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/{web,landing}/src/**/*.{jsx,tsx}

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

packages/{web,landing}/src/**/*.{jsx,tsx}: Use Ark UI components from @corates/ui package, not local component implementations
Use solid-icons library (e.g., solid-icons/bi, solid-icons/fi) for icon imports

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/web/src/**/*.{jsx,tsx}

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

packages/web/src/**/*.{jsx,tsx}: In SolidJS, do NOT prop-drill application state. Import stores directly where needed instead.
In SolidJS, do NOT destructure props. Access props.field directly or wrap in a function: () => props.field
In SolidJS components, components should receive at most 1-5 props (local config only, not shared state)

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/web/src/**/*.{jsx,tsx,js,ts}

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

In SolidJS, use createMemo for derived values

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/web/src/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

packages/web/src/**/*.{js,ts,jsx,tsx}: Always use handleFetchError from @/lib/error-utils.js for fetch calls in frontend code with options like { showToast: true } for error handling
Use createFormErrorSignals from @/lib/form-errors.js for form validation error handling with field-level and global error management

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)

packages/{web,workers}/src/**/*.{js,ts,jsx,tsx}: Never throw string literals; always throw Error objects or return domain errors from API routes
Use error utility functions like isErrorCode from @corates/shared or @/lib/error-utils.js to check specific error types instead of manual string comparisons

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}

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

{packages/web/**,packages/landing/**}/**/*.{jsx,tsx,js,ts}: Never destructure props in SolidJS components - destructuring breaks reactivity. Access props directly (e.g., props.name) or wrap in a function (e.g., const name = () => props.name) to maintain reactivity.
Import stores directly in components rather than prop-drilling store data through component hierarchies.
Use separate read and write patterns for stores: import the store directly for reading data (e.g., projectStore.getProjectList()) and import action stores separately for writing (e.g., projectActionsStore.createProject()).
Use createSignal from solid-js for managing simple reactive values. Prefer derived state with signals or memo over effects when possible.
Use createStore from solid-js/store for managing complex objects and arrays that require granular reactivity, enabling fine-grained updates where only affected parts re-render.
Use createMemo from solid-js for derived values that depend on reactive state, ensuring computed values update only when their dependencies change.
Always clean up effects that create subscriptions or timers using the onCleanup function from solid-js. Use effects sparingly, only when derived values won't work well.
Keep components lean and focused on rendering. Move business logic to stores (for shared state and operations), primitives (for reusable hooks/logic), or utilities (for pure functions).
Use the Show component from solid-js for conditional rendering instead of JavaScript ternary operators or logical AND operators.
Use the For component from solid-js for rendering lists. It provides better performance and keying compared to JavaScript's map function in JSX.
When manipulating children in wrapper components, use the children helper from solid-js to ensure proper reactivity and handling of child elements.

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/{web,ui}/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

packages/{web,ui}/**/*.{js,jsx,ts,tsx}: Import UI components from '@corates/ui' package instead of local component files. Do not import Ark UI components from local paths like '@/components/zag/' or 'packages/web/src/components/zag/'
Always use 'solid-icons' library for icons. Never use emoji characters or text as icon replacements. Import from specific icon sets like 'solid-icons/bi', 'solid-icons/fi', 'solid-icons/ai', etc.

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/web/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Use import aliases from 'packages/web/jsconfig.json' instead of relative paths. Aliases include: '@/' (src/), '@components/' (src/components/), '@auth-ui/' (src/components/auth-ui/), '@checklist-ui/' (src/components/checklist-ui/), '@project-ui/' (src/components/project-ui/), '@routes/' (src/routes/), '@primitives/' (src/primitives/), '@api/' (src/api/), '@config/' (src/config/), and '@lib/' (src/lib/)

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
packages/{web,ui}/**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Use Tailwind CSS classes for styling components

Files:

  • packages/web/src/components/checklist-ui/common/NoteEditor.jsx
  • packages/web/src/components/sidebar/Sidebar.jsx
🧬 Code graph analysis (2)
packages/web/src/components/checklist-ui/common/NoteEditor.jsx (2)
packages/web/src/components/project-ui/todo-tab/TodoStudyRow.jsx (1)
  • expanded (28-28)
packages/web/src/components/checklist-ui/compare/NotesCompareSection.jsx (1)
  • expanded (26-26)
packages/web/src/components/sidebar/Sidebar.jsx (14)
packages/web/src/components/Navbar.jsx (1)
  • navigate (11-11)
packages/web/src/components/project-ui/ProjectDashboard.jsx (1)
  • navigate (16-16)
packages/web/src/components/checklist-ui/ChecklistYjsWrapper.jsx (1)
  • navigate (19-19)
packages/web/src/components/project-ui/ProjectView.jsx (1)
  • navigate (35-35)
packages/web/src/components/checklist-ui/compare/ReconciliationWrapper.jsx (1)
  • navigate (23-23)
packages/web/src/components/sidebar/ChecklistTreeItem.jsx (1)
  • navigate (11-11)
packages/web/src/components/checklist-ui/CreateLocalChecklist.jsx (1)
  • navigate (15-15)
packages/web/src/components/sidebar/ProjectTreeItem.jsx (1)
  • navigate (13-13)
packages/web/src/components/auth-ui/SignIn.jsx (1)
  • navigate (27-27)
packages/web/src/components/auth-ui/SignUp.jsx (1)
  • navigate (25-25)
packages/web/src/components/auth-ui/AuthLayout.jsx (1)
  • navigate (11-11)
packages/web/src/components/auth-ui/CheckEmail.jsx (1)
  • navigate (19-19)
packages/web/src/components/checklist-ui/ChecklistsDashboard.jsx (1)
  • navigate (10-10)
packages/web/src/components/sidebar/LocalChecklistItem.jsx (1)
  • navigate (9-9)
⏰ 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 (2)
packages/web/src/components/checklist-ui/common/NoteEditor.jsx (1)

26-30: Initialization logic aligns with PR objectives.

The expanded state now initializes based on readOnly status, which ensures notes are visible by default on read-only (completed) checklists. The eslint-disable comment correctly indicates this is a one-time read at initialization.

Note: If readOnly changes after mount, the expanded state won't automatically update. This appears intentional based on the comment, but be aware that toggling readOnly dynamically won't affect the expansion state unless the user manually toggles it.

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

167-167: The /checklist route exists and properly handles new checklist creation.

The route is defined in packages/web/src/Routes.jsx with path /checklist/*, which matches the navigation target. When navigate('/checklist') is called, it routes to LocalChecklistView, which detects the absence of a checklistId parameter and renders the CreateLocalChecklist component. This flow correctly handles the creation of new checklists. No remaining references to /checklist/new exist in the codebase.

@InfinityBowman
InfinityBowman merged commit 17cce03 into main Dec 25, 2025
3 checks passed
@InfinityBowman
InfinityBowman deleted the 140-final-notes-for-checklist branch December 25, 2025 23:03
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.

Final notes for checklist

1 participant