-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Fix: Limiting Saving flow to flow page #11101
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 pull request removes unused imports and state extraction logic from the header components. Specifically, DataStaxLogo, CustomProductSelector, and the ENABLE_DATASTAX_LANGFLOW feature flag are removed from the appHeaderComponent. The FlowMenu component no longer extracts isFlowLocked from state and adds a guard condition in handleSave. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 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
Additional details and impacted files@@ Coverage Diff @@
## main #11101 +/- ##
==========================================
- Coverage 33.21% 33.21% -0.01%
==========================================
Files 1391 1391
Lines 65849 65849
Branches 9745 9746 +1
==========================================
- Hits 21873 21872 -1
Misses 42854 42854
- Partials 1122 1123 +1
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx (1)
41-50: Remove unusedisFlowLockedfrom the state selector.The
isFlowLockedproperty is extracted from the flow state (line 43) but is not included in the destructuring assignment (lines 36-41), making it dead code. IfisFlowLockedis no longer needed, it should be completely removed from the selector function.🔎 Apply this diff to remove the unused extraction:
const { currentFlowName, currentFlowId, currentFlowFolderId, currentFlowIcon, currentFlowGradient, } = useFlowStore( useShallow((state) => ({ - isFlowLocked: state.currentFlow?.locked, currentFlowName: state.currentFlow?.name, currentFlowId: state.currentFlow?.id, currentFlowFolderId: state.currentFlow?.folder_id, currentFlowIcon: state.currentFlow?.icon, currentFlowGradient: state.currentFlow?.gradient, })), );
🧹 Nitpick comments (1)
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx (1)
82-83: Consider conditionally enabling the hotkey to avoid interfering with native browser behavior.The hotkey is registered with
preventDefault: trueunconditionally (React hooks cannot be conditional), which means it prevents the browser's native Cmd+S behavior even when not on the flow page. While the guard at line 76 prevents the save action, users may expect native browser save functionality on other pages (e.g., settings).🔎 Apply this diff to conditionally enable the hotkey:
const changes = useShortcutsStore((state) => state.changesSave); - useHotkeys(changes, handleSave, { preventDefault: true }); + useHotkeys(changes, handleSave, { + preventDefault: true, + enabled: onFlowPage + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx(1 hunks)src/frontend/src/components/core/appHeaderComponent/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/components/FlowMenu/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/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx
src/frontend/src/components/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/components/**/*.{tsx,jsx}: Use React Flow for flow graph visualization with Node, Edge, Controls, and Background components
Use the cn() utility function for combining Tailwind CSS classes in components
Use TypeScript interfaces for defining component props in React components
Files:
src/frontend/src/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/components/FlowMenu/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/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx
🧠 Learnings (5)
📚 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/components/core/appHeaderComponent/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/**/*.{tsx,jsx} : Use Lucide React for icon components in the application
Applied to files:
src/frontend/src/components/core/appHeaderComponent/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/components/core/appHeaderComponent/index.tsxsrc/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx
📚 Learning: 2025-06-16T11:14:04.200Z
Learnt from: dolfim-ibm
Repo: langflow-ai/langflow PR: 8394
File: src/frontend/src/icons/Docling/index.tsx:4-6
Timestamp: 2025-06-16T11:14:04.200Z
Learning: The Langflow codebase consistently uses `React.PropsWithChildren<{}>` as the prop type for all icon components using forwardRef, rather than `React.SVGProps<SVGSVGElement>`. This is an established pattern across hundreds of icon files in src/frontend/src/icons/.
Applied to files:
src/frontend/src/components/core/appHeaderComponent/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/nodes/**/*.{tsx,jsx} : Memoize custom React Flow node components using memo() to prevent unnecessary re-renders
Applied to files:
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx
⏰ 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: Test Docker Images / Test docker images
- GitHub Check: Run Frontend Tests / Determine Test Suites and Shard Distribution
- GitHub Check: Test Starter Templates
🔇 Additional comments (2)
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx (1)
75-80: Guard implementation correctly prevents save on non-flow pages.The early return when
!onFlowPagealigns perfectly with the PR objective to restrict the save functionality to the flow page only.src/frontend/src/components/core/appHeaderComponent/index.tsx (1)
13-13: LGTM! Unused imports removed.The cleanup of unused imports (
DataStaxLogo,CustomProductSelector,ENABLE_DATASTAX_LANGFLOW) simplifies the component's dependencies while retaining the actively usedLANGFLOW_AGENTIC_EXPERIENCEflag.
Adam-Aghili
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.
Minor changes
Description
I noticed that when we click CMD + S on none flow pages the "Saved Successfully" alert banner is shown. We want to limit it to the flow page.
Testcase
Screenshot
Before
Screen.Recording.2025-12-18.at.3.35.23.PM.mov
After
https://github.com/user-attachments/assets/9cda7413-5000-44a2-b184-c546100d1a69
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.