Skip to content

Add right-click context menu to request tabs with MenuDropdown ##6502

Merged
bijin-bruno merged 10 commits intousebruno:mainfrom
abhishek-bruno:feat/request-tab-right-click-menu-v2
Dec 24, 2025
Merged

Add right-click context menu to request tabs with MenuDropdown ##6502
bijin-bruno merged 10 commits intousebruno:mainfrom
abhishek-bruno:feat/request-tab-right-click-menu-v2

Conversation

@abhishek-bruno
Copy link
Member

@abhishek-bruno abhishek-bruno commented Dec 24, 2025

Description

JIRA

Add right-click context menu to request tabs with MenuDropdown

Worked on top of #6459

Contribution Checklist:

  • I've used AI significantly to create this pull request
  • The pull request only addresses one issue or adds one feature.
  • The pull request does not introduce any breaking changes
  • I have added screenshots or gifs to help explain the change if applicable.
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

Summary by CodeRabbit

  • Refactor

    • Consolidated dropdown/menu behavior to a single MenuDropdown flow and standardized how tabs provide positioning refs.
  • Style

    • Revamped dropdown/menu styling and item states (focus, hover, active, disabled, delete) for clearer visuals and accessibility.
  • Bug Fixes

    • Improved right-click and menu alignment by making dropdown mounting target configurable and using fixed positioning.
  • Chore

    • Removed legacy wrapper and added a dedicated dropdown container exposed via the sidebar context.
  • Tests

    • Updated test helper to locate overflow tab items in the new DOM structure.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 24, 2025

Walkthrough

Dropdowns gained an optional appendTo prop and now render via Tippy's render prop. Multiple consumers were migrated from Dropdown to a new MenuDropdown using imperative refs and a shared dropdownContainerRef from SidebarAccordionContext; MenuDropdown's previous StyledWrapper file was removed and styling consolidated.

Changes

Cohort / File(s) Summary
Dropdown component
packages/bruno-app/src/components/Dropdown/index.js
Adds public appendTo prop; composes tippyProps with `appendTo
Dropdown styling
packages/bruno-app/src/components/Dropdown/StyledWrapper.js
Reworked CSS structure and class names (.label-item, .dropdown-item, .dropdown-right-section, .dropdown-separator); updated focus/hover/disabled/delete-item styles and layout/alignment.
MenuDropdown implementation
packages/bruno-app/src/ui/MenuDropdown/index.js
Removed external StyledWrapper wrapper; renders Dropdown content directly and relocated header/footer rendering into the menu content.
MenuDropdown styling removed
packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
File deleted — default export StyledWrapper removed.
RequestTabs → MenuDropdown integration
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js, packages/bruno-app/src/components/RequestTabs/index.js
Replaced Dropdown with MenuDropdown; added tabLabelRef and menuDropdownRef; introduced getTabLabelRect for positioning; right-click now opens MenuDropdown imperatively; centralized menuItems; adjusted handler signatures (e.g., handleCloseTab now accepts tabUid).
Sidebar dropdown mount context
packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js, packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js, packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
Added dropdownContainerRef to SidebarAccordionContext (useRef) and wrapped children with a container ref; consumers pass `appendTo={dropdownContainerRef?.current
RequestTabs container ref
packages/bruno-app/src/components/RequestTabs/index.js
Added collectionTabsRef and pass as dropdownContainerRef into each RequestTab.
Tests update
tests/utils/page/actions.ts
Updated overflow tab locator to target .tippy-box .dropdown-item and match visible text for selecting overflow tabs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant TabLabel as TabLabel (DOM)
  participant RequestTab as RequestTab (component)
  participant MenuRef as menuDropdownRef
  participant Menu as MenuDropdown
  participant Dropdown as Dropdown/Tippy
  participant Container as dropdownContainerRef (DOM)

  Note over RequestTab,Menu: Right-click / show contextual menu flow
  User->>TabLabel: contextmenu (right-click)
  TabLabel->>RequestTab: onContextMenu event
  RequestTab->>RequestTab: preventDefault & stopPropagation
  RequestTab->>TabLabel: tabLabelRef.getBoundingClientRect()
  RequestTab->>MenuRef: menuDropdownRef.show({ anchorRect })
  MenuRef->>Menu: show() (imperative)
  Menu->>Dropdown: render (appendTo = Container.current || document.body)
  Dropdown->>Container: mount menu UI into appendTo target
  User->>Dropdown: click menu item
  Dropdown-->>RequestTab: dispatch action / callback
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • lohit-bruno
  • naman-bruno
  • helloanoop

Poem

A tab finds a ref as menus find their place,
Tippy renders tidy, anchored with grace.
Right-click summons structure, appendTo set just right,
Menus mount where intended — click, dispatch, delight. 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding a right-click context menu to request tabs using the MenuDropdown component.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

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 (2)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)

447-461: Silent error swallowing in async operation.

The handleCloseTab function silently catches and ignores all errors (line 460: catch (err) { }). This can hide critical issues such as:

  • Failed save operations leaving data in an inconsistent state
  • Unexpected dispatch failures
  • Network or permission errors

Consider logging the error or displaying a user-facing notification.

🔎 Suggested error handling
 async function handleCloseTab(tabUid) {
   if (!tabUid) {
     return;
   }

   try {
     const item = findItemInCollection(collection, tabUid);
     // silently save unsaved changes before closing the tab
     if (hasRequestChanges(item)) {
       await dispatch(saveRequest(item.uid, collection.uid, true));
     }

     dispatch(closeTabs({ tabUids: [tabUid] }));
-  } catch (err) { }
+  } catch (err) {
+    console.error('Failed to close tab:', err);
+    // Optionally show toast notification: toast.error('Failed to close tab');
+  }
 }

463-477: Silent error handling in revert operation.

The handleRevertChanges function also silently catches errors (line 476). While less critical than the close operation, logging would help debug issues with draft deletion.

🔎 Suggested error handling
 function handleRevertChanges() {
   if (!currentTabUid) {
     return;
   }

   try {
     const item = findItemInCollection(collection, currentTabUid);
     if (item.draft) {
       dispatch(deleteRequestDraft({
         itemUid: item.uid,
         collectionUid: collection.uid
       }));
     }
-  } catch (err) { }
+  } catch (err) {
+    console.error('Failed to revert changes:', err);
+  }
 }
🧹 Nitpick comments (2)
packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js (2)

3-173: Consider the implications of rendering global styles within the component.

The MenuDropdownGlobalStyle is rendered inside the MenuDropdown component (line 440 in index.js), which means styles are injected/removed with each component mount/unmount. While styled-components handles deduplication, this pattern can lead to:

  • Style recalculation on every MenuDropdown mount
  • Potential flash of unstyled content if multiple dropdowns exist
  • Coupling global styles to component lifecycle
💡 Alternative approach

Consider moving MenuDropdownGlobalStyle to a higher app-level component (e.g., App.js or layout) so styles are injected once globally:

// In App.js or similar
+import { MenuDropdownGlobalStyle } from 'ui/MenuDropdown/StyledWrapper';
+
 function App() {
   return (
     <>
+      <MenuDropdownGlobalStyle />
       {/* rest of app */}
     </>
   );
 }

Then remove it from MenuDropdown/index.js line 440.


175-178: Empty StyledWrapper pattern seems unnecessary.

The StyledWrapper is now an empty styled.div but still used as a wrapper. Consider either:

  • Removing it entirely and using a regular <div> in the component
  • Adding a comment explaining why it's preserved (e.g., for future styling or consistency)
💡 Simplification option

If the wrapper serves no styling purpose, replace in index.js:

-<StyledWrapper>
+<div>
   <MenuDropdownGlobalStyle />
   <Dropdown {...props}>
     {/* ... */}
   </Dropdown>
-</StyledWrapper>
+</div>

Or add a comment explaining the architectural decision:

+// StyledWrapper preserved for potential future scoped styles or layout adjustments
 const StyledWrapper = styled.div``;
📜 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 1f05ffd and 1f4990c.

📒 Files selected for processing (4)
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
🧠 Learnings (4)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Applied to files:

  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Use styled component's theme prop to manage CSS colors and not CSS variables when in the context of a styled component or any React component using the styled component

Applied to files:

  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Applied to files:

  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
🧬 Code graph analysis (2)
packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js (3)
packages/bruno-app/src/components/FolderSettings/AuthMode/StyledWrapper.js (1)
  • StyledWrapper (3-18)
packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultTypeSelector/StyledWrapper.js (1)
  • StyledWrapper (3-30)
packages/bruno-app/src/ui/ActionIcon/StyledWrapper.js (1)
  • StyledWrapper (22-55)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (2)
packages/bruno-app/src/utils/collections/index.js (3)
  • items (1351-1351)
  • hasRequestChanges (1015-1031)
  • hasRequestChanges (1015-1031)
packages/bruno-app/src/ui/MenuDropdown/index.js (1)
  • MenuDropdown (53-473)
⏰ 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). (6)
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: CLI Tests
  • GitHub Check: Playwright E2E Tests
🔇 Additional comments (16)
packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js (1)

1-1: LGTM: Import addition.

The createGlobalStyle import is correctly added to support the new global styling approach.

packages/bruno-app/src/components/Dropdown/index.js (1)

5-9: LGTM: Clean appendTo prop addition.

The appendTo prop is properly added and consistently applied to both controlled and uncontrolled Tippy modes with appropriate fallback to 'parent' for backward compatibility.

packages/bruno-app/src/ui/MenuDropdown/index.js (5)

1-3: LGTM: Import updates.

The Fragment and MenuDropdownGlobalStyle imports are correctly added to support the updated component structure.


68-68: LGTM: Appropriate default for appendTo.

Defaulting appendTo to document.body ensures the dropdown is properly portaled and avoids overflow/clipping issues within parent containers.


435-436: LGTM: Proper className composition.

The tippyClassName correctly combines the base 'menu-dropdown-tippy' class with any user-provided className, ensuring both scoped styles and customization work together.


440-440: Global style injection pattern noted.

Rendering <MenuDropdownGlobalStyle /> here means styles are injected whenever a MenuDropdown mounts. This is architecturally acceptable but may have performance implications if many instances are created/destroyed frequently. See the comment in StyledWrapper.js for alternative approaches.

Related to the architectural decision discussed in packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js lines 3-173.


445-448: LGTM: Props correctly wired to Dropdown.

The tippyClassName and appendTo props are properly passed down to the underlying Dropdown component, ensuring consistent behavior and styling.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (9)

20-20: LGTM: Import and ref additions.

The transition from Dropdown to MenuDropdown and the addition of tabLabelRef and menuDropdownRef properly support the new context menu functionality.

Also applies to: 34-34, 42-42


102-106: LGTM: Right-click handler implementation.

The handleRightClick correctly prevents default context menu behavior and triggers the MenuDropdown via the imperative ref API.


379-379: LGTM: Ref wiring.

The tabLabelRef is properly attached to the tab label element and both refs are correctly passed to RequestTabMenu for positioning and control.

Also applies to: 400-401


425-425: LGTM: Updated component signature.

The RequestTabMenu signature correctly reflects the new ref-based approach, removing the previous onDropdownCreate and dropdownTippyRef props in favor of menuDropdownRef and tabLabelRef.


429-436: LGTM: Positioning helper with proper guard.

The getTabLabelRect helper provides a zero-sized fallback rect when the ref isn't mounted, preventing Tippy errors during initialization.


479-503: LGTM: Batch async operations.

The use of Promise.all for batch tab closing operations in handleCloseOtherTabs, handleCloseTabsToTheLeft, and handleCloseTabsToTheRight is appropriate and efficient. The handleCloseSavedTabs synchronously closes multiple tabs, which is acceptable since they have no unsaved changes.


505-555: LGTM: Properly memoized menu items.

The menuItems array is well-structured with appropriate dependencies in the useMemo hook, ensuring efficient re-renders and correct disabled states based on tab context.


557-567: LGTM: MenuDropdown configuration.

The menuDropdown JSX is properly configured with:

  • Correct ref for imperative control
  • bottom-start placement for natural positioning
  • document.body appendTo for portal behavior
  • getReferenceClientRect for custom positioning

The empty <span></span> trigger is appropriate since the menu is controlled programmatically via ref.


583-583: LGTM: MenuDropdown rendering.

The menuDropdown is correctly rendered within the Fragment alongside the modal components.

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: 1

🧹 Nitpick comments (3)
packages/bruno-app/src/components/Dropdown/StyledWrapper.js (2)

109-115: Inline RGBA calculation for delete-item hover background.

The inline hex-to-rgba conversion works but could be extracted to a theme utility function for reusability across components.

Optional: Extract to utility function

Consider adding a utility function to your theme or utils:

// utils/colors.js
export const hexToRgba = (hex, alpha) => {
  const cleanHex = hex.replace('#', '');
  const r = parseInt(cleanHex.substring(0, 2), 16);
  const g = parseInt(cleanHex.substring(2, 4), 16);
  const b = parseInt(cleanHex.substring(4, 6), 16);
  return `rgba(${r}, ${g}, ${b}, ${alpha})`;
};

Then use it:

-        background-color: ${({ theme }) => {
-          const hex = theme.colors.text.danger.replace('#', '');
-          const r = parseInt(hex.substring(0, 2), 16);
-          const g = parseInt(hex.substring(2, 4), 16);
-          const b = parseInt(hex.substring(4, 6), 16);
-          return `rgba(${r}, ${g}, ${b}, 0.04)`;
-        }} !important;
+        background-color: ${({ theme }) => hexToRgba(theme.colors.text.danger, 0.04)} !important;

84-86: Inconsistent state class naming.

The .selected-focused class doesn't follow the dropdown-item-* naming pattern used by other state classes (.dropdown-item-focused, .dropdown-item-active). Consider renaming to .dropdown-item-selected-focused for consistency.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

505-555: Consider memoizing handlers with useCallback.

The menuItems array is memoized, but the handler functions are recreated on every render. For better optimization, wrap handlers like handleCloseTab, handleCloseOtherTabs, etc., in useCallback with appropriate dependencies.

Optional: Memoize handlers
+const handleCloseTab = useCallback(async (tabUid) => {
+  if (!tabUid) return;
+  try {
+    const item = findItemInCollection(collection, tabUid);
+    if (hasRequestChanges(item)) {
+      await dispatch(saveRequest(item.uid, collection.uid, true));
+    }
+    dispatch(closeTabs({ tabUids: [tabUid] }));
+  } catch (err) {}
+}, [collection, dispatch]);

+const handleRevertChanges = useCallback(() => {
+  if (!currentTabUid) return;
+  try {
+    const item = findItemInCollection(collection, currentTabUid);
+    if (item.draft) {
+      dispatch(deleteRequestDraft({
+        itemUid: item.uid,
+        collectionUid: collection.uid
+      }));
+    }
+  } catch (err) {}
+}, [currentTabUid, collection, dispatch]);

// ... similarly for other handlers
📜 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 1f4990c and 6f40dc5.

📒 Files selected for processing (7)
  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
  • packages/bruno-app/src/components/RequestTabs/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
💤 Files with no reviewable changes (1)
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
✅ Files skipped from review due to trivial changes (1)
  • packages/bruno-app/src/components/RequestTabs/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/bruno-app/src/ui/MenuDropdown/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Applied to files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Applied to files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js (1)
  • isFolder (13-13)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/CloneCollectionItem/index.js (1)
  • isFolder (20-20)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RenameCollectionItem/index.js (1)
  • isFolder (21-21)
packages/bruno-app/src/ui/ActionIcon/index.js (1)
  • ActionIcon (22-56)
⏰ 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). (6)
  • GitHub Check: CLI Tests
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - Linux
🔇 Additional comments (6)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

647-673: LGTM — Chevron rendering refactor.

The conditional rendering eliminates unnecessary DOM elements when no chevron is needed, improving performance and clarity. Logic correctly handles folders, requests with examples, and requests without examples.

packages/bruno-app/src/components/Dropdown/index.js (1)

5-26: LGTM — appendTo prop and render refactor.

The appendTo prop addition enables flexible dropdown positioning while maintaining backward compatibility with the default 'parent' value. Moving StyledWrapper into Tippy's render prop aligns with modern Tippy.js patterns and ensures proper attribute handling.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (4)

102-106: LGTM — Right-click handler.

Correctly prevents default context menu and triggers the MenuDropdown via ref. Optional chaining ensures safety.


431-436: LGTM — Safe positioning helper.

The zero-sized rect fallback when the element isn't mounted prevents Tippy positioning errors.


447-503: LGTM — Handler refactoring.

The handlers are correctly refactored to support async operations and the new MenuDropdown pattern. The signature change for handleCloseTab (removing the event parameter) aligns with the new usage pattern, and the Promise.all usage for batch operations is appropriate.


399-406: LGTM — RequestTabMenu refactor.

The signature change from callback-based (onDropdownCreate) to ref-based (menuDropdownRef, tabLabelRef) control is cleaner and aligns with the MenuDropdown pattern. All necessary props are correctly passed.

Also applies to: 425-425

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 (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js (1)

434-434: Memoize the DOM selector to avoid repeated queries on every render.

document.querySelector('.sidebar-sections-container') executes on every render. Store the result in a ref or memoize it with useMemo to improve performance.

Additionally, verify that MenuDropdown handles a null appendTo value gracefully if the container element hasn't mounted yet.

🔎 Suggested fix using useMemo

Add this near the other hooks (around line 65):

+ const appendToContainer = React.useMemo(() => document.querySelector('.sidebar-sections-container'), []);

Then update line 434:

-              appendTo={document.querySelector('.sidebar-sections-container')}
+              appendTo={appendToContainer}
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

687-687: Memoize the DOM selector to avoid repeated queries on every render.

Same concern as in Collection/index.js: document.querySelector('.sidebar-sections-container') runs on every render. Memoize the result to avoid unnecessary DOM queries.

Also verify that MenuDropdown handles null appendTo gracefully if the container hasn't mounted yet.

🔎 Suggested fix using useMemo

Add this near the other hooks (around line 72):

+ const appendToContainer = React.useMemo(() => document.querySelector('.sidebar-sections-container'), []);

Then update line 687:

-              appendTo={document.querySelector('.sidebar-sections-container')}
+              appendTo={appendToContainer}
📜 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 6f40dc5 and 128615a.

📒 Files selected for processing (2)
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (4)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js (1)
  • isFolder (13-13)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/CloneCollectionItem/index.js (1)
  • isFolder (20-20)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RenameCollectionItem/index.js (1)
  • isFolder (21-21)
packages/bruno-app/src/ui/ActionIcon/index.js (1)
  • ActionIcon (22-56)
⏰ 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). (6)
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: Unit Tests
  • GitHub Check: CLI Tests
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Windows
🔇 Additional comments (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

647-673: LGTM! Clean conditional rendering for chevrons.

The refactored logic correctly shows chevrons only when they're functional:

  • Folders get a chevron to expand/collapse child items
  • Requests with examples get a chevron to expand/collapse examples
  • Other requests render nothing (no expandable content)

This improves the UI by removing non-functional chevrons.

bijin-bruno
bijin-bruno previously approved these changes Dec 24, 2025
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: 1

🧹 Nitpick comments (1)
tests/utils/page/actions.ts (1)

671-671: Consider a more specific locator to avoid potential conflicts.

The selector .tippy-box .dropdown-item could theoretically match items from multiple dropdowns if several tippy boxes are visible simultaneously. While this is unlikely given the controlled test environment and the overflow button was just clicked, you could make it more robust by scoping to the recently opened tippy box.

💡 More specific locator approach
-      const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName });
+      const dropdownItem = page.locator('.tippy-box[data-state="visible"] .dropdown-item').filter({ hasText: tabName });

This ensures you're targeting only the currently visible tippy box, though the current implementation is likely sufficient for the test environment.

📜 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 9c2d6dd and 387fda7.

📒 Files selected for processing (1)
  • tests/utils/page/actions.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • tests/utils/page/actions.ts
tests/**/**.*

⚙️ CodeRabbit configuration file

tests/**/**.*: Review the following e2e test code written using the Playwright test library. Ensure that:

  • Follow best practices for Playwright code and e2e automation

  • Try to reduce usage of page.waitForTimeout(); in code unless absolutely necessary and the locator cannot be found using existing expect() playwright calls

  • Avoid using page.pause() in code

  • Use locator variables for locators

  • Avoid using test.only

  • Use multiple assertions

  • Promote the use of test.step as much as possible so the generated reports are easier to read

  • Ensure that the fixtures like the collections are nested inside the fixtures folder

    Fixture Example*: Here's an example of possible fixture and test pair

    .
    ├── fixtures
    │   └── collection
    │       ├── base.bru
    │       ├── bruno.json
    │       ├── collection.bru
    │       ├── ws-test-request-with-headers.bru
    │       ├── ws-test-request-with-subproto.bru
    │       └── ws-test-request.bru
    ├── connection.spec.ts # <- Depends on the collection in ./fixtures/collection
    ├── headers.spec.ts
    ├── persistence.spec.ts
    ├── variable-interpolation
    │   ├── fixtures
    │   │   └── collection
    │   │       ├── environments
    │   │       ├── bruno.json
    │   │       └── ws-interpolation-test.bru
    │   ├── init-user-data
    │   └── variable-interpolation.spec.ts # <- Depends on the collection in ./variable-interpolation/fixtures/collection
    └── subproto.spec.ts
    

Files:

  • tests/utils/page/actions.ts
⏰ 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). (6)
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: CLI Tests
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: SSL Tests - Windows

Comment on lines 670 to +671
// Wait for dropdown to appear and click the menu item (overflow tabs are rendered as menuitems)
const dropdownItem = page.locator('.tippy-content').getByRole('menuitem', { name: tabName });
const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName });
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update comment to reflect the actual implementation.

The comment mentions "menuitems" but the implementation uses .dropdown-item. This inconsistency could confuse future maintainers.

🔎 Proposed fix
-      // Wait for dropdown to appear and click the menu item (overflow tabs are rendered as menuitems)
+      // Wait for dropdown to appear and click the dropdown item
       const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Wait for dropdown to appear and click the menu item (overflow tabs are rendered as menuitems)
const dropdownItem = page.locator('.tippy-content').getByRole('menuitem', { name: tabName });
const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName });
// Wait for dropdown to appear and click the dropdown item
const dropdownItem = page.locator('.tippy-box .dropdown-item').filter({ hasText: tabName });
🤖 Prompt for AI Agents
In tests/utils/page/actions.ts around lines 670 to 671, the inline comment
incorrectly refers to "menuitems" while the code selects '.dropdown-item';
update the comment to accurately describe the implementation (e.g., "Wait for
dropdown to appear and click the dropdown item (overflow tabs are rendered as
.dropdown-item)") so future maintainers aren't misled by the terminology
mismatch.

@abhishek-bruno abhishek-bruno force-pushed the feat/request-tab-right-click-menu-v2 branch from 387fda7 to 8222496 Compare December 24, 2025 13:22
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)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

448-462: Silent error swallowing in handleCloseTab.

The empty catch (err) { } block silently discards errors. At minimum, log the error for debugging.

🔎 Suggested fix
- } catch (err) { }
+ } catch (err) {
+   console.error('Failed to close tab:', err);
+ }
🧹 Nitpick comments (3)
packages/bruno-app/src/components/Dropdown/StyledWrapper.js (2)

109-115: Hex parsing assumes 6-character format.

The inline hex-to-rgba conversion will fail silently for 3-character shorthand hex codes (e.g., #f00). Consider extracting this to a utility function with proper validation, or document that theme colors must use 6-char format.

🔎 Suggested utility extraction
// utils/color.js
export const hexToRgba = (hex, alpha) => {
  const cleanHex = hex.replace('#', '');
  const fullHex = cleanHex.length === 3 
    ? cleanHex.split('').map(c => c + c).join('') 
    : cleanHex;
  const r = parseInt(fullHex.substring(0, 2), 16);
  const g = parseInt(fullHex.substring(2, 4), 16);
  const b = parseInt(fullHex.substring(4, 6), 16);
  return `rgba(${r}, ${g}, ${b}, ${alpha})`;
};

50-55: Consider reducing !important usage.

Multiple !important declarations (lines 51, 53-54, 115, 117) suggest specificity conflicts. If these are necessary for overriding Tippy's default styles, that's fine—otherwise consider increasing selector specificity instead.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)

430-437: Consider memoizing getTabLabelRect.

This function is recreated on every render and passed to MenuDropdown. Wrap it in useCallback for stability.

🔎 Suggested fix
- const getTabLabelRect = () => {
-   if (!tabLabelRef.current) {
-     return { width: 0, height: 0, top: 0, bottom: 0, left: 0, right: 0 };
-   }
-   return tabLabelRef.current.getBoundingClientRect();
- };
+ const getTabLabelRect = useCallback(() => {
+   if (!tabLabelRef.current) {
+     return { width: 0, height: 0, top: 0, bottom: 0, left: 0, right: 0 };
+   }
+   return tabLabelRef.current.getBoundingClientRect();
+ }, []);
📜 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 387fda7 and 8222496.

📒 Files selected for processing (10)
  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
  • packages/bruno-app/src/components/RequestTabs/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
  • packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
  • tests/utils/page/actions.ts
💤 Files with no reviewable changes (1)
  • packages/bruno-app/src/ui/MenuDropdown/StyledWrapper.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
  • packages/bruno-app/src/components/RequestTabs/index.js
  • tests/utils/page/actions.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
  • packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
🧠 Learnings (3)
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles

Applied to files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
📚 Learning: 2025-12-05T20:31:33.005Z
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Component CSS might also change layout but Tailwind classes shouldn't define colors

Applied to files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/Dropdown/StyledWrapper.js
  • packages/bruno-app/src/components/Dropdown/index.js
  • packages/bruno-app/src/components/RequestTabs/RequestTab/index.js
  • packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js
  • packages/bruno-app/src/ui/MenuDropdown/index.js
🧬 Code graph analysis (2)
packages/bruno-app/src/components/Dropdown/index.js (1)
packages/bruno-app/src/components/CollectionSettings/Protobuf/StyledWrapper.js (1)
  • StyledWrapper (3-11)
packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (1)
packages/bruno-app/src/ui/MenuDropdown/index.js (1)
  • MenuDropdown (52-464)
⏰ 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). (6)
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: Unit Tests
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: CLI Tests
🔇 Additional comments (12)
packages/bruno-app/src/components/Dropdown/index.js (2)

5-9: LGTM! Clean prop addition with sensible defaults.

