Skip to content

Conversation

@olayinkaadelakun
Copy link
Collaborator

@olayinkaadelakun olayinkaadelakun commented Dec 18, 2025

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

  1. When we click CMD + S on the setting page we shouldn't see the alert banner
  2. If we click CMD + S on the flow page we should see the the alert banner

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

  • Chores
    • Removed unused UI components from the application header
    • Streamlined flow state management and menu navigation logic
    • Cleaned up feature flag configuration

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This 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

Cohort / File(s) Summary
FlowMenu component
src/frontend/src/components/core/appHeaderComponent/components/FlowMenu/index.tsx
Removed isFlowLocked from flow state extraction; added early guard in handleSave to no-op if onFlowPage is falsy.
App header component
src/frontend/src/components/core/appHeaderComponent/index.tsx
Removed DataStaxLogo and CustomProductSelector imports; removed ENABLE_DATASTAX_LANGFLOW feature flag from imports, leaving only LANGFLOW_AGENTIC_EXPERIENCE.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Simple import removals and unused state extraction cleanup
  • Straightforward guard condition addition with no complex logic changes

Pre-merge checks and finishing touches

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 2 warnings)
Check name Status Explanation Resolution
Test Coverage For New Implementations ❌ Error PR implements bug fix restricting save alert banner to flow page only, but lacks corresponding test coverage for this behavior change. Add Playwright regression tests verifying save alert does not appear on non-flow pages and does appear on flow page, plus unit tests for handleSave function's onFlowPage guard condition.
Test Quality And Coverage ⚠️ Warning The PR implements a guard clause to prevent saves on non-flow pages, but this critical behavioral change lacks test validation. Add tests validating the guard clause for both onFlowPage true/false states and verify setSuccessData behavior accordingly.
Excessive Mock Usage Warning ⚠️ Warning The FlowMenu test uses excessive mocking (15+ mocks) that obscures the PR's key fix: the new if (!onFlowPage) return; guard in handleSave is untested because the test hardcodes onFlowPage: true, preventing verification that the guard actually prevents saves when false. Add a test case with onFlowPage: false to verify the guard works, replace component mocks with real implementations or stubs, and use setup helpers to reduce mock boilerplate while focusing on actual component behavior.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix: Limiting Saving flow to flow page' directly addresses the main objective of the PR: restricting the CMD+S save shortcut to the flow page only.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Test File Naming And Structure ✅ Passed Test file exists and follows correct patterns and structure.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 17%
16.66% (4695/28165) 10% (2180/21796) 10.94% (677/6183)

Unit Test Results

Tests Skipped Failures Errors Time
1829 0 💤 0 ❌ 0 🔥 23.98s ⏱️

@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.21%. Comparing base (4a3462a) to head (5d3471b).

Files with missing lines Patch % Lines
...e/appHeaderComponent/components/FlowMenu/index.tsx 0.00% 0 Missing and 1 partial ⚠️
...d/src/components/core/appHeaderComponent/index.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
frontend 15.36% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...e/appHeaderComponent/components/FlowMenu/index.tsx 68.42% <0.00%> (-1.32%) ⬇️
...d/src/components/core/appHeaderComponent/index.tsx 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 unused isFlowLocked from the state selector.

The isFlowLocked property is extracted from the flow state (line 43) but is not included in the destructuring assignment (lines 36-41), making it dead code. If isFlowLocked is 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: true unconditionally (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

📥 Commits

Reviewing files that changed from the base of the PR and between 4a3462a and 5d3471b.

📒 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.tsx
  • src/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.tsx
  • src/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.tsx
  • src/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.tsx
  • src/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.tsx
  • src/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 !onFlowPage aligns 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 used LANGFLOW_AGENTIC_EXPERIENCE flag.

Copy link
Collaborator

@Adam-Aghili Adam-Aghili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants