feat: add alerts when note is updated or created and won't be visible#358
feat: add alerts when note is updated or created and won't be visible#358
Conversation
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRefactors note add/update to return visibility info, adds a filtered-note alert hook and UI, exposes a wrapped Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant NoteManager
participant useNotes as useNoteManagerNotes
participant Provider as NotesProvider
participant Labels as useLabels\n(isVisibleByActiveLabelsLatest)
participant Alert as useFilteredNoteAlert
User->>NoteManager: create/update note
NoteManager->>useNotes: addNote / updateNote
useNotes->>Provider: handleAddNote / handleUpdateNote
Provider->>Labels: isVisibleByActiveLabelsLatest(note)
Labels-->>Provider: boolean (isVisible)
Provider-->>useNotes: { isVisible }
useNotes-->>NoteManager: { isVisible }
alt isVisible == true
NoteManager->>User: render note list / selection kept
else isVisible == false
NoteManager->>Alert: triggerFilteredNoteAlert("visibleForCreated"|"visibleForUpdated")
Alert-->>NoteManager: alert visible (auto-dismiss after 6s)
NoteManager->>NoteManager: clear selection
NoteManager->>User: render alert + updated UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Visual Regression Test Report ✅ PassedGithub run id: 20607065733 🔗 Artifacts: Download |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/NoteManager/hooks/useFilteredNoteAlert.ts (2)
29-31: Consider clearing the timeout when manually closing the alert.When
closeNoteAlertis called, the existing timeout continues running. While this is harmless (it will just set an already-hidden alert to hidden), clearing the timeout would be cleaner and avoid unnecessary callback execution.🔎 Proposed fix
const closeNoteAlert = useCallback(() => { + if (alertTimeoutRef.current) { + window.clearTimeout(alertTimeoutRef.current); + alertTimeoutRef.current = null; + } setNoteAlertVisibiltyState("hidden"); }, []);
6-6: Minor typo in state setter name.
setNoteAlertVisibiltyStateis missing an 'i' — should besetNoteAlertVisibilityState.🔎 Proposed fix
-const [noteAlertVisibilityState, setNoteAlertVisibiltyState] = useState< +const [noteAlertVisibilityState, setNoteAlertVisibilityState] = useState< "hidden" | "visibleForCreated" | "visibleForUpdated" >("hidden");Then update all usages of
setNoteAlertVisibiltyStatetosetNoteAlertVisibilityState.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NoteManager/useNoteManagerNotes.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NotesProvider/hooks/useLabels.ts
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Feature folders should keep components, hooks, and styles together insrc/
Prefer hooks over Higher-Order Components (HOCs)
Colocate helper functions with their consumer components
Use PascalCase for component names
Favor semantic wrappers over long Tailwind utility class strings; complement Tailwind with@fluffylabs/shared-uicomponents
Files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript as the primary language with functional React components
Use camelCase for function and variable names
Use SCREAMING_SNAKE_CASE for constants
Files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 357
File: src/components/NoteManager/NoteManager.tsx:52-58
Timestamp: 2025-12-28T21:10:04.074Z
Learning: In `src/components/NoteManager/NoteManager.tsx`, the direct mutation of `note.original.content` in `memoizedHandleUpdateNote` before calling `updateNote` is intentional and used for optimistic UI updates.
📚 Learning: 2025-12-28T21:10:33.529Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 357
File: src/components/NoteManager/NoteManager.tsx:52-58
Timestamp: 2025-12-28T21:10:33.529Z
Learning: In src/components/NoteManager/NoteManager.tsx, direct mutation of `note.original.content` before calling `updateNote` is intentional for optimistic UI updates. This pattern is documented with an explicit comment in the code.
Applied to files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
📚 Learning: 2025-12-28T21:10:04.074Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 357
File: src/components/NoteManager/NoteManager.tsx:52-58
Timestamp: 2025-12-28T21:10:04.074Z
Learning: In `src/components/NoteManager/NoteManager.tsx`, the direct mutation of `note.original.content` in `memoizedHandleUpdateNote` before calling `updateNote` is intentional and used for optimistic UI updates.
Applied to files:
src/components/NoteManager/hooks/useFilteredNoteAlert.tssrc/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.tssrc/components/NoteManager/useNoteManagerNotes.ts
📚 Learning: 2025-12-28T13:25:05.152Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 355
File: src/components/NoteManager/components/NoteLayout.tsx:39-39
Timestamp: 2025-12-28T13:25:05.152Z
Learning: In Tailwind CSS 4.x, max-h-68 is a valid utility that sets max-height: 17rem (272px) and is part of the extended spacing scale. When reviewing TSX files, verify components use this class consistently and that tailwind.config.js includes the 68 spacing key in the extended spacing scale if you rely on it.
Applied to files:
src/components/NotesProvider/NotesProvider.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsx
📚 Learning: 2025-10-22T20:36:10.440Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 330
File: src/components/Outline/OutlineLink.tsx:17-21
Timestamp: 2025-10-22T20:36:10.440Z
Learning: Project targets React 19+; prefer passing refs as a regular prop in function components and avoid React.forwardRef in new code. File: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/NoteManager/components/Note.tsxsrc/components/NoteManager/NoteManager.tsxsrc/components/NotesProvider/hooks/useLabels.ts
📚 Learning: 2025-12-28T21:09:55.992Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 357
File: src/components/NoteManager/NoteManager.tsx:52-58
Timestamp: 2025-12-28T21:09:55.992Z
Learning: In src/components/NoteManager/NoteManager.tsx, note.original.content is mutated directly within memoizedHandleUpdateNote before updateNote is called to support optimistic UI updates. This is intentional for this component; during reviews, don't flag this as an improper mutation. Ensure the mutation is isolated, safe, and revertible, and consider documenting the rationale in code comments.
Applied to files:
src/components/NoteManager/NoteManager.tsx
📚 Learning: 2025-10-22T20:36:10.440Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 330
File: src/components/Outline/OutlineLink.tsx:17-21
Timestamp: 2025-10-22T20:36:10.440Z
Learning: This repo targets React 19+; prefer passing refs as a normal prop to function components and avoid React.forwardRef in new code. File context: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/NoteManager/NoteManager.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NotesProvider/hooks/useLabels.ts
📚 Learning: 2025-10-28T20:41:06.887Z
Learnt from: chmurson
Repo: FluffyLabs/graypaper-reader PR: 333
File: src/components/PdfProvider/PdfProvider.tsx:34-37
Timestamp: 2025-10-28T20:41:06.887Z
Learning: In the graypaper-reader codebase, for text layer render tracking in PdfProvider, use RefObject<number[]> approach (textLayerRenderedRef) without adding reactive signals or version numbers. Consumers should check the ref imperatively when needed rather than reacting to changes.
Applied to files:
src/components/NotesProvider/hooks/useLabels.ts
🧬 Code graph analysis (6)
src/components/NotesProvider/NotesProvider.tsx (3)
src/components/NotesProvider/types/StorageNote.ts (1)
IStorageNote(5-5)src/components/NotesProvider/types/DecoratedNote.ts (1)
IDecoratedNote(4-19)src/components/NotesProvider/hooks/useLabels.ts (1)
useLabels(119-247)
src/components/NoteManager/components/Note.tsx (2)
src/components/NotesProvider/types/DecoratedNote.ts (1)
IDecoratedNote(4-19)src/components/NotesProvider/types/StorageNote.ts (1)
IStorageNote(5-5)
src/components/NoteManager/NoteManager.tsx (4)
src/components/NoteManager/hooks/useFilteredNoteAlert.ts (1)
useFilteredNoteAlert(5-34)src/components/NotesProvider/utils/areSelectionsEqual.ts (1)
areSelectionsEqual(8-13)src/components/NoteManager/components/NewNote.tsx (1)
NewNote(21-99)src/components/NoteManager/components/InactiveNoteSkeleton.tsx (1)
InactiveNoteSkeleton(6-26)
src/components/NoteManager/components/NoteContext.tsx (2)
src/components/NotesProvider/types/DecoratedNote.ts (1)
IDecoratedNote(4-19)src/components/NotesProvider/types/StorageNote.ts (1)
IStorageNote(5-5)
src/components/NotesProvider/hooks/useLabels.ts (3)
src/components/NotesProvider/types/DecoratedNote.ts (1)
IDecoratedNote(4-19)src/components/NotesProvider/types/StorageNote.ts (1)
IStorageNote(5-5)src/hooks/useLatestCallback.ts (1)
useLatestCallback(4-8)
src/components/NoteManager/useNoteManagerNotes.ts (1)
src/components/NotesProvider/NotesProvider.tsx (1)
INotesContext(20-40)
⏰ 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: visual-tests
🔇 Additional comments (13)
src/components/NoteManager/useNoteManagerNotes.ts (3)
30-37: LGTM!The
addNotewrapper correctly delegates tolatestHandleAddNote.currentand propagates theisVisiblereturn value. The type annotation usingINotesContext["handleAddNote"]ensures type safety with the provider's interface.
39-45: LGTM!The
updateNotefunction now correctly returns the result from the underlying handler, maintaining consistency with the updatedINotesContext["handleUpdateNote"]return type.
118-118: LGTM!Exposing
addNoteinstead oflatestHandleAddNoteprovides a cleaner API surface that properly encapsulates the ref-based callback pattern.src/components/NotesProvider/hooks/useLabels.ts (2)
231-244: LGTM!The
activeLabelsmemo is a good refactor that avoids duplicate computation. TheisVisibleByActiveLabelsLatestusinguseLatestCallbackprovides a stable reference to check note visibility against current active labels without triggering re-renders — essential for the add/update flow to determine if a note should trigger an alert.
119-124: LGTM!The updated return type signature correctly reflects the new
isVisibleByActiveLabelsLatestmember as aRefObject, aligning with theuseLatestCallbackpattern used elsewhere in this codebase.src/components/NoteManager/components/Note.tsx (1)
20-28: LGTM!The explicit
onEditNotesignature is cleaner and appropriately decoupled fromINotesContext. SinceNote.tsxdoesn't need the{ isVisible }return value (handled upstream inNoteManager.tsx), thevoidreturn type is correct here.src/components/NoteManager/components/NoteContext.tsx (1)
5-21: LGTM!The explicit
onEditNotesignature inISingleNoteContextproperly aligns withNote.tsxand removes the coupling toINotesContext. This makes the context interface self-documenting and consistent with the component layer above it.src/components/NotesProvider/NotesProvider.tsx (2)
150-161: LGTM!The
handleAddNoteimplementation correctly computes visibility before adding the note and returns the result. UsingisVisibleByActiveLabelsLatest.current(note)properly leverages the ref-based pattern to get the latest visibility check.
163-181: LGTM!The
handleUpdateNoteimplementation correctly:
- Returns
{ isVisible: true }when refusing to edit remote notes (appropriate since no action was taken)- Computes visibility of the
newNoteagainst active labels- Returns the visibility result for the caller to handle alerts
The dependency array correctly includes
isVisibleByActiveLabelsLatest.src/components/NoteManager/NoteManager.tsx (4)
54-66: LGTM!The optimistic mutation of
note.original.labelson line 58 follows the established pattern fornote.original.content(documented in the existing comment on line 56). The visibility check and alert triggering logic is correctly implemented — clearing selection and showing the alert only when the updated note won't be visible due to label filtering.Based on learnings, this optimistic UI mutation pattern is intentional for this component.
72-107: LGTM!The
handleAddNoteClickcorrectly:
- Creates the new note with all required fields
- Calls
addNoteand destructures theisVisibleresult- Triggers the alert and clears selection when the note won't be visible
- Maintains the
keepShowingNewNoteref for the transitional UI stateThe dependency array is complete.
149-168: LGTM!The Alert UI is well-implemented:
- Clear conditional rendering based on
noteAlertVisibilityState- Distinct messages for "created" vs "updated" scenarios
- Close button properly wired to
closeNoteAlert- Uses semantic
Alertcomponent withwarningintent from shared-ui
169-207: LGTM!The restructured rendering correctly:
- Wraps content in a fragment to accommodate the Alert at the top level
- Maintains proper conditional rendering for NewNote, skeletons, empty state, and notes list
- Preserves the
cn()utility for conditional classes on the note-manager container
1 participant
What?
Fixes weird situation when note is added or updated and it disappears because it no longer matches selected labels.
filtered labels:
When such thing happens, there is a warning alert that appears on the top of notes container..
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.