The appendTo prop is properly destructured and defaults to 'parent' in both controlled and uncontrolled branches.


12-25: Good refactor to Tippy's render prop pattern.

Using render instead of content gives better control over the dropdown's DOM structure. The attrs spread ensures Tippy's data attributes are properly applied.

packages/bruno-app/src/ui/MenuDropdown/index.js (2)

433-462: LGTM! Clean composition with the Dropdown component.

The render structure properly handles header/footer with dividers, and dropdownProps spread correctly passes through props like appendTo to the underlying Dropdown.


444-448: Consider if header click should close the dropdown.

The header container has onClick={handleClickOutside} which closes the dropdown when clicking the header area. Verify this is intentional behavior—it might surprise users clicking within the header.

packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js (2)

15-15: LGTM! Clean ref setup for dropdown container.

The dropdownContainerRef provides a stable mounting target for dropdowns across the sidebar.


60-62: Add styling to the wrapper div to accommodate the flex layout.

The wrapper div needs explicit styling to support the flex layout inside. Add width: 100%; height: 100% or display: contents to prevent height collapse when StyledWrapper uses h-full.

<div ref={dropdownContainerRef} style={{ width: '100%', height: '100%' }}>
  {children}
</div>

Alternatively, use display: contents if the ref is only for event delegation and you don't need layout impact.

⛔ Skipped due to learnings
Learnt from: CR
Repo: usebruno/bruno PR: 0
File: CODING_STANDARDS.md:0-0
Timestamp: 2025-12-05T20:31:33.005Z
Learning: Applies to **/*.{jsx,tsx} : Styled Components are used as wrappers to define both self and children components style; Tailwind classes are used specifically for layout based styles
packages/bruno-app/src/components/Dropdown/StyledWrapper.js (2)

4-13: LGTM! Solid base wrapper styling.

Good defaults with max-height: 90vh for overflow handling and overflow-y: auto for scrollable content.


152-156: Good accessibility handling with focus-visible.

The outline with outline-offset: -2px provides clear keyboard navigation feedback while keeping it visually contained.

packages/bruno-app/src/components/RequestTabs/RequestTab/index.js (4)

29-29: Good addition of dropdownContainerRef prop.

This addresses the previous review concern about fragile DOM selectors.


102-106: LGTM! Clean imperative menu trigger.

Using menuDropdownRef.current?.show() is the correct pattern for programmatic menu opening on right-click.


506-556: Stale closure risk in menuItems useMemo.

The menuItems array captures handler functions that reference currentTabUid, collection, etc. but the useMemo dependency array doesn't include handleCloseTab, handleRevertChanges, etc. Since these are plain functions (not wrapped in useCallback), they always have access to fresh closures—but if you later refactor them, this could bite you.

Current code works correctly, but for maintainability, consider wrapping handlers in useCallback or acknowledge this is intentional.


558-568: LGTM! Proper MenuDropdown integration.

The appendTo fallback to document.body and getReferenceClientRect usage correctly positions the dropdown relative to the tab label while mounting in the container ref.

…llectionItem components for improved dropdown behavior
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)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (1)

645-670: LGTM! Conditional ActionIcon wrapping improves UI consistency.

The chevron rendering logic is clear and correct. The ActionIcon wrapper provides consistent hover states and styling while preserving the original click behavior on the icons themselves.

Optional: Consider adding aria-labels for improved accessibility

The chevron icons serve as interactive collapse/expand controls. Adding descriptive aria-labels would improve screen reader support:

 {isFolder ? (
-  <ActionIcon style={{ width: 16, minWidth: 16 }}>
+  <ActionIcon style={{ width: 16, minWidth: 16 }} aria-label={itemIsCollapsed ? 'Expand folder' : 'Collapse folder'}>
     <IconChevronRight
 ) : hasExamples ? (
-  <ActionIcon style={{ width: 16, minWidth: 16 }}>
+  <ActionIcon style={{ width: 16, minWidth: 16 }} aria-label={examplesExpanded ? 'Collapse examples' : 'Expand examples'}>
     <IconChevronRight
📜 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 8222496 and e289a52.

📒 Files selected for processing (2)
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CODING_STANDARDS.md)

**/*.{js,jsx,ts,tsx}: Use 2 spaces for indentation. No tabs, just spaces
Stick to single quotes for strings. For JSX/TSX attributes, use double quotes (e.g., )
Always add semicolons at the end of statements
No trailing commas
Always use parentheses around parameters in arrow functions, even for single params
For multiline constructs, put opening braces on the same line, and ensure consistency. Minimum 2 elements for multiline
No newlines inside function parentheses
Space before and after the arrow in arrow functions. () => {} is good
No space between function name and parentheses. func() not func ()
Semicolons go at the end of the line, not on a new line
Names for functions need to be concise and descriptive
Add in JSDoc comments to add more details to the abstractions if needed
Add in meaningful comments instead of obvious ones where complex code flow is explained properly

