Skip to content

Conversation

@olayinkaadelakun
Copy link
Collaborator

@olayinkaadelakun olayinkaadelakun commented Nov 28, 2025

Description
This is arises on safari because of the sticky style applied

For the Language model component, the "refresh list" was float around due to the fact of safari weirdness with the sticky styling. After evaluation i noticed that we didn't need the sticky component as the list dynamically grows and the "Refresh list" is always just placed at the buttom

Testcase

  1. Ensure that when you zoom in or out on Safari we don't see the "refresh list" button moving around
  2. Regression: ensure on other browsers that are affect by this change

Screenshot
Before Change
Screenshot 2025-11-28 at 9 56 36 AM

After change
Screenshot 2025-11-28 at 9 56 56 AM

Summary by CodeRabbit

  • Style

    • Adjusted dropdown refresh container layout—no longer fixed to bottom when scrolling.
  • Tests

    • Applied formatting improvements to test suite.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Walkthrough

Removes sticky positioning from the dropdown footer refresh list container, changing its layout to scroll naturally within the popover. Additionally includes minor formatting and indentation adjustments to a test suite file without functional changes.

Changes

Cohort / File(s) Summary
Dropdown layout adjustment
src/frontend/src/components/core/dropdownComponent/index.tsx
Removed sticky bottom-0 classes from the refresh list container footer, transitioning from sticky bottom-0 border-t bg-background to border-t bg-background, changing the element's positioning behavior during scroll.
Test file formatting
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
Normalized whitespace and indentation across test definitions, assertions, and await expressions with no functional or control-flow changes.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Layout change to dropdown footer—verify sticky positioning removal doesn't negatively impact scrolling UX in the dropdown popover
  • Test formatting is cosmetic only

Possibly related PRs

Suggested labels

bug, lgtm

Suggested reviewers

  • Cristhianzl

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 removes sticky positioning from dropdown component's refresh button but includes no regression tests to verify the fix or prevent regression. Add a Playwright E2E or Jest unit test to verify the dropdown component's refresh button behavior after removing sticky positioning.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Test Quality And Coverage ⚠️ Warning PR removes sticky positioning from refresh list button but adds no tests to validate the fix or prevent regressions in Safari zoom behavior. Add tests validating that the refresh list button remains stationary during zoom interactions and confirm the sticky CSS class removal has no unintended side effects.
✅ 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 directly addresses the main change: removing sticky positioning from the refresh list button to fix Safari zoom behavior. It clearly summarizes the primary fix.
Test File Naming And Structure ✅ Passed Test files follow correct naming patterns with .spec.ts convention and proper directory structure in src/frontend/tests/core/unit/. Tests use Playwright framework with proper fixtures, setup logic, data-testid selectors, and cover both positive and negative scenarios.
Excessive Mock Usage Warning ✅ Passed The test file is a Playwright E2E test that appropriately uses real browser interactions without excessive mocking, validating actual user workflows.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refresh-list-safari-fix

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: 15%
15.24% (4188/27479) 8.46% (1778/20993) 9.57% (579/6049)

Unit Test Results

Tests Skipped Failures Errors Time
1638 0 💤 0 ❌ 0 🔥 19.786s ⏱️

@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.39%. Comparing base (271c7ff) to head (c097756).
⚠️ Report is 8 commits behind head on main.

❌ Your project status has failed because the head coverage (40.04%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10777   +/-   ##
=======================================
  Coverage   32.39%   32.39%           
=======================================
  Files        1368     1368           
  Lines       63414    63414           
  Branches     9373     9373           
=======================================
  Hits        20541    20541           
  Misses      41840    41840           
  Partials     1033     1033           
Flag Coverage Δ
frontend 14.08% <ø> (ø)

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

Files with missing lines Coverage Δ
...nd/src/components/core/dropdownComponent/index.tsx 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

🧹 Nitpick comments (1)
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (1)

5-103: Playwright test remains behaviorally sound; consider adding a brief scenario comment

The test structure, selectors, and assertions still exercise real user behavior (bootstrap → open modal → validate error on invalid code → ensure code persists), and your changes here are effectively formatting only, so behavior is preserved and looks good.

Given the repo’s testing guidelines for *.spec.ts under src/frontend/**, you might want to add a short comment above this test explaining the scenario and expected outcome (e.g., “verifies CodeArea modal validates syntax errors and saves valid custom component code”), to make the test’s intent clearer when scanning the suite. This is non-blocking and can be done in a follow‑up.

📜 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 271c7ff and c097756.

📒 Files selected for processing (2)
  • src/frontend/src/components/core/dropdownComponent/index.tsx (1 hunks)
  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx}: Use @pytest.mark.asyncio decorator for async frontend tests; structure tests to verify component behavior, state updates, and proper cleanup
Frontend tests should validate component rendering, user interactions, state management, and async operations using appropriate testing libraries (React Testing Library, Vitest, Jest, etc.)
Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes

Files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py}

📄 CodeRabbit inference engine (Custom checks)

**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py}: Review test files for excessive use of mocks that may indicate poor test design - check if tests have too many mock objects that obscure what's actually being tested
Warn when mocks are used instead of testing real behavior and interactions
Suggest using real objects or test doubles when mocks become excessive
Ensure mocks are used appropriately for external dependencies, not core logic
Test files should have descriptive test function names that explain what is being tested
Tests should be organized logically with proper setup and teardown
Consider including edge cases and error conditions for comprehensive coverage
Verify tests cover both positive and negative scenarios where appropriate
Tests should cover the main functionality being implemented
Check that tests are not just smoke tests but actually validate behavior
Ensure tests follow the project's testing patterns (pytest for backend, Playwright for frontend)
For API endpoints, verify both success and error response testing

Files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
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/dropdownComponent/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/dropdownComponent/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/dropdownComponent/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/dropdownComponent/index.tsx
🧠 Learnings (11)
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx} : Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Ensure tests follow the project's testing patterns (pytest for backend, Playwright for frontend)

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Tests should cover the main functionality being implemented

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Consider including edge cases and error conditions for comprehensive coverage

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 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/**/*.test.{ts,tsx} : Write component tests using React Testing Library with render(), screen, and fireEvent utilities

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/frontend/**/*.{test,spec}.{ts,tsx,js,jsx} : Frontend tests should validate component rendering, user interactions, state management, and async operations using appropriate testing libraries (React Testing Library, Vitest, Jest, etc.)

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-07-11T22:12:46.255Z
Learnt from: namastex888
Repo: langflow-ai/langflow PR: 9018
File: src/frontend/src/modals/apiModal/codeTabs/code-tabs.tsx:244-244
Timestamp: 2025-07-11T22:12:46.255Z
Learning: In src/frontend/src/modals/apiModal/codeTabs/code-tabs.tsx, the inconsistent showLineNumbers setting between Step 1 (false) and Step 2 (true) in the API modal is intentional to prevent breaking the modal height. Step 1 uses showLineNumbers={false} to save vertical space while Step 2 uses showLineNumbers={true} for better readability of longer code.

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Check that tests are not just smoke tests but actually validate behavior

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx,test_*.py} : Ensure mocks are used appropriately for external dependencies, not core logic

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 Learning: 2025-11-24T19:47:40.400Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2025-11-24T19:47:40.400Z
Learning: Applies to **/*.test.{ts,tsx} : Frontend tests should use *.test.ts or *.test.tsx with Playwright

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
📚 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: Frontend tests should be written using testing-library/react, with both component and integration tests verifying rendering, user interaction, and data loading.

Applied to files:

  • src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts
🧬 Code graph analysis (1)
src/frontend/tests/core/unit/codeAreaModalComponent.spec.ts (1)
src/frontend/tests/utils/await-bootstrap-test.ts (1)
  • awaitBootstrapTest (4-67)
⏰ 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 Tests / Determine Test Suites and Shard Distribution
  • GitHub Check: Test Docker Images / Test docker images
  • GitHub Check: Run Frontend Unit Tests / Frontend Jest Unit Tests
  • GitHub Check: Test Starter Templates
🔇 Additional comments (1)
src/frontend/src/components/core/dropdownComponent/index.tsx (1)

691-712: Removing sticky from the refresh footer matches the Safari bugfix intent

Dropping sticky bottom-0 in favor of a plain border-t bg-background wrapper keeps the “Refresh list” button behavior and test id intact while eliminating the sticky layout that was causing the Safari zoom issue. The action wiring (handleRefreshButtonPress) and visibility conditions are unchanged.

Please double‑check in Safari and at least one Chromium/Firefox build with a long options list that:

  • The button no longer “floats” when zooming.
  • It remains reachable at the bottom of the scrollable list without layout glitches.

Copy link
Collaborator

@Wallgau Wallgau left a comment

Choose a reason for hiding this comment

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

Great Work!

@github-actions github-actions bot added the lgtm This PR has been approved by a maintainer label Nov 28, 2025
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.

LGTM! Thank you @olayinkaadelakun

Tested accross FF, chrome, safari and Edge

FF:

Screen.Recording.2025-11-28.at.1.07.28.PM.mov

Chrome:

Screen.Recording.2025-11-28.at.1.07.58.PM.mov

Safari:

Screen.Recording.2025-11-28.at.1.08.42.PM.mov

Edge:

Screen.Recording.2025-11-28.at.1.09.28.PM.mov

@Adam-Aghili
Copy link
Collaborator

Adam-Aghili commented Nov 28, 2025

1 somewhat related issues I noticed Agent component does not have refresh list across all browsers. I don't know if that is intentional. If there is no issue for it, it might be worth asking in private/eng.

Screenshot 2025-11-28 at 1 10 15 PM

@olayinkaadelakun
Copy link
Collaborator Author

1 somewhat related issues I noticed Agent component does not have refresh list across all browsers. I don't know if that is intentional. If there is no issue for it, it might be worth asking in private/eng.

Screenshot 2025-11-28 at 1 10 15 PM

Ill look into that

Copy link
Member

@Cristhianzl Cristhianzl left a comment

Choose a reason for hiding this comment

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

lgtm

@Adam-Aghili
Copy link
Collaborator

@olayinkaadelakun if we do want refresh list in agent component I would suggest making that a seperate PR

@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2025
@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2025
@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 1, 2025
@olayinkaadelakun olayinkaadelakun added this pull request to the merge queue Dec 1, 2025
Merged via the queue into main with commit 461a032 Dec 1, 2025
83 of 84 checks passed
@olayinkaadelakun olayinkaadelakun deleted the refresh-list-safari-fix branch December 1, 2025 19:49
@Adam-Aghili
Copy link
Collaborator

I have this listed as needed for 1.7.0. if that is still true please cherrypick these changes into release-1.7.0

Example: #10738

olayinkaadelakun added a commit that referenced this pull request Dec 1, 2025
…10777)

* remove sticky as it was causing the refresh list to float on safari

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes

---------

Co-authored-by: Olayinka Adelakun <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
olayinkaadelakun added a commit that referenced this pull request Dec 1, 2025
#10827)

Fix: Allow refresh list button to stay stagnant while zoom (Safari) (#10777)

* remove sticky as it was causing the refresh list to float on safari

* [autofix.ci] apply automated fixes

* [autofix.ci] apply automated fixes

---------

Co-authored-by: Olayinka Adelakun <[email protected]>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants