-
Notifications
You must be signed in to change notification settings - Fork 8.2k
feat: Improvements to Sticky Notes #11057
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR refactors note node sizing to use explicit width and height properties instead of measured dimensions. It introduces a DEFAULT_NOTE_SIZE constant (324px), reduces minimum dimensions (MIN_WIDTH to 260, MIN_HEIGHT to 100), updates the NoteNodeType interface to include width and height fields, and adjusts component logic to respect these new explicit sizing properties. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (38.80%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11057 +/- ##
==========================================
- Coverage 33.21% 33.04% -0.18%
==========================================
Files 1391 1390 -1
Lines 65849 65464 -385
Branches 9745 9658 -87
==========================================
- Hits 21873 21631 -242
+ Misses 42854 42711 -143
Partials 1122 1122
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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: 0
🧹 Nitpick comments (1)
src/frontend/src/constants/constants.ts (1)
885-887: Consider whether MIN_HEIGHT=100 is appropriate for note usability.The separation of minimum dimensions (260x100) from default size (324) is architecturally sound and enables flexible note sizing. However,
NOTE_NODE_MIN_HEIGHT = 100allows notes to be resized to just 100px tall, which may be too small for practical use.Consider whether a slightly larger minimum height (e.g., 150-200) would prevent users from accidentally creating unusably small notes while still providing resizing flexibility. If 100px is intentional for compact notes, this can be disregarded.
-export const NOTE_NODE_MIN_HEIGHT = 100; +export const NOTE_NODE_MIN_HEIGHT = 150; // Adjust if 100px is too small
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.secrets.baseline(2 hunks)src/frontend/src/CustomNodes/NoteNode/index.tsx(4 hunks)src/frontend/src/constants/constants.ts(1 hunks)src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management
Files:
src/frontend/src/constants/constants.tssrc/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
src/frontend/src/**/*.{tsx,jsx,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Use Tailwind CSS for styling
Files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
src/frontend/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{tsx,jsx}: Implement dark mode support using the useDarkMode hook and dark store
Use Lucide React for icon components in the application
Files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:46:45.790Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:45.790Z
Learning: Applies to src/frontend/src/components/nodes/**/*.{tsx,jsx} : Memoize custom React Flow node components using memo() to prevent unnecessary re-renders
Applied to files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
📚 Learning: 2025-11-24T19:46:45.790Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:45.790Z
Learning: Applies to src/frontend/src/components/**/*.{tsx,jsx} : Use React Flow for flow graph visualization with Node, Edge, Controls, and Background components
Applied to files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: Custom React Flow node types should be implemented as memoized components, using Handle components for connection points and supporting optional icons and labels.
Applied to files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
📚 Learning: 2025-06-23T12:46:52.420Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/icons.mdc:0-0
Timestamp: 2025-06-23T12:46:52.420Z
Learning: Export custom icon components in React using React.forwardRef to ensure proper ref forwarding and compatibility with parent components.
Applied to files:
src/frontend/src/CustomNodes/NoteNode/index.tsx
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: React Flow should be used for flow graph visualization, with nodes and edges passed as props, and changes handled via onNodesChange and onEdgesChange callbacks.
Applied to files:
src/frontend/src/CustomNodes/NoteNode/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
🧬 Code graph analysis (2)
src/frontend/src/CustomNodes/NoteNode/index.tsx (1)
src/frontend/src/constants/constants.ts (3)
DEFAULT_NOTE_SIZE(887-887)NOTE_NODE_MIN_WIDTH(885-885)NOTE_NODE_MIN_HEIGHT(886-886)
src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx (1)
src/frontend/src/constants/constants.ts (1)
DEFAULT_NOTE_SIZE(887-887)
🪛 Gitleaks (8.30.0)
.secrets.baseline
[high] 1412-1412: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 1420-1420: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (4)
- GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
- GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Test Starter Templates
🔇 Additional comments (8)
.secrets.baseline (1)
1406-1422: LGTM! Routine secrets baseline maintenance.The line number updates correctly reflect the addition of new constants in
src/frontend/src/constants/constants.ts. The static analysis hints flagged by Gitleaks appear to be the same entries that existed before (based on matching hashed_secret values), just at new line positions.Also applies to: 1531-1531
src/frontend/src/CustomNodes/NoteNode/index.tsx (4)
6-6: LGTM! Centralized default size constant.Importing
DEFAULT_NOTE_SIZEfrom constants provides a single source of truth for the default note dimensions.
149-150: Correct use of minimum dimensions for resizer constraints.Using
NOTE_NODE_MIN_WIDTH(260) andNOTE_NODE_MIN_HEIGHT(100) directly allows notes to be resized smaller than the default size (324), which is the intended behavior. The previousMath.maxapproach would have prevented resizing below the default.
165-166: Proper use of explicit dimensions for controlled sizing.Rendering with explicit
widthandheight(rather thanminWidth/minHeight) ensures the node displays at the exact dimensions stored in state, which is essential for the explicit sizing approach introduced in this PR.
106-107: The switch frommeasuredto explicit dimensions is correct and requires no changes.All code paths properly initialize note node dimensions:
- New notes created via PageComponent: width/height explicitly set to
DEFAULT_NOTE_SIZE- Duplicated notes via paste(): width/height use the fallback to
DEFAULT_NOTE_SIZE(note: duplicated notes reset to default size)- Flow loading: nodes loaded from API retain their stored dimensions
- Missing dimensions: fallback to
DEFAULT_NOTE_SIZEprovides backward compatibilityThe implementation is safe and handles edge cases appropriately.
src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx (3)
27-27: LGTM! Import added for new sizing constant.The
DEFAULT_NOTE_SIZEimport supports the explicit sizing changes throughout this file.
165-166: Improved preview sizing for note placement.Using
DEFAULT_NOTE_SIZE(324) for the shadow box preview is better UX than using the minimum dimensions (260x100), as it shows users the actual size the note will be created at. This is a sensible functional improvement.
660-661: Essential initialization of explicit dimensions for new notes.Setting explicit
widthandheightproperties when creating new note nodes ensures the state correctly reflects the node's dimensions from creation. This is critical for the explicit sizing approach and enables proper persistence and synchronization.
| export const NOTE_NODE_MIN_WIDTH = 324; | ||
| export const NOTE_NODE_MIN_HEIGHT = 324; | ||
| export const NOTE_NODE_MIN_WIDTH = 260; | ||
| export const NOTE_NODE_MIN_HEIGHT = 100; |
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.
these min numbers come from product.
viktoravelino
left a comment
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.
lgtm
This pull request updates the sizing logic and minimum constraints for Note nodes in the frontend, making note sizing more consistent and flexible. The changes introduce a new
DEFAULT_NOTE_SIZEconstant, reduce the minimum allowed size for Note nodes, and update how node dimensions are handled and rendered. Additionally, the.secrets.baselinefile is updated to reflect line number changes and a new generation timestamp.Note node sizing and behavior improvements:
DEFAULT_NOTE_SIZEand reducedNOTE_NODE_MIN_WIDTHto 260 andNOTE_NODE_MIN_HEIGHTto 100 inconstants.ts, making notes more flexible in size.NoteNodeto usewidthandheightdirectly from node data (falling back toDEFAULT_NOTE_SIZE), and set explicitwidthandheightstyles instead ofminWidth/minHeight. Also, the resizer now uses the new minimum sizes. [1] [2] [3] [4] [5]PageComponentto useDEFAULT_NOTE_SIZEfor the shadow box dimensions and to set new notes' initial width and height toDEFAULT_NOTE_SIZE. [1] [2] [3]Original PR
#10149