Files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
🧠 Learnings (1)
📚 Learning: 2025-12-17T21:41:24.730Z
Learnt from: naman-bruno
Repo: usebruno/bruno PR: 6407
File: packages/bruno-app/src/components/Environments/ConfirmCloseEnvironment/index.js:5-41
Timestamp: 2025-12-17T21:41:24.730Z
Learning: Do not suggest PropTypes validation for React components in the Bruno codebase. The project does not use PropTypes, so reviews should avoid proposing PropTypes and rely on the existing typing/validation approach (e.g., TypeScript or alternative runtime checks) if applicable. This guideline applies broadly to all JavaScript/JSX components in the repo.

Applied to files:

  • packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js
🧬 Code graph analysis (1)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (6)
packages/bruno-app/src/components/Sidebar/Collections/Collection/index.js (1)
  • useSidebarAccordion (51-51)
packages/bruno-app/src/components/Sidebar/SidebarAccordionContext.js (3)
  • useSidebarAccordion (5-11)
  • useSidebarAccordion (5-11)
  • dropdownContainerRef (15-15)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/DeleteCollectionItem/index.js (1)
  • isFolder (13-13)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/RenameCollectionItem/index.js (1)
  • isFolder (21-21)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/CloneCollectionItem/index.js (1)
  • isFolder (20-20)
packages/bruno-app/src/ui/ActionIcon/index.js (1)
  • ActionIcon (22-56)
⏰ 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). (5)
  • GitHub Check: SSL Tests - macOS
  • GitHub Check: SSL Tests - Windows
  • GitHub Check: SSL Tests - Linux
  • GitHub Check: Playwright E2E Tests
  • GitHub Check: CLI Tests
🔇 Additional comments (2)
packages/bruno-app/src/components/Sidebar/Collections/Collection/CollectionItem/index.js (2)

56-56: LGTM! Hook usage follows the established pattern.

The import and usage of useSidebarAccordion is consistent with the same pattern in the Collection component. The hook will throw a clear error if used outside the context provider.

Also applies to: 59-59


685-686: LGTM! Fixed positioning improves dropdown behavior.

The popperOptions with strategy: 'fixed' and the appendTo pattern align with the coordinated refactor mentioned in the PR objectives. The optional chaining and fallback to document.body safely handle edge cases.

@bijin-bruno bijin-bruno merged commit 1b8eece into usebruno:main Dec 24, 2025
10 of 11 checks passed
bijin-bruno added a commit that referenced this pull request Feb 13, 2026
* feat: button storybook

* feat: update button component with new rounded options and story

* fix: pasting request ito parent folder even if request is selected (#6446)

* Add right-click context menu to request tabs with MenuDropdown # (#6502)

* refactor: replace Dropdown with MenuDropdown in RequestTab component; update Dropdown props handling in Dropdown component

* refactor: remove Portal import and simplify menuDropdown rendering in RequestTab component

* refactor: streamline RequestTabMenu functionality and improve tab closing methods with async handling

* refactor: enhance Dropdown and MenuDropdown components with improved props handling and styling adjustments

* refactor: enhance Dropdown and MenuDropdown components by improving structure and removing unused styles

* refactor: update Dropdown and MenuDropdown components to append to sidebar sections container for improved layout

* refactor: integrate dropdownContainerRef for improved MenuDropdown positioning in RequestTabs and Sidebar components

* refactor: update Dropdown component to include 'tippy-box' class for e2e test selections

* refactor: update dropdown item selection logic in selectRequestPaneTab function for improved accuracy

* refactor: add fixed positioning to popperOptions in Collection and CollectionItem components for improved dropdown behavior

---------

Co-authored-by: sanjai <sanjai@usebruno.com>

* export & import in opencollection format (#6329)

---------

Co-authored-by: Anoop M D <anoop.md1421@gmail.com>
Co-authored-by: Anoop M D <anoop@usebruno.com>
Co-authored-by: Pooja <pooja@usebruno.com>
Co-authored-by: Abhishek S Lal <abhishek@usebruno.com>
Co-authored-by: sanjai <sanjai@usebruno.com>
Co-authored-by: naman-bruno <naman@usebruno.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants