-
Notifications
You must be signed in to change notification settings - Fork 6
Input for labels #338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Input for labels #338
Conversation
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (6)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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: 19276638911 🔗 Artifacts: Download |
There was a problem hiding this 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
🧹 Nitpick comments (7)
src/components/RemoteSources/components/Versions.tsx (1)
28-28: Use thecheckedparameter for clarity.The callback currently uses
isAllVersions(current state) to toggle, butonCheckedChangeprovides acheckedparameter representing the new state. Using it directly is more idiomatic and clearer.Apply this diff:
- <Checkbox checked={isAllVersions} onCheckedChange={() => onChange?.(isAllVersions ? [] : null)} /> all versions + <Checkbox checked={isAllVersions} onCheckedChange={(checked) => onChange?.(checked ? null : [])} /> all versionssrc/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
17-17: Consider using CSS variables for alignment adjustments.The hardcoded
-top-0.5relative positioning appears to be a visual alignment tweak. While functional, magic positioning values can be fragile across different contexts or font changes.Consider extracting this to a CSS variable or documenting why this specific offset is needed for consistent alignment.
src/components/NoteManager/components/SiimpleComponents/common.ts (1)
1-1: Consider using CSS variables for theme colors.The hardcoded hex values for borders and backgrounds may make theme maintenance more difficult. If these colors are already defined in your design system, consider referencing CSS variables instead.
For example:
-export const inputClassNames = "dark:border-[#728D8D] dark:bg-[#475959] bg-[#eef4f4] border-[#a7bbbb]"; +export const inputClassNames = "dark:border-[var(--border-dark)] dark:bg-[var(--bg-dark)] bg-[var(--bg-light)] border-[var(--border-light)]";This approach centralizes color definitions and simplifies future theme updates.
src/components/NoteManager/components/Note.tsx (1)
181-181: Consider icon accessibility and cross-platform rendering.The emoji icon "✏️" may not render consistently across different platforms and screen readers might not announce it appropriately. While the
aria-label="Edit note"helps, consider using an SVG icon from a design system for better consistency.Example with an icon library:
<TinyIconButton data-testid="edit-button" onClick={handleEditClick} aria-label="Edit note" icon={<EditIcon />} />src/components/NoteManager/components/NoteLabels.tsx (3)
60-73: Simplify redundant validation.The
isValidcheck on line 61 and theif (value)check on line 67 are redundant.const handleSelectLabel = (value: string) => { - const isValid = value !== ""; - - if (!isValid) { - return; - } - - if (value) { + if (value.trim()) { const newUniqueLabels = new Set(noteDirty.labels); newUniqueLabels.add(value); handleNoteLabelsChange(Array.from(newUniqueLabels)); setCurrentInput(""); } };
118-122: Consider refactoring manual onChange invocation.Manually constructing synthetic events to trigger
onChangeis fragile and couples the component to implementation details of the parent.Consider adding a dedicated callback for clearing the input:
type NoteInputProps = ComponentProps<typeof Input> & { onSelectLabel?: (value: string) => void; onDoubleBackspace?: (event: React.KeyboardEvent<HTMLInputElement>) => void; + onClear?: () => void; visibleLabels: string[]; };Then in the parent component (NoteLabelsEdit):
const handleSelectLabel = (value: string) => { if (value.trim()) { const newUniqueLabels = new Set(noteDirty.labels); newUniqueLabels.add(value); handleNoteLabelsChange(Array.from(newUniqueLabels)); } }; const handleClear = () => { setCurrentInput(""); }; <NoteInputWithDropdown // ... onSelectLabel={handleSelectLabel} onClear={handleClear} />And in NoteInputWithDropdown:
const handleItemSelect = (value: string) => { if (inputRef.current) { onSelectLabel?.(value.replace("local/", "")); onClear?.(); // Let parent handle clearing inputRef.current.focus(); } };
210-214: Use cn() utility for conditional classNames.The inline template literal could be replaced with the
cn()utility that's already imported, improving consistency and readability.<div key={item} - className={`px-2 py-1.5 text-sm cursor-pointer transition-colors ${ - index === selectedIndex - ? "bg-accent text-accent-foreground" - : "hover:bg-accent hover:text-accent-foreground" - }`} + className={cn( + "px-2 py-1.5 text-sm cursor-pointer transition-colors", + index === selectedIndex + ? "bg-accent text-accent-foreground" + : "hover:bg-accent hover:text-accent-foreground" + )} onClick={() => handleItemSelect(item)}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
tools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-after-note-activation-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-after-note-activation-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-after-note-edit-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-after-note-edit-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-initial-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/notes-tab-initial-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-initial-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-initial-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-with-query-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-with-query-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-with-results-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/search-with-results-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-more-settings-clicked-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-more-settings-clicked-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-dropdown-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-dropdown-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-selected-off-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-selected-off-light-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-selected-on-dark-mode-linux.pngis excluded by!**/*.pngtools/snapshot-tests/tests/basic-snapshots.spec.ts-snapshots/topbar-notes-selected-on-light-mode-linux.pngis excluded by!**/*.png
📒 Files selected for processing (17)
src/components/Label/Label.css(0 hunks)src/components/Label/Label.tsx(2 hunks)src/components/NoteManager/NoteManager.css(0 hunks)src/components/NoteManager/components/Note.tsx(6 hunks)src/components/NoteManager/components/NoteContext.tsx(2 hunks)src/components/NoteManager/components/NoteLabels.tsx(1 hunks)src/components/NoteManager/components/NoteLayout.tsx(3 hunks)src/components/NoteManager/components/SiimpleComponents/Input.tsx(1 hunks)src/components/NoteManager/components/SiimpleComponents/Textarea.tsx(1 hunks)src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx(1 hunks)src/components/NoteManager/components/SiimpleComponents/common.ts(1 hunks)src/components/NoteManager/components/SiimpleComponents/index.ts(1 hunks)src/components/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.tsx(1 hunks)src/components/RemoteSources/components/RemoteSource.tsx(3 hunks)src/components/RemoteSources/components/Versions.tsx(3 hunks)src/components/Search/Search.tsx(2 hunks)src/index.css(0 hunks)
💤 Files with no reviewable changes (3)
- src/index.css
- src/components/NoteManager/NoteManager.css
- src/components/Label/Label.css
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/components/SiimpleComponents/index.tssrc/components/NoteManager/components/SiimpleComponents/Input.tsxsrc/components/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.tsxsrc/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsxsrc/components/Search/Search.tsxsrc/components/NoteManager/components/SiimpleComponents/Textarea.tsxsrc/components/NoteManager/components/SiimpleComponents/common.tssrc/components/NoteManager/components/NoteLabels.tsxsrc/components/RemoteSources/components/Versions.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.tsxsrc/components/RemoteSources/components/RemoteSource.tsxsrc/components/NoteManager/components/NoteLayout.tsx
🧠 Learnings (5)
📓 Common learnings
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.
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.
📚 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/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.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/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.tsxsrc/components/NoteManager/components/SiimpleComponents/Textarea.tsxsrc/components/NoteManager/components/NoteLabels.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.tsxsrc/components/NoteManager/components/NoteLayout.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/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.tsxsrc/components/NoteManager/components/SiimpleComponents/Textarea.tsxsrc/components/NoteManager/components/NoteLabels.tsxsrc/components/NoteManager/components/NoteContext.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.tsxsrc/components/RemoteSources/components/RemoteSource.tsxsrc/components/NoteManager/components/NoteLayout.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: Tailwind CSS v4 is used; avoid non-default width utilities like border-b-1 unless configured. Prefer border-b (1px) or border-b-[1px]. File context: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/Label/Label.tsx
🧬 Code graph analysis (8)
src/components/NoteManager/components/SiimpleComponents/Input.tsx (2)
src/components/NoteManager/components/SiimpleComponents/index.ts (1)
NoteSimpleInput(1-1)src/components/NoteManager/components/SiimpleComponents/common.ts (1)
inputClassNames(1-1)
src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
src/components/NoteManager/components/SiimpleComponents/index.ts (1)
TinyIconButton(3-3)
src/components/NoteManager/components/SiimpleComponents/Textarea.tsx (1)
src/components/NoteManager/components/SiimpleComponents/common.ts (1)
inputClassNames(1-1)
src/components/NoteManager/components/NoteLabels.tsx (6)
src/components/NoteManager/components/NoteContext.tsx (1)
useNoteContext(19-25)src/components/Label/Label.tsx (1)
Label(5-46)src/components/NotesProvider/hooks/useLabels.ts (1)
prefixLabel(28-30)src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
TinyIconButton(4-25)src/components/NotesProvider/NotesProvider.tsx (2)
NotesContext(18-18)INotesContext(20-40)src/components/NoteManager/components/SiimpleComponents/Input.tsx (1)
NoteSimpleInput(6-10)
src/components/NoteManager/components/NoteContext.tsx (1)
src/components/NotesProvider/types/StorageNote.ts (1)
IStorageNote(5-5)
src/components/NoteManager/components/Note.tsx (2)
src/components/NoteManager/components/NoteLayout.tsx (1)
NoteLayout(65-71)src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
TinyIconButton(4-25)
src/components/Label/Label.tsx (1)
src/components/Label/color-utils.ts (1)
hslColorToCss(1-3)
src/components/NoteManager/components/NoteLayout.tsx (2)
src/components/NoteManager/components/SiimpleComponents/Textarea.tsx (1)
NoteSimpleTextarea(6-10)src/components/NoteManager/components/NoteLabels.tsx (1)
NoteLabels(11-43)
⏰ 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 (17)
src/components/RemoteSources/components/Versions.tsx (1)
41-50: LGTM!The Checkbox component is correctly integrated with proper handling of the
checkedparameter inonCheckedChange. The logic correctly adds or removes version hashes based on the checked state.src/components/RemoteSources/components/RemoteSource.tsx (3)
43-52: LGTM!The
toggleEnabledcallback is correctly refactored to accept the new checked state as a boolean parameter, which aligns with the Checkbox component'sonCheckedChangeAPI. The logic properly updatesisEnabledwith the provided value.
57-57: LGTM!The migration from native
<input>elements to the sharedInputcomponent is clean and maintains the same event handler pattern withe.target.value. The placeholder props are correctly preserved.Also applies to: 59-59
78-78: LGTM!The Checkbox component is correctly integrated with the
toggleEnabledcallback, which now accepts the boolean parameter provided byonCheckedChange.src/components/PdfViewer/NoteRenderer/components/HighlightNote/HighlightNote.tsx (1)
111-111: Remove the commented-out code; labels are intentionally excluded from HighlightNote.The NoteLabels migration to context-based rendering is complete in Note.tsx. HighlightNote is architecturally separate and was never intended to use the NoteLayout context system. The commented code at line 111 is remnant dead code from the old implementation and should be removed.
- Line 111: Remove {/*<NoteLabels note={note} />*/}src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
4-24: Well-structured accessible button component.The component follows good practices with required
aria-label, proper prop spreading, and type safety viaComponentProps<typeof Button>.src/components/NoteManager/components/SiimpleComponents/index.ts (1)
1-3: LGTM!Clean barrel export pattern for the Siimple components.
src/components/NoteManager/components/SiimpleComponents/Input.tsx (1)
6-10: LGTM!Clean wrapper component that properly composes className using
cnand forwards all props. The use ofComponentProps<typeof Input>ensures type safety.src/components/NoteManager/components/NoteLayout.tsx (3)
1-1: Good use of type-only import.Importing
Textareaas a type-only dependency reduces the bundle size since it's only used for type inference inComponentProps<typeof Textarea>.
54-61: LGTM!The refactor to use
NoteSimpleTextareaproperly delegates className composition to the wrapper component while maintaining all the original functionality (ref, autoFocus, keyboard handlers, etc.).
70-70: Clean API surface extension.Adding
Labels: NoteLabelsto theNoteLayoutobject provides a cohesive compound component pattern for note rendering.src/components/NoteManager/components/SiimpleComponents/Textarea.tsx (1)
6-10: LGTM!Consistent wrapper pattern matching
NoteSimpleInput. Properly typed and composes className usingcn.src/components/NoteManager/components/Note.tsx (1)
178-178: Clean integration of label management.The refactor to use
NoteLayout.Labelsprovides better separation of concerns and aligns with the compound component pattern.Also applies to: 192-192
src/components/NoteManager/components/NoteContext.tsx (1)
4-4: Improved type safety for note context.Upgrading
noteDirtyfrom{ content: string }toIStorageNoteand addinghandleNoteLabelsChangeprovides better type coverage and supports the label editing functionality. The wider type ensures all note properties (including labels) are properly tracked during editing.Also applies to: 13-14
src/components/NoteManager/components/NoteLabels.tsx (2)
47-47: Verify direct context access pattern.This directly accesses NotesContext with type casting instead of using a safe hook pattern (like useNoteContext). While NotesContext should always provide a value, this bypasses null-safety checks.
Consider whether a
useNotesContext()hook (similar touseNoteContext()at line 8) would be more consistent and type-safe:// In NotesProvider.tsx export const useNotesContext = () => { const context = useContext(NotesContext); if (!context) { throw new Error("useNotesContext must be used within NotesProvider"); } return context; };Then use it here:
- const { labels } = useContext(NotesContext) as INotesContext; + const { labels } = useNotesContext();
113-149: Keyboard navigation logic is well-implemented.The keyboard handling for Enter, Backspace (with double-press confirmation), Escape, and Arrow keys provides a solid UX. The double-backspace pattern with visual feedback (warning intent) is a thoughtful touch for preventing accidental label removal.
src/components/Search/Search.tsx (1)
49-56: Verify Input component supports ref forwarding and autoFocus via @fluffylabs/shared-ui Storybook.The code change is syntactically correct—ref is passed as a regular prop (aligned with React 19+ conventions) and standard input props are preserved. However, since the Input component is from the external @fluffylabs/shared-ui v0.4.5 package and the ref is used for focus operations (lines 37, 43), confirm in the package's Storybook documentation (fluffylabs.dev/shared-ui) that:
- The Input component properly forwards refs to the underlying HTMLInputElement
- The autoFocus prop is supported
- The styling and behavior are as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/Label/Label.tsx(2 hunks)src/components/NoteManager/NoteManager.tsx(3 hunks)src/components/NoteManager/components/Note.tsx(6 hunks)src/components/NoteManager/components/NoteLabels.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/NoteManager.tsxsrc/components/NoteManager/components/NoteLabels.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.tsx
🧠 Learnings (3)
📚 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/NoteLabels.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.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/NoteManager.tsxsrc/components/NoteManager/components/NoteLabels.tsxsrc/components/NoteManager/components/Note.tsxsrc/components/Label/Label.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: Tailwind CSS v4 is used; avoid non-default width utilities like border-b-1 unless configured. Prefer border-b (1px) or border-b-[1px]. File context: src/components/Outline/OutlineLink.tsx.
Applied to files:
src/components/Label/Label.tsx
🧬 Code graph analysis (3)
src/components/NoteManager/components/NoteLabels.tsx (6)
src/components/NoteManager/components/NoteContext.tsx (1)
useNoteContext(19-25)src/components/Label/Label.tsx (1)
Label(5-46)src/components/NotesProvider/hooks/useLabels.ts (1)
prefixLabel(28-30)src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
TinyIconButton(4-25)src/components/NotesProvider/NotesProvider.tsx (2)
NotesContext(18-18)INotesContext(20-40)src/components/NoteManager/components/SiimpleComponents/Input.tsx (1)
NoteSimpleInput(6-10)
src/components/NoteManager/components/Note.tsx (3)
src/components/NoteManager/components/NoteLayout.tsx (1)
NoteLayout(65-71)src/components/NoteManager/components/SiimpleComponents/index.ts (1)
TinyIconButton(3-3)src/components/NoteManager/components/SiimpleComponents/TinyIconButton.tsx (1)
TinyIconButton(4-25)
src/components/Label/Label.tsx (1)
src/components/Label/color-utils.ts (1)
hslColorToCss(1-3)
⏰ 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/NoteManager.tsx (2)
3-3: LGTM!The migration to the shared-ui
Textareacomponent is clean and preserves all necessary props and behaviors. The flex column layout appropriately stacks the input elements vertically.Also applies to: 79-87
90-92: LGTM!The
Buttoncomponent correctly implements the disabled logic and preserves the click handler. Thevariant="secondary"provides appropriate visual hierarchy for the add action.src/components/NoteManager/components/Note.tsx (4)
1-1: LGTM!Import updates correctly support the new shared-ui components and TinyIconButton usage.
Also applies to: 4-4, 10-10
57-63: LGTM!Both callbacks correctly use functional updaters for stable identity and optimal re-render behavior. The
handleNoteLabelsChangeoptimization from previous feedback is properly implemented.
131-131: LGTM!The
handleNoteLabelsChangecallback is correctly added to both the context value and theuseMemodependency array, ensuring proper propagation to child components.Also applies to: 143-143
172-172: LGTM!The integration of
NoteLayout.Labelsin both read-only and editing states is consistent. TheTinyIconButtonis correctly configured with proper accessibility attributes and test ID.Also applies to: 175-175, 186-186
src/components/NoteManager/components/NoteLabels.tsx (4)
11-39: LGTM!The
NoteLabelscomponent correctly uses context for data access and renders labels with appropriate removal buttons in edit mode. The label filtering logic correctly removes the selected label from the array.
41-92: LGTM!The
NoteLabelsEditcomponent correctly filters available labels, prevents duplicates, and implements a helpful backspace-to-edit UX. TherequestAnimationFrameusage on line 78-80 ensures the removed label value populates the input after state updates complete, which is an acceptable pattern for this use case.
108-144: LGTM!The keyboard navigation implementation is comprehensive and handles all expected interactions: Enter for selection, Escape for closing, and arrow keys with wrap-around. The double-backspace pattern provides a discoverable way to edit the last label.
165-223: LGTM!The popover rendering includes proper ARIA attributes for accessibility, visual feedback for the backspace state, and handles both keyboard and mouse interactions correctly. The
biome-ignorecomment on line 203 is justified since keyboard events are handled through the input element.src/components/Label/Label.tsx (3)
1-1: LGTM!The function signature correctly adds the
endSlotprop for flexible trailing content, and the import updates support the new ReactNode type.Also applies to: 5-17
22-31: LGTM!The CSS variable approach provides clean theme support with dynamic colors. The
useMemodependencies correctly track all values used in the style computation, and the memoizeddimmedMainColorensures stable object identity.
33-45: LGTM!The rendering correctly uses CSS variables with Tailwind 4.x syntax, and the
truncatespan handles text overflow gracefully. ThedimColorfunction provides appropriate contrast for both light and dark themes. TheendSlotplacement enables flexible composition with trailing icons or buttons.Also applies to: 48-52
Summary by CodeRabbit
New Features
Bug Fixes
Style