Lite Note: Core note organization improvements - #9
Open
ark-commits wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughApp and sidebar changes add dual-key localStorage for notes (v2 preferred, v1 fallback), normalize note shape (id, createdAt, updatedAt, isPinned), introduce search/filter/sort (visibleNotes), and add in-app note mutations (rename, delete, duplicate, toggle-pin) with UI wiring and formatted timestamps. ChangesNotes Storage, State, Search & UI
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Sidebar as NotesSidebar
participant App as App
participant Storage as localStorage
User->>Sidebar: types in search / clicks action
Sidebar->>App: onSearchChange / onRenameNote / onDeleteNote / onDuplicateNote / onTogglePin
App->>App: update searchQuery or mutate notes (normalize, set updatedAt, toggle isPinned)
App->>App: recompute visibleNotes (filter + sort)
App->>Storage: persist notes to notesV2
App->>Sidebar: render visibleNotes + current searchQuery
Sidebar->>User: display filtered/sorted notes and updated UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.jsx`:
- Around line 189-209: The visibleNotes useMemo currently sorts by updatedAt
when normalizedSearchQuery is truthy, which ignores isPinned; instead, first
filter notes by normalizedSearchQuery (if present) and then sort the resulting
list so pinned notes always come first and within each group sort by updatedAt.
Update the visibleNotes logic to: compute filteredNotes = normalizedSearchQuery
? notes.filter(...) : notes; then compute sortedNotes =
[...filteredNotes].sort((a,b) => (a.isPinned === b.isPinned) ? (b.updatedAt -
a.updatedAt) : (a.isPinned ? -1 : 1)); and return sortedNotes; ensure you
reference normalizedSearchQuery, visibleNotes, sortedNotes and isPinned in the
change.
Comment on lines
+189
to
+209
| const normalizedSearchQuery = useMemo(() => searchQuery.trim().toLowerCase(), [searchQuery]) | ||
|
|
||
| const visibleNotes = useMemo(() => { | ||
| const sortedNotes = [...notes].sort((a, b) => { | ||
| if (normalizedSearchQuery) { | ||
| return b.updatedAt - a.updatedAt | ||
| } | ||
|
|
||
| if (a.isPinned !== b.isPinned) { | ||
| return a.isPinned ? -1 : 1 | ||
| } | ||
|
|
||
| return b.updatedAt - a.updatedAt | ||
| }) | ||
|
|
||
| if (!normalizedSearchQuery) { | ||
| return sortedNotes | ||
| } | ||
|
|
||
| return sortedNotes.filter((note) => note.title.toLowerCase().includes(normalizedSearchQuery)) | ||
| }, [notes, normalizedSearchQuery]) |
There was a problem hiding this comment.
Pinned notes lose priority during search.
When normalizedSearchQuery is set, sorting ignores isPinned, so pinned notes can appear below unpinned ones. This violates the requirement that pinned notes remain on top.
✅ Suggested fix (filter first, then always pin-sort)
- const visibleNotes = useMemo(() => {
- const sortedNotes = [...notes].sort((a, b) => {
- if (normalizedSearchQuery) {
- return b.updatedAt - a.updatedAt
- }
-
- if (a.isPinned !== b.isPinned) {
- return a.isPinned ? -1 : 1
- }
-
- return b.updatedAt - a.updatedAt
- })
-
- if (!normalizedSearchQuery) {
- return sortedNotes
- }
-
- return sortedNotes.filter((note) => note.title.toLowerCase().includes(normalizedSearchQuery))
- }, [notes, normalizedSearchQuery])
+ const visibleNotes = useMemo(() => {
+ const filteredNotes = normalizedSearchQuery
+ ? notes.filter((note) => note.title.toLowerCase().includes(normalizedSearchQuery))
+ : notes
+
+ return [...filteredNotes].sort((a, b) => {
+ if (a.isPinned !== b.isPinned) {
+ return a.isPinned ? -1 : 1
+ }
+ return b.updatedAt - a.updatedAt
+ })
+ }, [notes, normalizedSearchQuery])📝 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
| const normalizedSearchQuery = useMemo(() => searchQuery.trim().toLowerCase(), [searchQuery]) | |
| const visibleNotes = useMemo(() => { | |
| const sortedNotes = [...notes].sort((a, b) => { | |
| if (normalizedSearchQuery) { | |
| return b.updatedAt - a.updatedAt | |
| } | |
| if (a.isPinned !== b.isPinned) { | |
| return a.isPinned ? -1 : 1 | |
| } | |
| return b.updatedAt - a.updatedAt | |
| }) | |
| if (!normalizedSearchQuery) { | |
| return sortedNotes | |
| } | |
| return sortedNotes.filter((note) => note.title.toLowerCase().includes(normalizedSearchQuery)) | |
| }, [notes, normalizedSearchQuery]) | |
| const normalizedSearchQuery = useMemo(() => searchQuery.trim().toLowerCase(), [searchQuery]) | |
| const visibleNotes = useMemo(() => { | |
| const filteredNotes = normalizedSearchQuery | |
| ? notes.filter((note) => note.title.toLowerCase().includes(normalizedSearchQuery)) | |
| : notes | |
| return [...filteredNotes].sort((a, b) => { | |
| if (a.isPinned !== b.isPinned) { | |
| return a.isPinned ? -1 : 1 | |
| } | |
| return b.updatedAt - a.updatedAt | |
| }) | |
| }, [notes, normalizedSearchQuery]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.jsx` around lines 189 - 209, The visibleNotes useMemo currently sorts
by updatedAt when normalizedSearchQuery is truthy, which ignores isPinned;
instead, first filter notes by normalizedSearchQuery (if present) and then sort
the resulting list so pinned notes always come first and within each group sort
by updatedAt. Update the visibleNotes logic to: compute filteredNotes =
normalizedSearchQuery ? notes.filter(...) : notes; then compute sortedNotes =
[...filteredNotes].sort((a,b) => (a.isPinned === b.isPinned) ? (b.updatedAt -
a.updatedAt) : (a.isPinned ? -1 : 1)); and return sortedNotes; ensure you
reference normalizedSearchQuery, visibleNotes, sortedNotes and isPinned in the
change.
Owner
Author
There was a problem hiding this comment.
@coderabbitai create a new issue for this in linear under the ARK team
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
curl -X POST \
-H "Authorization: Bearer $LINEAR_API_TOKEN" \
-H "Content-Type: application/json" \
-d "{
\"query\": \"mutation { issueCreate(input: { title: \\\"Pinned notes lose priority during search\\\", description: \\\"## Issue\\n\\nWhen a search query is active, the sorting logic in \\\\\`visibleNotes\\\\\` ignores the \\\\\`isPinned\\\\\` property, causing pinned notes to appear below unpinned notes based solely on \\\\\`updatedAt\\\\\`. This violates the requirement that pinned notes should always remain on top.\\n\\n## Location\\n\\nFile: \\\\\`src/App.jsx\\\\\`\\nFunction: \\\\\`visibleNotes\\\\\` useMemo hook\\n\\n## Context\\n\\n- PR: https://github.com/ark-commits/lite-note/pull/9\\n- Comment: https://github.com/ark-commits/lite-note/pull/9#discussion_r2824135699\\n- Reported by: `@ark-commits`\\n\\n## Expected Behavior\\n\\nPinned notes should always appear first in the list, regardless of whether a search filter is active, followed by unpinned notes. Within each group (pinned and unpinned), notes should be sorted by \\\\\`updatedAt\\\\\` descending.\\\", teamId: \\\"ARK\\\" }) { success issue { id identifier url } } }\"
}" \
"https://api.linear.app/graphql"
Length of output: 803
🏁 Script executed:
#!/bin/bash
# Create the description with proper escaping
DESCRIPTION="## Issue\\n\\nWhen a search query is active, the sorting logic in \`visibleNotes\` ignores the \`isPinned\` property, causing pinned notes to appear below unpinned notes based solely on \`updatedAt\`. This violates the requirement that pinned notes should always remain on top.\\n\\n## Location\\n\\nFile: \`src/App.jsx\`\\nFunction: \`visibleNotes\` useMemo hook\\n\\n## Context\\n\\n- PR: https://github.com/ark-commits/lite-note/pull/9\\n- Comment: https://github.com/ark-commits/lite-note/pull/9#discussion_r2824135699\\n- Reported by: `@ark-commits`\\n\\n## Expected Behavior\\n\\nPinned notes should always appear first in the list, regardless of whether a search filter is active, followed by unpinned notes. Within each group (pinned and unpinned), notes should be sorted by \`updatedAt\` descending."
# Create the mutation query
MUTATION="mutation { issueCreate(input: { title: \"Pinned notes lose priority during search\", description: \"$DESCRIPTION\", teamId: \"ARK\" }) { success issue { id identifier url } } }"
# Execute the GraphQL request
curl -X POST \
-H "Authorization: Bearer $LINEAR_API_TOKEN" \
-H "Content-Type: application/json" \
-d "$(jq -n --arg query "$MUTATION" '{query: $query}')" \
"https://api.linear.app/graphql"
Length of output: 1639
🏁 Script executed:
#!/bin/bash
# First, get the team UUID for team key "ARK"
TEAM_QUERY='query { teams { nodes { id key name } } }'
TEAM_RESPONSE=$(curl -s -X POST \
-H "Authorization: Bearer $LINEAR_API_TOKEN" \
-H "Content-Type: application/json" \
-d "$(jq -n --arg query "$TEAM_QUERY" '{query: $query}')" \
"https://api.linear.app/graphql")
echo "Team response:"
echo "$TEAM_RESPONSE"
# Extract the team ID for ARK team
TEAM_ID=$(echo "$TEAM_RESPONSE" | jq -r '.data.teams.nodes[] | select(.key == "ARK") | .id')
echo ""
echo "Team ID for ARK: $TEAM_ID"
if [ -z "$TEAM_ID" ] || [ "$TEAM_ID" = "null" ]; then
echo "Error: Could not find team with key ARK"
exit 1
fi
# Now create the issue with the correct team UUID
DESCRIPTION="## Issue\\n\\nWhen a search query is active, the sorting logic in \`visibleNotes\` ignores the \`isPinned\` property, causing pinned notes to appear below unpinned notes based solely on \`updatedAt\`. This violates the requirement that pinned notes should always remain on top.\\n\\n## Location\\n\\nFile: \`src/App.jsx\`\\nFunction: \`visibleNotes\` useMemo hook\\n\\n## Context\\n\\n- PR: https://github.com/ark-commits/lite-note/pull/9\\n- Comment: https://github.com/ark-commits/lite-note/pull/9#discussion_r2824135699\\n- Reported by: `@ark-commits`\\n\\n## Expected Behavior\\n\\nPinned notes should always appear first in the list, regardless of whether a search filter is active, followed by unpinned notes. Within each group (pinned and unpinned), notes should be sorted by \`updatedAt\` descending."
MUTATION="mutation { issueCreate(input: { title: \"Pinned notes lose priority during search\", description: \"$DESCRIPTION\", teamId: \"$TEAM_ID\" }) { success issue { id identifier url } } }"
echo ""
echo "Creating issue..."
curl -s -X POST \
-H "Authorization: Bearer $LINEAR_API_TOKEN" \
-H "Content-Type: application/json" \
-d "$(jq -n --arg query "$MUTATION" '{query: $query}')" \
"https://api.linear.app/graphql" | jq '.'
Length of output: 483
@ark-commits I've successfully created a Linear issue for this problem:
ARK-7: Pinned notes lose priority during search
The issue documents the sorting bug where pinned notes lose their top position when a search query is active, includes the location in the code, and links back to this PR and comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/App.jsx (1)
191-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSearch results still break pin priority and content matching consistency.
When search is active, sorting ignores
isPinned, and content matching uses rawsearchQuery(not normalized), so body search can fail for uppercase/mixed-case input.✅ Suggested fix
const visibleNotes = useMemo(() => { - const sortedNotes = [...notes].sort((a, b) => { - if (normalizedSearchQuery) { - return b.updatedAt - a.updatedAt - } - - if (a.isPinned !== b.isPinned) { - return a.isPinned ? -1 : 1 - } - - return b.updatedAt - a.updatedAt - }) - - if (!normalizedSearchQuery) { - return sortedNotes - } - - return sortedNotes.filter( - (note) => - note.title.toLowerCase().includes(normalizedSearchQuery) || - note.content.toLowerCase().includes(searchQuery), - ) + const filteredNotes = normalizedSearchQuery + ? notes.filter( + (note) => + note.title.toLowerCase().includes(normalizedSearchQuery) || + note.content.toLowerCase().includes(normalizedSearchQuery), + ) + : notes + + return [...filteredNotes].sort((a, b) => { + if (a.isPinned !== b.isPinned) { + return a.isPinned ? -1 : 1 + } + return b.updatedAt - a.updatedAt + }) }, [notes, normalizedSearchQuery])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.jsx` around lines 191 - 213, The visibleNotes useMemo is dropping pin priority when a search is active and is using the raw searchQuery for body matching; update the sorting and filtering in visibleNotes so sorting always respects isPinned first then updatedAt (i.e., sort by pinned difference then updatedAt for all cases inside the sort callback of visibleNotes) and use normalizedSearchQuery for both title and content comparisons (replace the content check note.content.toLowerCase().includes(searchQuery) with note.content.toLowerCase().includes(normalizedSearchQuery)) so case-insensitive search and pin precedence remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/App.jsx`:
- Around line 487-511: handleDuplicateNote can capture stale content because
flushPendingCommit() schedules an update but sourceNote is read from notes
immediately; to fix, after finding sourceNote in handleDuplicateNote (and before
building duplicatedNote) check if sourceNote.id === activeNoteIdRef.current and
if so use the latest draftContent (instead of sourceNote.content) when setting
duplicatedNote.content (keep using createNoteId(), timestamps, isPinned reset,
and update setNotes/setActiveNoteId/activeNoteIdRef.set accordingly). This
ensures the duplicated note uses the most recent draft when duplicating the
currently active note while leaving other notes unchanged.
---
Duplicate comments:
In `@src/App.jsx`:
- Around line 191-213: The visibleNotes useMemo is dropping pin priority when a
search is active and is using the raw searchQuery for body matching; update the
sorting and filtering in visibleNotes so sorting always respects isPinned first
then updatedAt (i.e., sort by pinned difference then updatedAt for all cases
inside the sort callback of visibleNotes) and use normalizedSearchQuery for both
title and content comparisons (replace the content check
note.content.toLowerCase().includes(searchQuery) with
note.content.toLowerCase().includes(normalizedSearchQuery)) so case-insensitive
search and pin precedence remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+487
to
+511
| const handleDuplicateNote = (noteId) => { | ||
| flushPendingCommit() | ||
|
|
||
| const sourceNote = notes.find((note) => note.id === noteId) | ||
| if (!sourceNote) { | ||
| return | ||
| } | ||
|
|
||
| const now = Date.now() | ||
| const duplicatedNote = { | ||
| ...sourceNote, | ||
| id: createNoteId(), | ||
| title: `${sourceNote.title} Copy`, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
| isPinned: false, | ||
| } | ||
|
|
||
| setNotes((previousNotes) => [...previousNotes, duplicatedNote]) | ||
| setActiveNoteId(duplicatedNote.id) | ||
| activeNoteIdRef.current = duplicatedNote.id | ||
| setDraftContent(duplicatedNote.content) | ||
| setActiveMobileTab('edit') | ||
| setIsMobileNotesOpen(false) | ||
| } |
There was a problem hiding this comment.
Duplicate can copy stale content right after a pending flush.
flushPendingCommit() schedules a state update, but sourceNote is read from current render state (notes) immediately after. That can duplicate pre-flush content instead of the latest committed draft.
✅ Suggested fix
const handleDuplicateNote = (noteId) => {
flushPendingCommit()
-
- const sourceNote = notes.find((note) => note.id === noteId)
- if (!sourceNote) {
- return
- }
-
- const now = Date.now()
- const duplicatedNote = {
- ...sourceNote,
- id: createNoteId(),
- title: `${sourceNote.title} Copy`,
- createdAt: now,
- updatedAt: now,
- isPinned: false,
- }
-
- setNotes((previousNotes) => [...previousNotes, duplicatedNote])
- setActiveNoteId(duplicatedNote.id)
- activeNoteIdRef.current = duplicatedNote.id
- setDraftContent(duplicatedNote.content)
- setActiveMobileTab('edit')
- setIsMobileNotesOpen(false)
+ setNotes((previousNotes) => {
+ const sourceNote = previousNotes.find((note) => note.id === noteId)
+ if (!sourceNote) {
+ return previousNotes
+ }
+
+ const now = Date.now()
+ const duplicatedNote = {
+ ...sourceNote,
+ id: createNoteId(),
+ title: `${sourceNote.title} Copy`,
+ createdAt: now,
+ updatedAt: now,
+ isPinned: false,
+ }
+
+ setActiveNoteId(duplicatedNote.id)
+ activeNoteIdRef.current = duplicatedNote.id
+ setDraftContent(duplicatedNote.content)
+ setActiveMobileTab('edit')
+ setIsMobileNotesOpen(false)
+
+ return [...previousNotes, duplicatedNote]
+ })
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/App.jsx` around lines 487 - 511, handleDuplicateNote can capture stale
content because flushPendingCommit() schedules an update but sourceNote is read
from notes immediately; to fix, after finding sourceNote in handleDuplicateNote
(and before building duplicatedNote) check if sourceNote.id ===
activeNoteIdRef.current and if so use the latest draftContent (instead of
sourceNote.content) when setting duplicatedNote.content (keep using
createNoteId(), timestamps, isPinned reset, and update
setNotes/setActiveNoteId/activeNoteIdRef.set accordingly). This ensures the
duplicated note uses the most recent draft when duplicating the currently active
note while leaving other notes unchanged.
ark-commits
force-pushed
the
note-organization-basics
branch
from
May 4, 2026 00:09
e773466 to
56133b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements core note organization improvements in Lite Note (search, pin/unpin, rename, duplicate, delete, recency sorting, and notes storage v2 migration).
Closes https://linear.app/arkcoderabbit/issue/ARK-5/lite-note-core-note-organization-improvements
Summary by CodeRabbit
New Features
Improvements